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

Always use invokelatest #65

Merged
merged 2 commits into from
Jan 20, 2021
Merged

Always use invokelatest #65

merged 2 commits into from
Jan 20, 2021

Conversation

timholy
Copy link
Member

@timholy timholy commented Jan 20, 2021

map was not safe in conjunction with @async. Since even one of the
examples in the manual used it that way, let's just get rid of the
special dispatch for InternalFunction.

Fixes #50

@SimonDanisch, I know this one might have made you sad. Is there a place where this matters more than others?

Benchmarks:

julia> f1(x) = sin(x)
f1 (generic function with 1 method)

julia> f2(x) = Base.invokelatest(sin, x)
f2 (generic function with 1 method)

julia> using BenchmarkTools

julia> @btime f1(x) setup=(x=rand());
  3.681 ns (0 allocations: 0 bytes)

julia> @btime f2(x) setup=(x=rand());
  55.407 ns (3 allocations: 48 bytes)

julia> f3(x) = sin(x[])
f3 (generic function with 1 method)

julia> @btime f3(x) setup=(x=Ref{Any}(rand()));
  18.016 ns (1 allocation: 16 bytes)

So it's about 3x the cost of a single runtime dispatch (which due to poor inferrability, Makie has a lot of).

`map` was not safe in conjunction with `@async`. Since even one of the
examples in the manual used it that way, let's just get rid of the
special dispatch for `InternalFunction`.

Fixes #50
@SimonDanisch
Copy link
Member

Well, if it crashes I guess we need to get rid of it in any case ;)
Maybe we can bring back something like this for code where it matters with FunctionWrappers or so!

@timholy timholy merged commit 2fe0cf8 into master Jan 20, 2021
@timholy timholy deleted the teh/fix_world_age branch January 20, 2021 15:00
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.

Mapping observable leads to world age error
2 participants