Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type Inference & Performance Improvements #112

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

uniment
Copy link

@uniment uniment commented Jan 30, 2024

Improve performance:

  1. Restore type information to MapCallback and ObserverFunction
  2. Allow method specializations for functions which are likely to be called many times
  3. Eliminate unnecessary invokelatest

With Julia 1.9's native code caching, some compile-time optimizations that had previously been helpful have become counterproductive.

Before this PR:

julia> using BenchmarkTools
       using Observables: Observable, @map
       const a = Observable(1.0);  @btime notify(a)
       const b = @map &a+1;        @btime notify(a)
       const c = @map &a+2;        @btime notify(a)
       a[], b[], c[]
  5.100 ns (0 allocations: 0 bytes)
  575.275 ns (10 allocations: 160 bytes)
  1.210 μs (20 allocations: 320 bytes)
(1.0, 2.0, 3.0)

After this PR:

julia> using BenchmarkTools
       using Observables: Observable, @map
       const a = Observable(1.0);  @btime notify(a)
       const b = @map &a+1;        @btime notify(a)
       const c = @map &a+2;        @btime notify(a)
       a[], b[], c[]
  5.200 ns (0 allocations: 0 bytes)
  30.723 ns (1 allocation: 16 bytes)
  55.193 ns (2 allocations: 32 bytes)
(1.0, 2.0, 3.0)

Because invokelatest is still used in notify, it is not necessary to use it in the functor for MapCallback. Thus this PR does not affect issue #50, which remains closed.

Restore type information to `MapCallback` and `ObserverFunction` and eliminate unnecessary `invokelatest` to improve performance.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant