Skip to content

Commit 330804a

Browse files
vtjnashKristofferC
authored andcommitted
remove log spamming from precompilepkg code (#59652)
Improves consistency between serial and parallel code and reduces useless log spamming. Logging messages should convey useful content actionable in some form by the user. Neither non-interactive mode nor simple progress meters achieve either goal. Fix #59527 (cherry picked from commit 42a33bb)
1 parent 1f8bc09 commit 330804a

File tree

3 files changed

+55
-45
lines changed

3 files changed

+55
-45
lines changed

base/loading.jl

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2622,33 +2622,37 @@ function __require_prelocked(pkg::PkgId, env)
26222622
if JLOptions().use_compiled_modules == 1
26232623
if !generating_output(#=incremental=#false)
26242624
project = active_project()
2625-
if !generating_output() && !parallel_precompile_attempted && !disable_parallel_precompile && @isdefined(Precompilation)
2626-
parallel_precompile_attempted = true
2627-
unlock(require_lock)
2628-
try
2629-
Precompilation.precompilepkgs([pkg]; _from_loading=true, ignore_loaded=false)
2630-
finally
2631-
lock(require_lock)
2632-
end
2633-
@goto load_from_cache
2634-
end
26352625
# spawn off a new incremental pre-compile task for recursive `require` calls
26362626
loaded = let path = path, reasons = reasons
26372627
maybe_cachefile_lock(pkg, path) do
26382628
# double-check the search now that we have lock
26392629
m = _require_search_from_serialized(pkg, path, UInt128(0), true)
26402630
m isa Module && return m
2641-
triggers = get(EXT_PRIMED, pkg, nothing)
2642-
loadable_exts = nothing
2643-
if triggers !== nothing # extension
2644-
loadable_exts = PkgId[]
2645-
for (ext′, triggers′) in EXT_PRIMED
2646-
if triggers′ triggers
2647-
push!(loadable_exts, ext′)
2631+
2632+
verbosity = isinteractive() ? CoreLogging.Info : CoreLogging.Debug
2633+
@logmsg verbosity "Precompiling $(repr("text/plain", pkg))$(list_reasons(reasons))"
2634+
2635+
unlock(require_lock)
2636+
try
2637+
if !generating_output() && !parallel_precompile_attempted && !disable_parallel_precompile && @isdefined(Precompilation)
2638+
parallel_precompile_attempted = true
2639+
Precompilation.precompilepkgs([pkg]; _from_loading=true, ignore_loaded=false)
2640+
return
2641+
end
2642+
triggers = get(EXT_PRIMED, pkg, nothing)
2643+
loadable_exts = nothing
2644+
if triggers !== nothing # extension
2645+
loadable_exts = PkgId[]
2646+
for (ext′, triggers′) in EXT_PRIMED
2647+
if triggers′ triggers
2648+
push!(loadable_exts, ext′)
2649+
end
26482650
end
26492651
end
2652+
return compilecache(pkg, path; loadable_exts)
2653+
finally
2654+
lock(require_lock)
26502655
end
2651-
return compilecache(pkg, path; reasons, loadable_exts)
26522656
end
26532657
end
26542658
loaded isa Module && return loaded
@@ -3149,18 +3153,18 @@ This can be used to reduce package load times. Cache files are stored in
31493153
`DEPOT_PATH[1]/compiled`. See [Module initialization and precompilation](@ref)
31503154
for important notes.
31513155
"""
3152-
function compilecache(pkg::PkgId, internal_stderr::IO = stderr, internal_stdout::IO = stdout; flags::Cmd=``, cacheflags::CacheFlags=CacheFlags(), reasons::Union{Dict{String,Int},Nothing}=Dict{String,Int}(), loadable_exts::Union{Vector{PkgId},Nothing}=nothing)
3156+
function compilecache(pkg::PkgId, internal_stderr::IO = stderr, internal_stdout::IO = stdout; flags::Cmd=``, cacheflags::CacheFlags=CacheFlags(), loadable_exts::Union{Vector{PkgId},Nothing}=nothing)
31533157
@nospecialize internal_stderr internal_stdout
31543158
path = locate_package(pkg)
31553159
path === nothing && throw(ArgumentError("$(repr("text/plain", pkg)) not found during precompilation"))
3156-
return compilecache(pkg, path, internal_stderr, internal_stdout; flags, cacheflags, reasons, loadable_exts)
3160+
return compilecache(pkg, path, internal_stderr, internal_stdout; flags, cacheflags, loadable_exts)
31573161
end
31583162

31593163
const MAX_NUM_PRECOMPILE_FILES = Ref(10)
31603164

31613165
function compilecache(pkg::PkgId, path::String, internal_stderr::IO = stderr, internal_stdout::IO = stdout,
31623166
keep_loaded_modules::Bool = true; flags::Cmd=``, cacheflags::CacheFlags=CacheFlags(),
3163-
reasons::Union{Dict{String,Int},Nothing}=Dict{String,Int}(), loadable_exts::Union{Vector{PkgId},Nothing}=nothing)
3167+
loadable_exts::Union{Vector{PkgId},Nothing}=nothing)
31643168

31653169
@nospecialize internal_stderr internal_stdout
31663170
# decide where to put the resulting cache file
@@ -3178,8 +3182,6 @@ function compilecache(pkg::PkgId, path::String, internal_stderr::IO = stderr, in
31783182
concrete_deps = empty(_concrete_dependencies)
31793183
end
31803184
# run the expression and cache the result
3181-
verbosity = isinteractive() ? CoreLogging.Info : CoreLogging.Debug
3182-
@logmsg verbosity "Precompiling $(repr("text/plain", pkg)) $(list_reasons(reasons))"
31833185

31843186
# create a temporary file in `cachepath` directory, write the cache in it,
31853187
# write the checksum, _and then_ atomically move the file to `cachefile`.
@@ -3877,7 +3879,7 @@ end
38773879
record_reason(::Nothing, ::String) = nothing
38783880
function list_reasons(reasons::Dict{String,Int})
38793881
isempty(reasons) && return ""
3880-
return "(cache misses: $(join(("$k ($v)" for (k,v) in reasons), ", ")))"
3882+
return " (cache misses: $(join(("$k ($v)" for (k,v) in reasons), ", ")))"
38813883
end
38823884
list_reasons(::Nothing) = ""
38833885

base/precompilation.jl

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
module Precompilation
22

3-
using Base: PkgId, UUID, SHA1, parsed_toml, project_file_name_uuid, project_names,
3+
using Base: CoreLogging, PkgId, UUID, SHA1, parsed_toml, project_file_name_uuid, project_names,
44
project_file_manifest_path, get_deps, preferences_names, isaccessibledir, isfile_casesensitive,
55
base_project, isdefined
66

@@ -532,9 +532,16 @@ function _precompilepkgs(pkgs::Union{Vector{String}, Vector{PkgId}},
532532
num_tasks = parse(Int, get(ENV, "JULIA_NUM_PRECOMPILE_TASKS", string(default_num_tasks)))
533533
parallel_limiter = Base.Semaphore(num_tasks)
534534

535-
# suppress passive loading printing in julia test suite. `JULIA_TESTS` is set in Base.runtests
536-
io = (_from_loading && !Sys.isinteractive() && Base.get_bool_env("JULIA_TESTS", false)) ? IOContext{IO}(devnull) : _io
537-
535+
# suppress precompilation progress messages when precompiling for loading packages, except during interactive sessions
536+
# or when specified by logging heuristics that explicitly require it
537+
# since the complicated IO implemented here can have somewhat disastrous consequences when happening in the background (e.g. #59599)
538+
io = _io
539+
logcalls = nothing
540+
if _from_loading && !isinteractive()
541+
io = IOContext{IO}(devnull)
542+
fancyprint = false
543+
logcalls = isinteractive() ? CoreLogging.Info : CoreLogging.Debug # sync with Base.compilecache
544+
end
538545

539546
nconfigs = length(configs)
540547
hascolor = get(io, :color, false)::Bool
@@ -608,8 +615,6 @@ function _precompilepkgs(pkgs::Union{Vector{String}, Vector{PkgId}},
608615
# consider exts of project deps to be project deps so that errors are reported
609616
append!(project_deps, keys(filter(d->last(d).name in keys(env.project_deps), ext_to_parent)))
610617

611-
@debug "precompile: deps collected"
612-
613618
# An extension effectively depends on another extension if it has a strict superset of its triggers
614619
for ext_a in keys(ext_to_parent)
615620
for ext_b in keys(ext_to_parent)
@@ -664,7 +669,6 @@ function _precompilepkgs(pkgs::Union{Vector{String}, Vector{PkgId}},
664669
end
665670
end
666671
end
667-
@debug "precompile: extensions collected"
668672

669673
serial_deps = Base.PkgId[] # packages that are being precompiled in serial
670674

@@ -704,7 +708,6 @@ function _precompilepkgs(pkgs::Union{Vector{String}, Vector{PkgId}},
704708
was_recompiled[pkg_config] = false
705709
end
706710
end
707-
@debug "precompile: signalling initialized"
708711

709712
# find and guard against circular deps
710713
cycles = Vector{Base.PkgId}[]
@@ -732,7 +735,6 @@ function _precompilepkgs(pkgs::Union{Vector{String}, Vector{PkgId}},
732735
if !isempty(circular_deps)
733736
@warn excluded_circular_deps_explanation(io, ext_to_parent, circular_deps, cycles)
734737
end
735-
@debug "precompile: circular dep check done"
736738

737739
# If you have a workspace and want to precompile all projects in it, look through all packages in the manifest
738740
# instead of collecting from a project i.e. not filter out packages that are in the current project.
@@ -785,7 +787,6 @@ function _precompilepkgs(pkgs::Union{Vector{String}, Vector{PkgId}},
785787
else
786788
target[] = "for $nconfigs compilation configurations..."
787789
end
788-
@debug "precompile: packages filtered"
789790

790791
pkg_queue = PkgConfig[]
791792
failed_deps = Dict{PkgConfig, String}()
@@ -1013,16 +1014,23 @@ function _precompilepkgs(pkgs::Union{Vector{String}, Vector{PkgId}},
10131014
return
10141015
end
10151016
try
1016-
# allows processes to wait if another process is precompiling a given package to
1017-
# a functionally identical package cache (except for preferences, which may differ)
1018-
t = @elapsed ret = precompile_pkgs_maybe_cachefile_lock(io, print_lock, fancyprint, pkg_config, pkgspidlocked, hascolor, parallel_limiter, ignore_loaded) do
1019-
Base.with_logger(Base.NullLogger()) do
1020-
# whether to respect already loaded dependency versions
1021-
keep_loaded_modules = !ignore_loaded
1022-
# for extensions, any extension in our direct dependencies is one we have a right to load
1023-
# for packages, we may load any extension (all possible triggers are accounted for above)
1024-
loadable_exts = haskey(ext_to_parent, pkg) ? filter((dep)->haskey(ext_to_parent, dep), direct_deps[pkg]) : nothing
1025-
Base.compilecache(pkg, sourcepath, std_pipe, std_pipe, keep_loaded_modules;
1017+
# for extensions, any extension in our direct dependencies is one we have a right to load
1018+
# for packages, we may load any extension (all possible triggers are accounted for above)
1019+
loadable_exts = haskey(ext_to_parent, pkg) ? filter((dep)->haskey(ext_to_parent, dep), direct_deps[pkg]) : nothing
1020+
if _from_loading && pkg in requested_pkgids
1021+
# loading already took the cachefile_lock and printed logmsg for its explicit requests
1022+
t = @elapsed ret = begin
1023+
Base.compilecache(pkg, sourcepath, std_pipe, std_pipe, !ignore_loaded;
1024+
flags, cacheflags, loadable_exts)
1025+
end
1026+
else
1027+
# allows processes to wait if another process is precompiling a given package to
1028+
# a functionally identical package cache (except for preferences, which may differ)
1029+
t = @elapsed ret = precompile_pkgs_maybe_cachefile_lock(io, print_lock, fancyprint, pkg_config, pkgspidlocked, hascolor, parallel_limiter, ignore_loaded) do
1030+
logcalls === nothing || @lock print_lock begin
1031+
Base.@logmsg logcalls "Precompiling $(repr("text/plain", pkg))"
1032+
end
1033+
Base.compilecache(pkg, sourcepath, std_pipe, std_pipe, !ignore_loaded;
10261034
flags, cacheflags, loadable_exts)
10271035
end
10281036
end

test/precompile.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -642,7 +642,7 @@ precompile_test_harness(false) do dir
642642
end
643643
""")
644644

645-
cachefile, _ = @test_logs (:debug, r"Precompiling FooBar") min_level=Logging.Debug match_mode=:any Base.compilecache(Base.PkgId("FooBar"))
645+
cachefile, _ = @test_logs (:debug, r"Generating object cache file for FooBar") min_level=Logging.Debug match_mode=:any Base.compilecache(Base.PkgId("FooBar"))
646646
empty_prefs_hash = Base.get_preferences_hash(nothing, String[])
647647
@test cachefile == Base.compilecache_path(Base.PkgId("FooBar"), empty_prefs_hash)
648648
@test isfile(joinpath(cachedir, "FooBar.ji"))

0 commit comments

Comments
 (0)