account for extensions indirectly depending on each other in parallel package precompilation #53972
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
KristofferC
force-pushed
the
kc/extension_indirect_deps
branch
from
4a3b248
to
b0d865e
Compare
This was referenced Apr 9, 2024
|
This seems to be generally more correct since it fixes other issues that pop up due to the "fake dependencies" that extensions now have on each other JuliaLang/Pkg.jl#3889. I'll run a PkgEval to evaluate this a bit more though. |
KristofferC
force-pushed
the
kc/extension_indirect_deps
branch
from
b0d865e
to
f0f464b
Compare
|
@nanosoldier |
KristofferC
changed the title
…package precompilation
KristofferC
force-pushed
the
kc/extension_indirect_deps
branch
from
f0f464b
to
6f785ca
Compare
IanButterworth
approved these changes
May 8, 2024
|
The package evaluation job you requested has completed - possible new issues were detected. |
|
Looking at the PkgEval log, this PR detects circular deps in: If we look at the log for a random one: |
|
@nanosoldier |
|
The package evaluation job you requested has completed - no new issues were detected. |
|
I think it is worth trying this on master at least. |
|
One thing I don't understand is how code loading works if precompilation isn't happy |
|
Yeah, I am not sure either. |
This was referenced May 23, 2024
|
Should this be backported to at least 1.11? |
|
Yeah, that's probably a good idea. |
IanButterworth
added
the
backport 1.11
KristofferC
added a commit
that referenced
this pull request
May 27, 2024
… package precompilation (#53972) In the parallel package precompilation code we are mapping what packages depend on what other packages so that we precompile things in the correct order ("bottom up") and so what we can also detect cycles and avoid precompiling packages in those cycles. However, we fail to detect one type of dependency which is that an extension can "indirectly" depend on another extension. This happens when the transitive dependencies of the extension (it's parent + triggers) are a superset of the dependencies of another extension. In other words, this means that the other extension will get loaded into the first extension once it gets loaded, effectively being a dependency. The failure to model this leads to some issues, for example using one of the examples in our own tests: ```julia julia> Base.active_project() "/home/kc/julia/test/project/Extensions/HasDepWithExtensions.jl/Project.toml" (HasDepWithExtensions) pkg> status --extensions Project HasDepWithExtensions v0.1.0 Status `~/julia/test/project/Extensions/HasDepWithExtensions.jl/Project.toml` [4d3288b3] HasExtensions v0.1.0 `../HasExtensions.jl` ├─ ExtensionFolder [ExtDep, ExtDep2] ├─ Extension [ExtDep] └─ LinearAlgebraExt [LinearAlgebra] julia> Base.Precompilation.precompilepkgs(; timing=true) Precompiling all packages... 196.1 ms ✓ HasExtensions 244.4 ms ✓ ExtDep2 207.9 ms ✓ SomePackage 201.6 ms ✓ ExtDep 462.5 ms ✓ HasExtensions → ExtensionFolder 200.1 ms ✓ HasExtensions → Extension 173.1 ms ✓ HasExtensions → LinearAlgebraExt 222.2 ms ✓ HasDepWithExtensions 8 dependencies successfully precompiled in 2 seconds julia> Base.Precompilation.precompilepkgs(; timing=true) Precompiling all packages... 213.4 ms ✓ HasExtensions → ExtensionFolder 1 dependency successfully precompiled in 0 seconds. 7 already precompiled. julia> Base.Precompilation.precompilepkgs(; timing=true) julia> ``` We can see here that `ExtensionFolder` gets precompiled twice which is due to `Extension` actually being an "indirect dependency" of `ExtensionFolder` and therefore should be precompiled before it. With this PR we instead get: ```julia julia> Precompilation.precompilepkgs(; timing=true) Precompiling all packages... 347.5 ms ✓ ExtDep2 294.0 ms ✓ SomePackage 293.2 ms ✓ HasExtensions 216.5 ms ✓ HasExtensions → LinearAlgebraExt 554.9 ms ✓ ExtDep 580.9 ms ✓ HasExtensions → Extension 593.8 ms ✓ HasExtensions → ExtensionFolder 261.3 ms ✓ HasDepWithExtensions 8 dependencies successfully precompiled in 2 seconds julia> Precompilation.precompilepkgs(; timing=true) julia> ``` `Extension` is precompiled after `ExtensionFolder` and nothing happens on the second call. Also, with this PR we get for the issue in #53081 (comment): ```julia (jl_zuuRGt) pkg> st Status `/private/var/folders/tp/2p4x9ygx48sgsdl1ccg1mp_40000gn/T/jl_zuuRGt/Project.toml` ⌃ [d236fae5] PreallocationTools v0.4.17 ⌃ [0c5d862f] Symbolics v5.16.1 Info Packages marked with ⌃ have new versions available and may be upgradable. julia> Precompilation.precompilepkgs(; timing=true) ┌ Warning: Circular dependency detected. Precompilation will be skipped for: │ SymbolicsPreallocationToolsExt │ SymbolicsForwardDiffExt ``` and we avoid precompiling the problematic extensions. This should also allow extensions to precompile in parallel which I think was prevented before (from the code that is removed in the beginning of the diff). (cherry picked from commit df89440)
KristofferC
removed
the
backport 1.11
lazarusA
pushed a commit
to lazarusA/julia
that referenced
this pull request
Jul 12, 2024
… package precompilation (JuliaLang#53972) In the parallel package precompilation code we are mapping what packages depend on what other packages so that we precompile things in the correct order ("bottom up") and so what we can also detect cycles and avoid precompiling packages in those cycles. However, we fail to detect one type of dependency which is that an extension can "indirectly" depend on another extension. This happens when the transitive dependencies of the extension (it's parent + triggers) are a superset of the dependencies of another extension. In other words, this means that the other extension will get loaded into the first extension once it gets loaded, effectively being a dependency. The failure to model this leads to some issues, for example using one of the examples in our own tests: ```julia julia> Base.active_project() "/home/kc/julia/test/project/Extensions/HasDepWithExtensions.jl/Project.toml" (HasDepWithExtensions) pkg> status --extensions Project HasDepWithExtensions v0.1.0 Status `~/julia/test/project/Extensions/HasDepWithExtensions.jl/Project.toml` [4d3288b3] HasExtensions v0.1.0 `../HasExtensions.jl` ├─ ExtensionFolder [ExtDep, ExtDep2] ├─ Extension [ExtDep] └─ LinearAlgebraExt [LinearAlgebra] julia> Base.Precompilation.precompilepkgs(; timing=true) Precompiling all packages... 196.1 ms ✓ HasExtensions 244.4 ms ✓ ExtDep2 207.9 ms ✓ SomePackage 201.6 ms ✓ ExtDep 462.5 ms ✓ HasExtensions → ExtensionFolder 200.1 ms ✓ HasExtensions → Extension 173.1 ms ✓ HasExtensions → LinearAlgebraExt 222.2 ms ✓ HasDepWithExtensions 8 dependencies successfully precompiled in 2 seconds julia> Base.Precompilation.precompilepkgs(; timing=true) Precompiling all packages... 213.4 ms ✓ HasExtensions → ExtensionFolder 1 dependency successfully precompiled in 0 seconds. 7 already precompiled. julia> Base.Precompilation.precompilepkgs(; timing=true) julia> ``` We can see here that `ExtensionFolder` gets precompiled twice which is due to `Extension` actually being an "indirect dependency" of `ExtensionFolder` and therefore should be precompiled before it. With this PR we instead get: ```julia julia> Precompilation.precompilepkgs(; timing=true) Precompiling all packages... 347.5 ms ✓ ExtDep2 294.0 ms ✓ SomePackage 293.2 ms ✓ HasExtensions 216.5 ms ✓ HasExtensions → LinearAlgebraExt 554.9 ms ✓ ExtDep 580.9 ms ✓ HasExtensions → Extension 593.8 ms ✓ HasExtensions → ExtensionFolder 261.3 ms ✓ HasDepWithExtensions 8 dependencies successfully precompiled in 2 seconds julia> Precompilation.precompilepkgs(; timing=true) julia> ``` `Extension` is precompiled after `ExtensionFolder` and nothing happens on the second call. Also, with this PR we get for the issue in JuliaLang#53081 (comment): ```julia (jl_zuuRGt) pkg> st Status `/private/var/folders/tp/2p4x9ygx48sgsdl1ccg1mp_40000gn/T/jl_zuuRGt/Project.toml` ⌃ [d236fae5] PreallocationTools v0.4.17 ⌃ [0c5d862f] Symbolics v5.16.1 Info Packages marked with ⌃ have new versions available and may be upgradable. julia> Precompilation.precompilepkgs(; timing=true) ┌ Warning: Circular dependency detected. Precompilation will be skipped for: │ SymbolicsPreallocationToolsExt │ SymbolicsForwardDiffExt ``` and we avoid precompiling the problematic extensions. This should also allow extensions to precompile in parallel which I think was prevented before (from the code that is removed in the beginning of the diff).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the parallel package precompilation code we are mapping what packages depend on what other packages so that we precompile things in the correct order ("bottom up") and so what we can also detect cycles and avoid precompiling packages in those cycles. However, we fail to detect one type of dependency which is that an extension can "indirectly" depend on another extension. This happens when the transitive dependencies of the extension (it's parent + triggers) are a superset of the dependencies of another extension. In other words, this means that the other extension will get loaded into the first extension once it gets loaded, effectively being a dependency.
The failure to model this leads to some issues, for example using one of the examples in our own tests:
We can see here that
ExtensionFoldergets precompiled twice which is due toExtensionactually being an "indirect dependency" ofExtensionFolderand therefore should be precompiled before it.With this PR we instead get:
Extensionis precompiled afterExtensionFolderand nothing happens on the second call.Also, with this PR we get for the issue in #53081 (comment):
and we avoid precompiling the problematic extensions.
This should also allow extensions to precompile in parallel which I think was prevented before (from the code that is removed in the beginning of the diff).
WIP: Because I need to think about this more and test it more etc.