-
Notifications
You must be signed in to change notification settings - Fork 19
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
Precompile some functions for faster TTFX #174
Conversation
I guess that the time spent on function definitions is anyway happening at precompile time inside packages though, but the precompile statements in this pr are practically zero cost so I think that they could be added nonetheless. |
hi @aplavin what do you think of this PR? |
Idk, we can add this I guess, what do you think @jw3126? |
Mhh the gain is minor, I wonder if it is worth the additional dependency. How does this impact precompile time? |
almost the same for precompile times (maybe 0.1 second more), but actually the additional dependency impacts a bit the importing: julia> @time using Accessors
0.039866 seconds (36.46 k allocations: 4.357 MiB, 10.73% compilation time) # this pr
0.031610 seconds (25.88 k allocations: 3.539 MiB, 13.73% compilation time) # master
julia> struct Q k::Float64 end
julia> q = Q(1.0)
Q(1.0)
julia> @time @eval f(q) = @set q.k = 2.0;
0.008145 seconds (2.08 k allocations: 144.578 KiB, 88.98% compilation time) # this pr
0.022843 seconds (14.67 k allocations: 1.015 MiB, 95.64% compilation time) # master
julia> @time f(q);
0.034677 seconds (23.59 k allocations: 1.614 MiB, 99.95% compilation time) # this pr
0.034935 seconds (23.64 k allocations: 1.632 MiB, 99.93% compilation time) # master so I close for now, let me know if you think it's still worth it |
Thanks @Tortar for experimenting with this. |
One possibility would be to add a weak-dep / extension targetting PrecompileTools.jl, so that if the user has another package that already going to bring in PrecompileTools, then we might as well also do some precompilation, but if not, we can skip it. |
This has not a big impact but it works, e.g. now
Addresses partially #173