-
Notifications
You must be signed in to change notification settings - Fork 101
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
Faster startup of generated JLL packages #812
Conversation
I keep asking you the question but not getting an answer: did you actually benchmark this and found real improvements? |
I only tested and confirmed speed-up this way (since this PR, I noticed your seemingly contradicting answer elsewhere, you got lower time than either, but strangely also fewer allocations than I do on 1.5.0-beta1.0):
|
Don't start Julia with As I've shown in JuliaBinaryWrappers/FFMPEG_jll.jl#1 (comment), I see a very slight regression (but too small to be taken seriously, can also be random jitter), but no concrete improvement anyway. |
The reason my change was ineffective (but not slower) when only applied to FFMPEG_jll (or FFMPEG.jl), is that it has many dependencies, and for (some of) those it matters (what https://github.com/JuliaBinaryWrappers/LAME_jll.jl/blob/master/src/LAME_jll.jl The best time I ever got over several tries for that one package unchanged was something over 400 ms for min. With the change I got almost 20 ms off, for just that one JLL:
Startup of Julia is 157.9 ms, what you need to subtract from all numbers, to get relative difference, otherwise it's seemingly lower. |
Bump. I think I showed some minor speedup, and enabled on all should be more. I think we should merge this/test. We then have time to revert before 1.5 is release if it turns out to be a problem. |
Those benchmark numbers are incredibly noisy (are you perhaps running the test on a laptop?) and so I wouldn't trust any deduction from that. So here is what I did to improve on that:
Here is what I got. First the startup speeds with default optimization /
Now let's see how much it costs to just do
Finally, I edited
As we can see, there is a slight improvement at the default optimization level: from 278.5 ms ± 1.2 ms down to 268.7 ms ± 0.7 ms. But it is much smaller than the improvement from going to |
I've repeated this with
Since this is quite a bit slower, I only used 20 runs.
After the patch:
A reduction from 1145 ms ± 4 ms down to 967.8 ms ± 3.7 ms, i.e., a saving of ~177 ms or about 15%,, indeed seems quite compelling. Of course 968ms is still long. Oh and if one could get even closer to what -O1/-O0 give, that'd be another 70 ms. |
I'm not on a laptop, yes noisy (with browser running), so I rely on min. time, but anyway it seems you're convinced. It doesn't have to be a lot of improvement, any helps, and for JLLs I do not see a potential for a downside (always same generated code, that doesn't rely on optimizer). |
Since Julia 1.5 RC1 is imminent (?), no longer blocked for merging, I think we should go for this, while we still have time for the final release of 1.5. |
@PallHaraldsson I don't see why the release of Julia 1.5 matters for this? Most JLLs are built via Yggdrasil. And they won't be regenerated when this PR is merged either -- each one will have to be manually updated. But this is independent of Julia 1.5. |
Hmm, I assumed would be regenerated for Yggdrasil. Or that would be helpful... if it's possible to make that happen. Otherwise, people will see gradual decrease of startup time for JLL packages, as JLL dependencies get updated to (some might never?). |
Thanks @PallHaraldsson! |
No description provided.