From 5f516832a7ed0a06533a5ee0643b99d93b3f15be 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 --- base/Base.jl | 2 +- base/loading.jl | 61 ++++++++++++++++++++++++++++------------ base/precompilation.jl | 63 +++++++++++++++++++++++++++--------------- 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, 112 insertions(+), 64 deletions(-) diff --git a/base/Base.jl b/base/Base.jl index fe17063a0e4fe1..5fb764bd4cc01f 100644 --- a/base/Base.jl +++ b/base/Base.jl @@ -533,6 +533,7 @@ include("deepcopy.jl") include("download.jl") include("summarysize.jl") include("errorshow.jl") +include("util.jl") include("initdefs.jl") Filesystem.__postinit__() @@ -549,7 +550,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 31671102235d8e..a71c21bd793a9f 100644 --- a/base/loading.jl +++ b/base/loading.jl @@ -1683,6 +1683,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) @@ -1694,12 +1696,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 @@ -2950,7 +2969,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) @@ -2993,24 +3013,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), @@ -3128,7 +3153,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 @@ -4133,5 +4158,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 359d2b61800b12..f597acef9b57f0 100644 --- a/base/precompilation.jl +++ b/base/precompilation.jl @@ -362,7 +362,7 @@ function printpkgstyle(io, header, msg; color=: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, @@ -375,8 +375,22 @@ function precompilepkgs(pkgs::Vector{String}=String[]; # asking for timing disables fancy mode, as timing is shown in non-fancy mode fancyprint::Bool = can_fancyprint(io) && !timing, manifest::Bool=false,) + # 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, manifest) +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, + manifest::Bool) requested_pkgs = copy(pkgs) # for understanding user intent time_start = time_ns() @@ -393,17 +407,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]) @@ -569,7 +598,6 @@ function precompilepkgs(pkgs::Vector{String}=String[]; end end - nconfigs = length(configs) target = nothing if nconfigs == 1 if !isempty(only(configs)[1]) @@ -584,7 +612,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() @@ -677,7 +705,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 = max(term_size[1] - 3, 2) # show at least 2 deps pkg_queue_show = if !interrupted_or_done.set && length(pkg_queue) > num_deps_show last(pkg_queue, num_deps_show) @@ -692,7 +720,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") @@ -700,12 +728,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) @@ -793,12 +817,7 @@ 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 + name = describe_pkg(pkg, is_direct_dep, flags, cacheflags) lock(print_lock) do if !fancyprint && isempty(pkg_queue) printpkgstyle(io, :Precompiling, something(target, "packages...")) diff --git a/base/show.jl b/base/show.jl index fb932838ac69a3..25ed99f50b5b08 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 95d62c4a16e1d7..3ce64e50f7e292 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" @@ -530,7 +530,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 9b2da3c6409db7..56f3a6aa417e7c 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 0bc035ee03b089..78b2618be549f0 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 5f1095fec91688..9a7653972ea7c8 100644 --- a/src/staticdata_utils.c +++ b/src/staticdata_utils.c @@ -605,15 +605,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 @@ -636,14 +636,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 1901018ab19272..ecba64ca45a73f 100644 --- a/test/loading.jl +++ b/test/loading.jl @@ -1206,10 +1206,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) @@ -1401,13 +1398,16 @@ end "JULIA_DEPOT_PATH" => depot_path, "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`)