Skip to content

Commit 6c5f221

Browse files
authored
Prevent extensions from blocking parallel pre-compilation (JuliaLang#55910)
Previously our precompilation code was causing anything with package A as a dependency to wait on all of A's extensions and weakdeps to finish before starting to pre-compile, even if it can't actually load those weakdeps (or the extensions themselves) This would lead to a pre-compile ordering like: ``` A B \ / \ Ext AB \ | / C / \ / D ``` Here `C` cannot pre-compile in parallel with `Ext {A,B}` and `B`, because it has to wait for `Ext {A,B}` to finish pre-compiling. That happens even though `C` has no way to load either of these. This change updates the pre-compile ordering to be more parallel, reflecting the true place where `Ext {A,B}` can be loaded: ``` A B / \ / \ C Ext AB | \ | / \-- D --/ ``` which allows `C` to compile in parallel with `B` and `Ext{A,B}`
1 parent d9d1fc5 commit 6c5f221

File tree

1 file changed

+97
-60
lines changed

1 file changed

+97
-60
lines changed

base/precompilation.jl

+97-60
Original file line numberDiff line numberDiff line change
@@ -417,13 +417,16 @@ function _precompilepkgs(pkgs::Vector{String},
417417
stale_cache = Dict{StaleCacheKey, Bool}()
418418
cachepath_cache = Dict{PkgId, Vector{String}}()
419419

420-
exts = Dict{PkgId, String}() # ext -> parent
421-
# make a flat map of each dep and its direct deps
422-
depsmap = Dict{PkgId, Vector{PkgId}}()
423-
pkg_exts_map = Dict{PkgId, Vector{PkgId}}()
420+
# a map from packages/extensions to their direct deps
421+
direct_deps = Dict{Base.PkgId, Vector{Base.PkgId}}()
422+
# a map from parent → extension, including all extensions that are loadable
423+
# in the current environment (i.e. their triggers are present)
424+
parent_to_exts = Dict{Base.PkgId, Vector{Base.PkgId}}()
425+
# inverse map of `parent_to_ext` above (ext → parent)
426+
ext_to_parent = Dict{Base.PkgId, Base.PkgId}()
424427

425428
function describe_pkg(pkg::PkgId, is_direct_dep::Bool, flags::Cmd, cacheflags::Base.CacheFlags)
426-
name = haskey(exts, pkg) ? string(exts[pkg], "", pkg.name) : pkg.name
429+
name = haskey(ext_to_parent, pkg) ? string(ext_to_parent[pkg].name, "", pkg.name) : pkg.name
427430
name = is_direct_dep ? name : color_string(name, :light_black)
428431
if nconfigs > 1 && !isempty(flags)
429432
config_str = join(flags, " ")
@@ -441,66 +444,101 @@ function _precompilepkgs(pkgs::Vector{String},
441444
pkg = Base.PkgId(dep, env.names[dep])
442445
Base.in_sysimage(pkg) && continue
443446
deps = [Base.PkgId(x, env.names[x]) for x in deps]
444-
depsmap[pkg] = filter!(!Base.in_sysimage, deps)
445-
# add any extensions
446-
pkg_exts = Dict{Base.PkgId, Vector{Base.PkgId}}()
447-
for (ext_name, extdep_uuids) in env.extensions[dep]
447+
direct_deps[pkg] = filter!(!Base.in_sysimage, deps)
448+
for (ext_name, trigger_uuids) in env.extensions[dep]
448449
ext_uuid = Base.uuid5(pkg.uuid, ext_name)
449450
ext = Base.PkgId(ext_uuid, ext_name)
450451
triggers[ext] = Base.PkgId[pkg] # depends on parent package
451-
all_extdeps_available = true
452-
for extdep_uuid in extdep_uuids
453-
extdep_name = env.names[extdep_uuid]
454-
if extdep_uuid in keys(env.deps)
455-
push!(triggers[ext], Base.PkgId(extdep_uuid, extdep_name))
452+
all_triggers_available = true
453+
for trigger_uuid in trigger_uuids
454+
trigger_name = env.names[trigger_uuid]
455+
if trigger_uuid in keys(env.deps)
456+
push!(triggers[ext], Base.PkgId(trigger_uuid, trigger_name))
456457
else
457-
all_extdeps_available = false
458+
all_triggers_available = false
458459
break
459460
end
460461
end
461-
all_extdeps_available || continue
462-
exts[ext] = pkg.name
463-
pkg_exts[ext] = depsmap[ext] = filter(!Base.in_sysimage, triggers[ext])
464-
end
465-
if !isempty(pkg_exts)
466-
pkg_exts_map[pkg] = collect(keys(pkg_exts))
462+
all_triggers_available || continue
463+
ext_to_parent[ext] = pkg
464+
direct_deps[ext] = filter(!Base.in_sysimage, triggers[ext])
465+
466+
if !haskey(parent_to_exts, pkg)
467+
parent_to_exts[pkg] = Base.PkgId[ext]
468+
else
469+
push!(parent_to_exts[pkg], ext)
470+
end
467471
end
468472
end
469473

470-
direct_deps = [
474+
project_deps = [
471475
Base.PkgId(uuid, name)
472476
for (name, uuid) in env.project_deps if !Base.in_sysimage(Base.PkgId(uuid, name))
473477
]
474478

475-
# consider exts of direct deps to be direct deps so that errors are reported
476-
append!(direct_deps, keys(filter(d->last(d) in keys(env.project_deps), exts)))
479+
# consider exts of project deps to be project deps so that errors are reported
480+
append!(project_deps, keys(filter(d->last(d).name in keys(env.project_deps), ext_to_parent)))
477481

478482
@debug "precompile: deps collected"
479483

480484
# An extension effectively depends on another extension if it has a strict superset of its triggers
481-
for ext_a in keys(exts)
482-
for ext_b in keys(exts)
485+
for ext_a in keys(ext_to_parent)
486+
for ext_b in keys(ext_to_parent)
483487
if triggers[ext_a] triggers[ext_b]
484-
push!(depsmap[ext_a], ext_b)
488+
push!(direct_deps[ext_a], ext_b)
485489
end
486490
end
487491
end
488492

489-
# this loop must be run after the full depsmap has been populated
490-
for (pkg, pkg_exts) in pkg_exts_map
491-
# find any packages that depend on the extension(s)'s deps and replace those deps in their deps list with the extension(s),
492-
# basically injecting the extension into the precompile order in the graph, to avoid race to precompile extensions
493-
for (_pkg, deps) in depsmap # for each manifest dep
494-
if !in(_pkg, keys(exts)) && pkg in deps # if not an extension and depends on pkg
495-
append!(deps, pkg_exts) # add the package extensions to deps
496-
filter!(!isequal(pkg), deps) # remove the pkg from deps
493+
# A package depends on an extension if it (indirectly) depends on all extension triggers
494+
function expand_indirect_dependencies(direct_deps)
495+
function visit!(visited, node, all_deps)
496+
if node in visited
497+
return
498+
end
499+
push!(visited, node)
500+
for dep in get(Set{Base.PkgId}, direct_deps, node)
501+
if !(dep in all_deps)
502+
push!(all_deps, dep)
503+
visit!(visited, dep, all_deps)
504+
end
505+
end
506+
end
507+
508+
indirect_deps = Dict{Base.PkgId, Set{Base.PkgId}}()
509+
for package in keys(direct_deps)
510+
# Initialize a set to keep track of all dependencies for 'package'
511+
all_deps = Set{Base.PkgId}()
512+
visited = Set{Base.PkgId}()
513+
visit!(visited, package, all_deps)
514+
# Update direct_deps with the complete set of dependencies for 'package'
515+
indirect_deps[package] = all_deps
516+
end
517+
return indirect_deps
518+
end
519+
520+
# this loop must be run after the full direct_deps map has been populated
521+
indirect_deps = expand_indirect_dependencies(direct_deps)
522+
for ext in keys(ext_to_parent)
523+
ext_loadable_in_pkg = Dict{Base.PkgId,Bool}()
524+
for pkg in keys(direct_deps)
525+
is_trigger = in(pkg, direct_deps[ext])
526+
is_extension = in(pkg, keys(ext_to_parent))
527+
has_triggers = issubset(direct_deps[ext], indirect_deps[pkg])
528+
ext_loadable_in_pkg[pkg] = !is_extension && has_triggers && !is_trigger
529+
end
530+
for (pkg, ext_loadable) in ext_loadable_in_pkg
531+
if ext_loadable && !any((dep)->ext_loadable_in_pkg[dep], direct_deps[pkg])
532+
# add an edge if the extension is loadable by pkg, and was not loadable in any
533+
# of the pkg's dependencies
534+
push!(direct_deps[pkg], ext)
497535
end
498536
end
499537
end
500538
@debug "precompile: extensions collected"
501539

502540
# return early if no deps
503-
if isempty(depsmap)
541+
if isempty(direct_deps)
504542
if isempty(pkgs)
505543
return
506544
elseif _from_loading
@@ -518,7 +556,7 @@ function _precompilepkgs(pkgs::Vector{String},
518556
was_processed = Dict{PkgConfig,Base.Event}()
519557
was_recompiled = Dict{PkgConfig,Bool}()
520558
for config in configs
521-
for pkgid in keys(depsmap)
559+
for pkgid in keys(direct_deps)
522560
pkg_config = (pkgid, config)
523561
started[pkg_config] = false
524562
was_processed[pkg_config] = Base.Event()
@@ -527,7 +565,6 @@ function _precompilepkgs(pkgs::Vector{String},
527565
end
528566
@debug "precompile: signalling initialized"
529567

530-
531568
# find and guard against circular deps
532569
circular_deps = Base.PkgId[]
533570
# Three states
@@ -554,8 +591,8 @@ function _precompilepkgs(pkgs::Vector{String},
554591
could_be_cycle[pkg] = false
555592
return false
556593
end
557-
for pkg in keys(depsmap)
558-
if scan_pkg!(pkg, depsmap)
594+
for pkg in keys(direct_deps)
595+
if scan_pkg!(pkg, direct_deps)
559596
push!(circular_deps, pkg)
560597
for pkg_config in keys(was_processed)
561598
# notify all to allow skipping
@@ -570,33 +607,33 @@ function _precompilepkgs(pkgs::Vector{String},
570607

571608
if !manifest
572609
if isempty(pkgs)
573-
pkgs = [pkg.name for pkg in direct_deps]
610+
pkgs = [pkg.name for pkg in project_deps]
574611
end
575612
# restrict to dependencies of given packages
576-
function collect_all_deps(depsmap, dep, alldeps=Set{Base.PkgId}())
577-
for _dep in depsmap[dep]
613+
function collect_all_deps(direct_deps, dep, alldeps=Set{Base.PkgId}())
614+
for _dep in direct_deps[dep]
578615
if !(_dep in alldeps)
579616
push!(alldeps, _dep)
580-
collect_all_deps(depsmap, _dep, alldeps)
617+
collect_all_deps(direct_deps, _dep, alldeps)
581618
end
582619
end
583620
return alldeps
584621
end
585622
keep = Set{Base.PkgId}()
586-
for dep in depsmap
623+
for dep in direct_deps
587624
dep_pkgid = first(dep)
588625
if dep_pkgid.name in pkgs
589626
push!(keep, dep_pkgid)
590-
collect_all_deps(depsmap, dep_pkgid, keep)
627+
collect_all_deps(direct_deps, dep_pkgid, keep)
591628
end
592629
end
593-
for ext in keys(exts)
594-
if issubset(collect_all_deps(depsmap, ext), keep) # if all extension deps are kept
630+
for ext in keys(ext_to_parent)
631+
if issubset(collect_all_deps(direct_deps, ext), keep) # if all extension deps are kept
595632
push!(keep, ext)
596633
end
597634
end
598-
filter!(d->in(first(d), keep), depsmap)
599-
if isempty(depsmap)
635+
filter!(d->in(first(d), keep), direct_deps)
636+
if isempty(direct_deps)
600637
if _from_loading
601638
# if called from loading precompilation it may be a package from another environment stack so
602639
# don't error and allow serial precompilation to try
@@ -709,7 +746,7 @@ function _precompilepkgs(pkgs::Vector{String},
709746
i = 1
710747
last_length = 0
711748
bar = MiniProgressBar(; indent=0, header = "Precompiling packages ", color = :green, percentage=false, always_reprint=true)
712-
n_total = length(depsmap) * length(configs)
749+
n_total = length(direct_deps) * length(configs)
713750
bar.max = n_total - n_already_precomp
714751
final_loop = false
715752
n_print_rows = 0
@@ -739,7 +776,7 @@ function _precompilepkgs(pkgs::Vector{String},
739776
dep, config = pkg_config
740777
loaded = warn_loaded && haskey(Base.loaded_modules, dep)
741778
flags, cacheflags = config
742-
name = describe_pkg(dep, dep in direct_deps, flags, cacheflags)
779+
name = describe_pkg(dep, dep in project_deps, flags, cacheflags)
743780
line = if pkg_config in precomperr_deps
744781
string(color_string(" ? ", Base.warn_color()), name)
745782
elseif haskey(failed_deps, pkg_config)
@@ -755,7 +792,7 @@ function _precompilepkgs(pkgs::Vector{String},
755792
# Offset each spinner animation using the first character in the package name as the seed.
756793
# If not offset, on larger terminal fonts it looks odd that they all sync-up
757794
anim_char = anim_chars[(i + Int(dep.name[1])) % length(anim_chars) + 1]
758-
anim_char_colored = dep in direct_deps ? anim_char : color_string(anim_char, :light_black)
795+
anim_char_colored = dep in project_deps ? anim_char : color_string(anim_char, :light_black)
759796
waiting = if haskey(pkgspidlocked, pkg_config)
760797
who_has_lock = pkgspidlocked[pkg_config]
761798
color_string(" Being precompiled by $(who_has_lock)", Base.info_color())
@@ -791,10 +828,10 @@ function _precompilepkgs(pkgs::Vector{String},
791828
if !_from_loading
792829
Base.LOADING_CACHE[] = Base.LoadingCache()
793830
end
794-
@debug "precompile: starting precompilation loop" depsmap direct_deps
831+
@debug "precompile: starting precompilation loop" direct_deps project_deps
795832
## precompilation loop
796833

797-
for (pkg, deps) in depsmap
834+
for (pkg, deps) in direct_deps
798835
cachepaths = get!(() -> Base.find_all_in_cache_path(pkg), cachepath_cache, pkg)
799836
sourcepath = Base.locate_package(pkg)
800837
single_requested_pkg = length(requested_pkgs) == 1 && only(requested_pkgs) == pkg.name
@@ -821,13 +858,13 @@ function _precompilepkgs(pkgs::Vector{String},
821858
is_stale = !Base.isprecompiled(pkg; ignore_loaded=true, stale_cache, cachepath_cache, cachepaths, sourcepath, flags=cacheflags)
822859
if !circular && is_stale
823860
Base.acquire(parallel_limiter)
824-
is_direct_dep = pkg in direct_deps
861+
is_project_dep = pkg in project_deps
825862

826863
# std monitoring
827864
std_pipe = Base.link_pipe!(Pipe(); reader_supports_async=true, writer_supports_async=true)
828865
t_monitor = @async monitor_std(pkg_config, std_pipe; single_requested_pkg)
829866

830-
name = describe_pkg(pkg, is_direct_dep, flags, cacheflags)
867+
name = describe_pkg(pkg, is_project_dep, flags, cacheflags)
831868
lock(print_lock) do
832869
if !fancyprint && isempty(pkg_queue)
833870
printpkgstyle(io, :Precompiling, something(target, "packages..."))
@@ -850,7 +887,7 @@ function _precompilepkgs(pkgs::Vector{String},
850887
keep_loaded_modules = false
851888
# for extensions, any extension in our direct dependencies is one we have a right to load
852889
# for packages, we may load any extension (all possible triggers are accounted for above)
853-
loadable_exts = haskey(exts, pkg) ? filter((dep)->haskey(exts, dep), depsmap[pkg]) : nothing
890+
loadable_exts = haskey(ext_to_parent, pkg) ? filter((dep)->haskey(ext_to_parent, dep), direct_deps[pkg]) : nothing
854891
Base.compilecache(pkg, sourcepath, std_pipe, std_pipe, keep_loaded_modules;
855892
flags, cacheflags, loadable_exts)
856893
end
@@ -965,7 +1002,7 @@ function _precompilepkgs(pkgs::Vector{String},
9651002
else
9661003
join(split(err, "\n"), color_string("\n", Base.warn_color()))
9671004
end
968-
name = haskey(exts, pkg) ? string(exts[pkg], "", pkg.name) : pkg.name
1005+
name = haskey(ext_to_parent, pkg) ? string(ext_to_parent[pkg].name, "", pkg.name) : pkg.name
9691006
print(iostr, color_string("\n", Base.warn_color()), name, color_string("\n", Base.warn_color()), err, color_string("\n", Base.warn_color()))
9701007
end
9711008
end
@@ -981,7 +1018,7 @@ function _precompilepkgs(pkgs::Vector{String},
9811018
n_direct_errs = 0
9821019
for (pkg_config, err) in failed_deps
9831020
dep, config = pkg_config
984-
if strict || (dep in direct_deps)
1021+
if strict || (dep in project_deps)
9851022
print(err_str, "\n", dep.name, " ")
9861023
for cfg in config[1]
9871024
print(err_str, cfg, " ")

0 commit comments

Comments
 (0)