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

Fix Pkg.precompile ext races #3645

Merged

Conversation

IanButterworth
Copy link
Member

If I force this to @info
https://github.com/JuliaLang/julia/blob/8cacc1c7320ac8092625b4dd2e4b96b55c14d9d3/base/loading.jl#L3069

On master I get

(@v1.11) pkg> precompile
Precompiling project...
  169 dependencies successfully precompiled in 98 seconds. 35 already precompiled.
  2 dependencies had output during precompilation:
┌ ChainRulesCore
│  ┌ Info: Waiting for another process (pid: 41643) to finish precompiling CompatLinearAlgebraExt [dbe5ba0b-aecc-598a-a867-79051b540f49]
│  └ @ Base loading.jl:3071
└  
┌ Distances → DistancesChainRulesCoreExt
│  ┌ Info: Waiting for another process (pid: 41643) to finish precompiling DistancesSparseArraysExt [feee1041-5e3e-5b48-b2f8-f25e4435f184]
│  └ @ Base loading.jl:3071
└  

which indicates that there is race that's being limited by the cache pidlocks.

If a package has multiple active extensions Pkg.precompile wasn't sequencing the extensions meaning they raced to precompile.

So, with this PR

(@v1.11) pkg> precompile
Precompiling project...
  169 dependencies successfully precompiled in 102 seconds. 35 already precompiled.
  1 dependency had output during precompilation:
┌ ChainRulesCore
│  ┌ Info: Waiting for another process (pid: 49287) to finish precompiling CompatLinearAlgebraExt [dbe5ba0b-aecc-598a-a867-79051b540f49]
│  └ @ Base loading.jl:3071
└  

I'm looking into the ChainRulesCore -> CompatLinearAlgebraExt issue too.

@IanButterworth IanButterworth added the precompile Pkg.precompile label Oct 9, 2023
@IanButterworth IanButterworth force-pushed the ib/precompile_ext_race branch from f1d0c39 to 0a14a7a Compare October 9, 2023 23:19
@IanButterworth IanButterworth changed the title make pkg exts precompile sequentially to avoid race Fix Pkg.precompile ext races Oct 10, 2023
@IanButterworth
Copy link
Member Author

IanButterworth commented Oct 10, 2023

Ok, after that 2nd commit I now get no warnings.

Turns out the extension dependency insertion loop needed to run after the full depsmap had been built, otherwise it wasn't aware of the full environment. (Duh)

By the way, the state management in Pkg.precompile has become a mess. I fully intend on tidying it up, but I think I will do it after moving it to Base, as doing both tasks at the same time has tripped up my efforts to get that done a couple of times already.

Copy link
Member

@KristofferC KristofferC left a comment

Choose a reason for hiding this comment

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

I haven't gone through the logic but the logging output seems to confirm it works so 👍

@IanButterworth IanButterworth merged commit b02fb95 into JuliaLang:master Oct 10, 2023
12 of 13 checks passed
@IanButterworth IanButterworth deleted the ib/precompile_ext_race branch October 10, 2023 14:01
IanButterworth added a commit that referenced this pull request Oct 10, 2023
(cherry picked from commit b02fb95)
IanButterworth added a commit that referenced this pull request Oct 10, 2023
(cherry picked from commit b02fb95)
@IanButterworth IanButterworth mentioned this pull request Oct 10, 2023
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
precompile Pkg.precompile
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants