From 34a8c47b3d139937d008ae0e2a7476780609819e Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Fri, 18 Oct 2024 15:37:36 -0400 Subject: [PATCH] fix precompile process flag propagation (#56214) CacheFlags could get set, but were never propagated to the target process, so the result would be unusable. Additionally, the debug and optimization levels were not synchronized with the sysimg, causing a regression in pkgimage usability after moving out stdlibs. Fixes #56207 Fixes #56054 Fixes #56206 (cherry picked from commit 82b150645e318bcb6bcb450e945b3b689a1eb264) --- base/Base.jl | 2 +- base/loading.jl | 61 ++++++++++++++++++++++++----------- base/precompilation.jl | 72 +++++++++++++++++++++++++++--------------- base/show.jl | 5 ++- base/util.jl | 3 +- base/uuid.jl | 2 ++ pkgimage.mk | 3 +- src/staticdata_utils.c | 15 ++++----- test/loading.jl | 22 ++++++------- 9 files changed, 117 insertions(+), 68 deletions(-) diff --git a/base/Base.jl b/base/Base.jl index 0aa191206e3df..881e9f258dd5d 100644 --- a/base/Base.jl +++ b/base/Base.jl @@ -508,6 +508,7 @@ include("deepcopy.jl") include("download.jl") include("summarysize.jl") include("errorshow.jl") +include("util.jl") include("initdefs.jl") Filesystem.__postinit__() @@ -524,7 +525,6 @@ include("loading.jl") # misc useful functions & macros include("timing.jl") -include("util.jl") include("client.jl") include("asyncmap.jl") diff --git a/base/loading.jl b/base/loading.jl index a839798b9f9b9..e0d060c56f302 100644 --- a/base/loading.jl +++ b/base/loading.jl @@ -1645,6 +1645,8 @@ function CacheFlags(cf::CacheFlags=CacheFlags(ccall(:jl_cache_flags, UInt8, ())) opt_level === nothing ? cf.opt_level : opt_level ) end +# reflecting jloptions.c defaults +const DefaultCacheFlags = CacheFlags(use_pkgimages=true, debug_level=isdebugbuild() ? 2 : 1, check_bounds=0, inline=true, opt_level=2) function _cacheflag_to_uint8(cf::CacheFlags)::UInt8 f = UInt8(0) @@ -1656,12 +1658,29 @@ function _cacheflag_to_uint8(cf::CacheFlags)::UInt8 return f end +function translate_cache_flags(cacheflags::CacheFlags, defaultflags::CacheFlags) + opts = String[] + cacheflags.use_pkgimages != defaultflags.use_pkgimages && push!(opts, cacheflags.use_pkgimages ? "--pkgimages=yes" : "--pkgimages=no") + cacheflags.debug_level != defaultflags.debug_level && push!(opts, "-g$(cacheflags.debug_level)") + cacheflags.check_bounds != defaultflags.check_bounds && push!(opts, ("--check-bounds=auto", "--check-bounds=yes", "--check-bounds=no")[cacheflags.check_bounds + 1]) + cacheflags.inline != defaultflags.inline && push!(opts, cacheflags.inline ? "--inline=yes" : "--inline=no") + cacheflags.opt_level != defaultflags.opt_level && push!(opts, "-O$(cacheflags.opt_level)") + return opts +end + function show(io::IO, cf::CacheFlags) - print(io, "use_pkgimages = ", cf.use_pkgimages) - print(io, ", debug_level = ", cf.debug_level) - print(io, ", check_bounds = ", cf.check_bounds) - print(io, ", inline = ", cf.inline) - print(io, ", opt_level = ", cf.opt_level) + print(io, "CacheFlags(") + print(io, "; use_pkgimages=") + print(io, cf.use_pkgimages) + print(io, ", debug_level=") + print(io, cf.debug_level) + print(io, ", check_bounds=") + print(io, cf.check_bounds) + print(io, ", inline=") + print(io, cf.inline) + print(io, ", opt_level=") + print(io, cf.opt_level) + print(io, ")") end struct ImageTarget @@ -2848,7 +2867,8 @@ end const PRECOMPILE_TRACE_COMPILE = Ref{String}() function create_expr_cache(pkg::PkgId, input::String, output::String, output_o::Union{Nothing, String}, - concrete_deps::typeof(_concrete_dependencies), flags::Cmd=``, internal_stderr::IO = stderr, internal_stdout::IO = stdout, isext::Bool=false) + concrete_deps::typeof(_concrete_dependencies), flags::Cmd=``, cacheflags::CacheFlags=CacheFlags(), + internal_stderr::IO = stderr, internal_stdout::IO = stdout, isext::Bool=false) @nospecialize internal_stderr internal_stdout rm(output, force=true) # Remove file if it exists output_o === nothing || rm(output_o, force=true) @@ -2891,24 +2911,29 @@ function create_expr_cache(pkg::PkgId, input::String, output::String, output_o:: deps = deps_eltype * "[" * join(deps_strs, ",") * "]" precomp_stack = "Base.PkgId[$(join(map(pkg_str, vcat(Base.precompilation_stack, pkg)), ", "))]" + if output_o === nothing + # remove options that make no difference given the other cache options + cacheflags = CacheFlags(cacheflags, opt_level=0) + end + opts = translate_cache_flags(cacheflags, CacheFlags()) # julia_cmd is generated for the running system, and must be fixed if running for precompile instead if output_o !== nothing @debug "Generating object cache file for $(repr("text/plain", pkg))" cpu_target = get(ENV, "JULIA_CPU_TARGET", nothing) - opts = `--output-o $(output_o) --output-ji $(output) --output-incremental=yes` + push!(opts, "--output-o", output_o) else @debug "Generating cache file for $(repr("text/plain", pkg))" cpu_target = nothing - opts = `-O0 --output-ji $(output) --output-incremental=yes` end + push!(opts, "--output-ji", output) + isassigned(PRECOMPILE_TRACE_COMPILE) && push!(opts, "--trace-compile=$(PRECOMPILE_TRACE_COMPILE[])") - trace = isassigned(PRECOMPILE_TRACE_COMPILE) ? `--trace-compile=$(PRECOMPILE_TRACE_COMPILE[]) --trace-compile-timing` : `` io = open(pipeline(addenv(`$(julia_cmd(;cpu_target)::Cmd) - $(flags) - $(opts) - --startup-file=no --history-file=no --warn-overwrite=yes - --color=$(have_color === nothing ? "auto" : have_color ? "yes" : "no") - $trace - -`, + $(flags) + $(opts) + --output-incremental=yes + --startup-file=no --history-file=no --warn-overwrite=yes + $(have_color === nothing ? "--color=auto" : have_color ? "--color=yes" : "--color=no") + -`, "OPENBLAS_NUM_THREADS" => 1, "JULIA_NUM_THREADS" => 1), stderr = internal_stderr, stdout = internal_stdout), @@ -3026,7 +3051,7 @@ function compilecache(pkg::PkgId, path::String, internal_stderr::IO = stderr, in close(tmpio_o) close(tmpio_so) end - p = create_expr_cache(pkg, path, tmppath, tmppath_o, concrete_deps, flags, internal_stderr, internal_stdout, isext) + p = create_expr_cache(pkg, path, tmppath, tmppath_o, concrete_deps, flags, cacheflags, internal_stderr, internal_stdout, isext) if success(p) if cache_objects @@ -3997,5 +4022,5 @@ end precompile(include_package_for_output, (PkgId, String, Vector{String}, Vector{String}, Vector{String}, typeof(_concrete_dependencies), Nothing)) || @assert false precompile(include_package_for_output, (PkgId, String, Vector{String}, Vector{String}, Vector{String}, typeof(_concrete_dependencies), String)) || @assert false -precompile(create_expr_cache, (PkgId, String, String, String, typeof(_concrete_dependencies), Cmd, IO, IO)) || @assert false -precompile(create_expr_cache, (PkgId, String, String, Nothing, typeof(_concrete_dependencies), Cmd, IO, IO)) || @assert false +precompile(create_expr_cache, (PkgId, String, String, String, typeof(_concrete_dependencies), Cmd, CacheFlags, IO, IO)) || @assert false +precompile(create_expr_cache, (PkgId, String, String, Nothing, typeof(_concrete_dependencies), Cmd, CacheFlags, IO, IO)) || @assert false diff --git a/base/precompilation.jl b/base/precompilation.jl index 34720a1078816..0cd0f6346c308 100644 --- a/base/precompilation.jl +++ b/base/precompilation.jl @@ -349,7 +349,7 @@ function printpkgstyle(io, header, msg; color=:light_green) end const Config = Pair{Cmd, Base.CacheFlags} -const PkgConfig = Tuple{Base.PkgId,Config} +const PkgConfig = Tuple{PkgId,Config} function precompilepkgs(pkgs::Vector{String}=String[]; internal_call::Bool=false, @@ -360,10 +360,23 @@ function precompilepkgs(pkgs::Vector{String}=String[]; configs::Union{Config,Vector{Config}}=(``=>Base.CacheFlags()), io::IO=stderr, # asking for timing disables fancy mode, as timing is shown in non-fancy mode - fancyprint::Bool = can_fancyprint(io) && !timing - ) + fancyprint::Bool = can_fancyprint(io) && !timing) + # monomorphize this to avoid latency problems + _precompilepkgs(pkgs, internal_call, strict, warn_loaded, timing, _from_loading, + configs isa Vector{Config} ? configs : [configs], + IOContext{IO}(io), fancyprint) +end - configs = configs isa Config ? [configs] : configs +function _precompilepkgs(pkgs::Vector{String}, + internal_call::Bool, + strict::Bool, + warn_loaded::Bool, + timing::Bool, + _from_loading::Bool, + configs::Vector{Config}, + io::IOContext{IO}, + fancyprint::Bool) + requested_pkgs = copy(pkgs) # for understanding user intent time_start = time_ns() @@ -379,17 +392,32 @@ function precompilepkgs(pkgs::Vector{String}=String[]; if _from_loading && !Sys.isinteractive() && Base.get_bool_env("JULIA_TESTS", false) # suppress passive loading printing in julia test suite. `JULIA_TESTS` is set in Base.runtests - io = devnull + io = IOContext{IO}(devnull) end + nconfigs = length(configs) hascolor = get(io, :color, false)::Bool color_string(cstr::String, col::Union{Int64, Symbol}) = _color_string(cstr, col, hascolor) stale_cache = Dict{StaleCacheKey, Bool}() - exts = Dict{Base.PkgId, String}() # ext -> parent + exts = Dict{PkgId, String}() # ext -> parent # make a flat map of each dep and its direct deps - depsmap = Dict{Base.PkgId, Vector{Base.PkgId}}() - pkg_exts_map = Dict{Base.PkgId, Vector{Base.PkgId}}() + depsmap = Dict{PkgId, Vector{PkgId}}() + pkg_exts_map = Dict{PkgId, Vector{PkgId}}() + + function describe_pkg(pkg::PkgId, is_direct_dep::Bool, flags::Cmd, cacheflags::Base.CacheFlags) + name = haskey(exts, pkg) ? string(exts[pkg], " → ", pkg.name) : pkg.name + name = is_direct_dep ? name : color_string(name, :light_black) + if nconfigs > 1 && !isempty(flags) + config_str = join(flags, " ") + name *= color_string(" `$config_str`", :light_black) + end + if nconfigs > 1 + config_str = join(Base.translate_cache_flags(cacheflags, Base.DefaultCacheFlags), " ") + name *= color_string(" $config_str", :light_black) + end + return name + end for (dep, deps) in env.deps pkg = Base.PkgId(dep, env.names[dep]) @@ -554,7 +582,6 @@ function precompilepkgs(pkgs::Vector{String}=String[]; else target = "project" end - nconfigs = length(configs) if nconfigs == 1 if !isempty(only(configs)[1]) target *= " for configuration $(join(only(configs)[1], " "))" @@ -569,7 +596,7 @@ function precompilepkgs(pkgs::Vector{String}=String[]; failed_deps = Dict{PkgConfig, String}() precomperr_deps = PkgConfig[] # packages that may succeed after a restart (i.e. loaded packages with no cache file) - print_lock = io isa Base.LibuvStream ? io.lock::ReentrantLock : ReentrantLock() + print_lock = io.io isa Base.LibuvStream ? io.io.lock::ReentrantLock : ReentrantLock() first_started = Base.Event() printloop_should_exit::Bool = !fancyprint # exit print loop immediately if not fancy printing interrupted_or_done = Base.Event() @@ -658,7 +685,7 @@ function precompilepkgs(pkgs::Vector{String}=String[]; n_print_rows = 0 while !printloop_should_exit lock(print_lock) do - term_size = Base.displaysize_(io) + term_size = displaysize(io) num_deps_show = term_size[1] - 3 pkg_queue_show = if !interrupted_or_done.set && length(pkg_queue) > num_deps_show last(pkg_queue, num_deps_show) @@ -673,7 +700,7 @@ function precompilepkgs(pkgs::Vector{String}=String[]; bar.max = n_total - n_already_precomp # when sizing to the terminal width subtract a little to give some tolerance to resizing the # window between print cycles - termwidth = Base.displaysize_(io)[2] - 4 + termwidth = displaysize(io)[2] - 4 if !final_loop str = sprint(io -> show_progress(io, bar; termwidth, carriagereturn=false); context=io) print(iostr, Base._truncate_at_width_or_chars(true, str, termwidth), "\n") @@ -681,12 +708,8 @@ function precompilepkgs(pkgs::Vector{String}=String[]; for pkg_config in pkg_queue_show dep, config = pkg_config loaded = warn_loaded && haskey(Base.loaded_modules, dep) - _name = haskey(exts, dep) ? string(exts[dep], " → ", dep.name) : dep.name - name = dep in direct_deps ? _name : string(color_string(_name, :light_black)) - if nconfigs > 1 && !isempty(config[1]) - config_str = "$(join(config[1], " "))" - name *= color_string(" $(config_str)", :light_black) - end + flags, cacheflags = config + name = describe_pkg(dep, dep in direct_deps, flags, cacheflags) line = if pkg_config in precomperr_deps string(color_string(" ? ", Base.warn_color()), name) elseif haskey(failed_deps, pkg_config) @@ -774,14 +797,11 @@ function precompilepkgs(pkgs::Vector{String}=String[]; std_pipe = Base.link_pipe!(Pipe(); reader_supports_async=true, writer_supports_async=true) t_monitor = @async monitor_std(pkg_config, std_pipe; single_requested_pkg) - _name = haskey(exts, pkg) ? string(exts[pkg], " → ", pkg.name) : pkg.name - name = is_direct_dep ? _name : string(color_string(_name, :light_black)) - if nconfigs > 1 && !isempty(flags) - config_str = "$(join(flags, " "))" - name *= color_string(" $(config_str)", :light_black) - end - !fancyprint && lock(print_lock) do - isempty(pkg_queue) && printpkgstyle(io, :Precompiling, target) + name = describe_pkg(pkg, is_direct_dep, flags, cacheflags) + lock(print_lock) do + if !fancyprint && isempty(pkg_queue) + printpkgstyle(io, :Precompiling, something(target, "packages...")) + end end push!(pkg_queue, pkg_config) started[pkg_config] = true diff --git a/base/show.jl b/base/show.jl index 724fc70f89678..3bec8a5c885a2 100644 --- a/base/show.jl +++ b/base/show.jl @@ -324,8 +324,11 @@ end convert(::Type{IOContext}, io::IOContext) = io convert(::Type{IOContext}, io::IO) = IOContext(io, ioproperties(io))::IOContext +convert(::Type{IOContext{IO_t}}, io::IOContext{IO_t}) where {IO_t} = io +convert(::Type{IOContext{IO_t}}, io::IO) where {IO_t} = IOContext{IO_t}(io, ioproperties(io))::IOContext{IO_t} IOContext(io::IO) = convert(IOContext, io) +IOContext{IO_t}(io::IO) where {IO_t} = convert(IOContext{IO_t}, io) function IOContext(io::IO, KV::Pair) d = ioproperties(io) @@ -427,7 +430,7 @@ get(io::IO, key, default) = default keys(io::IOContext) = keys(io.dict) keys(io::IO) = keys(ImmutableDict{Symbol,Any}()) -displaysize(io::IOContext) = haskey(io, :displaysize) ? io[:displaysize]::Tuple{Int,Int} : Base.displaysize_(io.io) +displaysize(io::IOContext) = haskey(io, :displaysize) ? io[:displaysize]::Tuple{Int,Int} : displaysize(io.io) show_circular(io::IO, @nospecialize(x)) = false function show_circular(io::IOContext, @nospecialize(x)) diff --git a/base/util.jl b/base/util.jl index 5e86f026f8f9a..3a621211162ec 100644 --- a/base/util.jl +++ b/base/util.jl @@ -249,7 +249,7 @@ function julia_cmd(julia=joinpath(Sys.BINDIR, julia_exename()); cpu_target::Unio end function julia_exename() - if !Base.isdebugbuild() + if !isdebugbuild() return @static Sys.iswindows() ? "julia.exe" : "julia" else return @static Sys.iswindows() ? "julia-debug.exe" : "julia-debug" @@ -512,7 +512,6 @@ function _crc32c(io::IO, nb::Integer, crc::UInt32=0x00000000) end _crc32c(io::IO, crc::UInt32=0x00000000) = _crc32c(io, typemax(Int64), crc) _crc32c(io::IOStream, crc::UInt32=0x00000000) = _crc32c(io, filesize(io)-position(io), crc) -_crc32c(uuid::UUID, crc::UInt32=0x00000000) = _crc32c(uuid.value, crc) _crc32c(x::UInt128, crc::UInt32=0x00000000) = ccall(:jl_crc32c, UInt32, (UInt32, Ref{UInt128}, Csize_t), crc, x, 16) _crc32c(x::UInt64, crc::UInt32=0x00000000) = diff --git a/base/uuid.jl b/base/uuid.jl index 9b2da3c6409db..56f3a6aa417e7 100644 --- a/base/uuid.jl +++ b/base/uuid.jl @@ -36,6 +36,8 @@ let Base.hash(uuid::UUID, h::UInt) = hash(uuid_hash_seed, hash(convert(NTuple{2, UInt64}, uuid), h)) end +_crc32c(uuid::UUID, crc::UInt32=0x00000000) = _crc32c(uuid.value, crc) + let @inline function uuid_kernel(s, i, u) _c = UInt32(@inbounds codeunit(s, i)) diff --git a/pkgimage.mk b/pkgimage.mk index 0bc035ee03b08..78b2618be549f 100644 --- a/pkgimage.mk +++ b/pkgimage.mk @@ -25,7 +25,8 @@ print-depot-path: @$(call PRINT_JULIA, $(call spawn,$(JULIA_EXECUTABLE)) --startup-file=no -e '@show Base.DEPOT_PATH') $(BUILDDIR)/stdlib/%.image: $(JULIAHOME)/stdlib/Project.toml $(JULIAHOME)/stdlib/Manifest.toml $(INDEPENDENT_STDLIBS_SRCS) $(JULIA_DEPOT_PATH)/compiled - @$(call PRINT_JULIA, JULIA_CPU_TARGET="$(JULIA_CPU_TARGET)" $(call spawn,$(JULIA_EXECUTABLE)) --startup-file=no -e 'Base.Precompilation.precompilepkgs(;configs=[``=>Base.CacheFlags(), `--check-bounds=yes`=>Base.CacheFlags(;check_bounds=1)])') + @$(call PRINT_JULIA, JULIA_CPU_TARGET="$(JULIA_CPU_TARGET)" $(call spawn,$(JULIA_EXECUTABLE)) --startup-file=no -e \ + 'Base.Precompilation.precompilepkgs(configs=[``=>Base.CacheFlags(debug_level=2, opt_level=3), ``=>Base.CacheFlags(check_bounds=1, debug_level=2, opt_level=3)])') touch $@ $(BUILDDIR)/stdlib/release.image: $(build_private_libdir)/sys.$(SHLIB_EXT) diff --git a/src/staticdata_utils.c b/src/staticdata_utils.c index c6bfcae1dca8d..1669bcc10d0ca 100644 --- a/src/staticdata_utils.c +++ b/src/staticdata_utils.c @@ -600,15 +600,15 @@ static void write_mod_list(ios_t *s, jl_array_t *a) write_int32(s, 0); } -// OPT_LEVEL should always be the upper bits #define OPT_LEVEL 6 +#define DEBUG_LEVEL 1 JL_DLLEXPORT uint8_t jl_cache_flags(void) { // OOICCDDP uint8_t flags = 0; flags |= (jl_options.use_pkgimages & 1); // 0-bit - flags |= (jl_options.debug_level & 3) << 1; // 1-2 bit + flags |= (jl_options.debug_level & 3) << DEBUG_LEVEL; // 1-2 bit flags |= (jl_options.check_bounds & 3) << 3; // 3-4 bit flags |= (jl_options.can_inline & 1) << 5; // 5-bit flags |= (jl_options.opt_level & 3) << OPT_LEVEL; // 6-7 bit @@ -631,14 +631,13 @@ JL_DLLEXPORT uint8_t jl_match_cache_flags(uint8_t requested_flags, uint8_t actua actual_flags &= ~1; } - // 2. Check all flags, except opt level must be exact - uint8_t mask = (1 << OPT_LEVEL)-1; + // 2. Check all flags, except opt level and debug level must be exact + uint8_t mask = (~(3u << OPT_LEVEL) & ~(3u << DEBUG_LEVEL)) & 0x7f; if ((actual_flags & mask) != (requested_flags & mask)) return 0; - // 3. allow for higher optimization flags in cache - actual_flags >>= OPT_LEVEL; - requested_flags >>= OPT_LEVEL; - return actual_flags >= requested_flags; + // 3. allow for higher optimization and debug level flags in cache to minimize required compile option combinations + return ((actual_flags >> OPT_LEVEL) & 3) >= ((requested_flags >> OPT_LEVEL) & 3) && + ((actual_flags >> DEBUG_LEVEL) & 3) >= ((requested_flags >> DEBUG_LEVEL) & 3); } JL_DLLEXPORT uint8_t jl_match_cache_flags_current(uint8_t flags) diff --git a/test/loading.jl b/test/loading.jl index f06b0fd65ffc7..664bd778ca926 100644 --- a/test/loading.jl +++ b/test/loading.jl @@ -1212,10 +1212,7 @@ end @test cf.check_bounds == 3 @test cf.inline @test cf.opt_level == 3 - - io = PipeBuffer() - show(io, cf) - @test read(io, String) == "use_pkgimages = true, debug_level = 3, check_bounds = 3, inline = true, opt_level = 3" + @test repr(cf) == "CacheFlags(; use_pkgimages=true, debug_level=3, check_bounds=3, inline=true, opt_level=3)" end empty!(Base.DEPOT_PATH) @@ -1403,13 +1400,16 @@ end "JULIA_LOAD_PATH" => dir, "JULIA_DEBUG" => "loading") - out = Pipe() - proc = run(pipeline(cmd, stdout=out, stderr=out)) - close(out.in) - - log = @async String(read(out)) - @test success(proc) - fetch(log) + out = Base.PipeEndpoint() + log = @async read(out, String) + try + proc = run(pipeline(cmd, stdout=out, stderr=out)) + @test success(proc) + catch + @show fetch(log) + rethrow() + end + return fetch(log) end log = load_package("Parent", `--compiled-modules=no --pkgimages=no`)