Skip to content

Commit 394f0af

Browse files
KristofferCKristofferC
authored andcommitted
account for extensions indirectly depending on each other in parallel 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)
1 parent af49d9f commit 394f0af

File tree

1 file changed

+49
-7
lines changed

1 file changed

+49
-7
lines changed

base/precompilation.jl

Lines changed: 49 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,6 @@ function precompilepkgs(pkgs::Vector{String}=String[];
399399
depsmap[pkg] = filter!(!Base.in_sysimage, deps)
400400
# add any extensions
401401
pkg_exts = Dict{Base.PkgId, Vector{Base.PkgId}}()
402-
prev_ext = nothing
403402
for (ext_name, extdep_uuids) in env.extensions[dep]
404403
ext_deps = Base.PkgId[]
405404
push!(ext_deps, pkg) # depends on parent package
@@ -414,13 +413,8 @@ function precompilepkgs(pkgs::Vector{String}=String[];
414413
end
415414
end
416415
all_extdeps_available || continue
417-
if prev_ext isa Base.PkgId
418-
# also make the exts depend on eachother sequentially to avoid race
419-
push!(ext_deps, prev_ext)
420-
end
421416
ext_uuid = Base.uuid5(pkg.uuid, ext_name)
422417
ext = Base.PkgId(ext_uuid, ext_name)
423-
prev_ext = ext
424418
filter!(!Base.in_sysimage, ext_deps)
425419
depsmap[ext] = ext_deps
426420
exts[ext] = pkg.name
@@ -431,6 +425,51 @@ function precompilepkgs(pkgs::Vector{String}=String[];
431425
end
432426
end
433427

428+
# An extension effectively depends on another extension if it has all the the
429+
# dependencies of that other extension
430+
function expand_dependencies(depsmap)
431+
function visit!(visited, node, all_deps)
432+
if node in visited
433+
return
434+
end
435+
push!(visited, node)
436+
for dep in get(Set{Base.PkgId}, depsmap, node)
437+
if !(dep in all_deps)
438+
push!(all_deps, dep)
439+
visit!(visited, dep, all_deps)
440+
end
441+
end
442+
end
443+
444+
depsmap_transitive = Dict{Base.PkgId, Set{Base.PkgId}}()
445+
for package in keys(depsmap)
446+
# Initialize a set to keep track of all dependencies for 'package'
447+
all_deps = Set{Base.PkgId}()
448+
visited = Set{Base.PkgId}()
449+
visit!(visited, package, all_deps)
450+
# Update depsmap with the complete set of dependencies for 'package'
451+
depsmap_transitive[package] = all_deps
452+
end
453+
return depsmap_transitive
454+
end
455+
456+
depsmap_transitive = expand_dependencies(depsmap)
457+
458+
for (_, extensions_1) in pkg_exts_map
459+
for extension_1 in extensions_1
460+
deps_ext_1 = depsmap_transitive[extension_1]
461+
for (_, extensions_2) in pkg_exts_map
462+
for extension_2 in extensions_2
463+
extension_1 == extension_2 && continue
464+
deps_ext_2 = depsmap_transitive[extension_2]
465+
if issubset(deps_ext_2, deps_ext_1)
466+
push!(depsmap[extension_1], extension_2)
467+
end
468+
end
469+
end
470+
end
471+
end
472+
434473
@debug "precompile: deps collected"
435474
# this loop must be run after the full depsmap has been populated
436475
for (pkg, pkg_exts) in pkg_exts_map
@@ -739,6 +778,7 @@ function precompilepkgs(pkgs::Vector{String}=String[];
739778
end
740779
@debug "precompile: starting precompilation loop" depsmap direct_deps
741780
## precompilation loop
781+
742782
for (pkg, deps) in depsmap
743783
cachepaths = Base.find_all_in_cache_path(pkg)
744784
sourcepath = Base.locate_package(pkg)
@@ -811,6 +851,7 @@ function precompilepkgs(pkgs::Vector{String}=String[];
811851
end
812852
loaded && (n_loaded += 1)
813853
catch err
854+
# @show err
814855
close(std_pipe.in) # close pipe to end the std output monitor
815856
wait(t_monitor)
816857
if err isa ErrorException || (err isa ArgumentError && startswith(err.msg, "Invalid header in cache file"))
@@ -834,7 +875,8 @@ function precompilepkgs(pkgs::Vector{String}=String[];
834875
notify(was_processed[pkg_config])
835876
catch err_outer
836877
# For debugging:
837-
# println("Task failed $err_outer") # logging doesn't show here
878+
# println("Task failed $err_outer")
879+
# Base.display_error(ErrorException(""), Base.catch_backtrace())# logging doesn't show here
838880
handle_interrupt(err_outer) || rethrow()
839881
notify(was_processed[pkg_config])
840882
finally

0 commit comments

Comments
 (0)