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

also check build ids when checking for staleness of precompile files #3308

Merged
merged 1 commit into from
Jan 13, 2023

Conversation

KristofferC
Copy link
Member

@KristofferC KristofferC commented Jan 12, 2023

I noticed a case (which I can't repro now) where pkg> precompile considered everything precompiled, but loading the package still had to recompile. When I debugged, I saw that the check at https://github.com/JuliaLang/julia/blob/fec8304a8c6e2157759b76d20339b426201a5f61/base/loading.jl#L2602-L2606 failed which caused Julia to recompile the package.

However, in Pkg, we only call the 2-arg version of stale_cachefile which disables this check https://github.com/JuliaLang/julia/blob/fec8304a8c6e2157759b76d20339b426201a5f61/base/loading.jl#L2554-L2556

This PR copies the recursive checks for staleness in https://github.com/JuliaLang/julia/blob/79ceb8dbeab1b5a47c6bd664214616c19607ffab/base/loading.jl#L1309-L1336 but it adds some caching to prevent checking this for the same package multiple times. This still leads to some slowdown. On a pretty big environment I measure before this change a "no-op" precompile to take 469ms while with this change it takes 834 ms. However, with some more caching in Base (JuliaLang/julia#48247) it now takes 578 ms which is not so bad compared to before.

@timholy
Copy link
Member

timholy commented Jan 12, 2023

Thank you! I don't know whether it's exactly this issue, but I've seen the phenomenon you describe quite frequently, and never tracked it down. If this fixes it, it's a significant QOL improvement.

@baggepinnen
Copy link

Fantastic! This bug has been eating up a lot of time and patience for quite some time now!

@KristofferC
Copy link
Member Author

Since I can't repro my original issue (I was being stupid not to save my .compiled folder) I am not 100% sure that this will fix all cases, but it should IMO no longer be able to trigger the same failure mode that I originally saw.

Copy link
Member

@IanButterworth IanButterworth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for hunting this out!

@IanButterworth IanButterworth changed the title also check build ids when checking for stalenss of precompile files also check build ids when checking for staleness of precompile files Jan 13, 2023
@KristofferC KristofferC merged commit 6fb3865 into master Jan 13, 2023
@KristofferC KristofferC deleted the kc/stale_buildid branch January 13, 2023 18:43
KristofferC added a commit that referenced this pull request Jan 13, 2023
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.

None yet

4 participants