Skip to content

Commit

Permalink
fix precompile process flag propagation (#56214)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
vtjnash authored and KristofferC committed Oct 21, 2024
1 parent dfc3521 commit 5f51683
Show file tree
Hide file tree
Showing 9 changed files with 112 additions and 64 deletions.
2 changes: 1 addition & 1 deletion base/Base.jl
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,7 @@ include("deepcopy.jl")
include("download.jl")
include("summarysize.jl")
include("errorshow.jl")
include("util.jl")

include("initdefs.jl")
Filesystem.__postinit__()
Expand All @@ -549,7 +550,6 @@ include("loading.jl")

# misc useful functions & macros
include("timing.jl")
include("util.jl")
include("client.jl")
include("asyncmap.jl")

Expand Down
61 changes: 43 additions & 18 deletions base/loading.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
63 changes: 41 additions & 22 deletions base/precompilation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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()
Expand All @@ -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])
Expand Down Expand Up @@ -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])
Expand All @@ -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()
Expand Down Expand Up @@ -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)
Expand All @@ -692,20 +720,16 @@ 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")
end
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)
Expand Down Expand Up @@ -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..."))
Expand Down
5 changes: 4 additions & 1 deletion base/show.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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))
Expand Down
3 changes: 1 addition & 2 deletions base/util.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) =
Expand Down
2 changes: 2 additions & 0 deletions base/uuid.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
3 changes: 2 additions & 1 deletion pkgimage.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
15 changes: 7 additions & 8 deletions src/staticdata_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
22 changes: 11 additions & 11 deletions test/loading.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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`)
Expand Down

0 comments on commit 5f51683

Please sign in to comment.