From f8845cfe786c116346cd2cea3bf37230081678ea Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Wed, 16 Oct 2024 09:31:00 +0000 Subject: [PATCH 1/2] Break dependency between loading and Core.Compiler This code was originally added in df81bf9a96c59f257a01307cd0ecf05035d8301f where Core.Compiler would keep an array of all the things it inferred, which could then be provieded to the runtime to be included in the package image. In 113efb6e0aa27879cb423ab323c0159911e4c5e7 keeping the array itself became a runtime service for locking considerations. As a result, the role of Core.Compiler here is a bit weird. It has the enable switch and the GC root, but all the actual state is being managed by the runtime. It would be desirable to remove the Core.Compiler reference, so that loading.jl can function even if `Core.Compiler` does not exist (which is in theory supposed to be possible, even though we currently never run in such a configuration; that said, post trimming one might imagine useful instances of such a setup). To do this, put the runtime fully in charge of managing this array. Core.Compiler will call the callback unconditionally for all newly inferred cis and the runtime can decide whether to save it or not. Extracted from #56128 --- base/compiler/cicache.jl | 6 ++++++ base/compiler/typeinfer.jl | 10 ---------- base/loading.jl | 13 ++++++++++--- src/staticdata_utils.c | 6 +++++- 4 files changed, 21 insertions(+), 14 deletions(-) diff --git a/base/compiler/cicache.jl b/base/compiler/cicache.jl index a6ed18fe5105f..6bc6e45f9b1a3 100644 --- a/base/compiler/cicache.jl +++ b/base/compiler/cicache.jl @@ -13,6 +13,12 @@ end function setindex!(cache::InternalCodeCache, ci::CodeInstance, mi::MethodInstance) @assert ci.owner === cache.owner + if cache.owner === nothing + m = mi.def + if isa(m, Method) && m.module != Core + ccall(:jl_push_newly_inferred, Cvoid, (Any,), ci) + end + end ccall(:jl_mi_cache_insert, Cvoid, (Any, Any), mi, ci) return cache end diff --git a/base/compiler/typeinfer.jl b/base/compiler/typeinfer.jl index 8b85f7c6f35f1..2a3bbf3854302 100644 --- a/base/compiler/typeinfer.jl +++ b/base/compiler/typeinfer.jl @@ -1,9 +1,5 @@ # This file is a part of Julia. License is MIT: https://julialang.org/license -# Tracking of newly-inferred CodeInstances during precompilation -const track_newly_inferred = RefValue{Bool}(false) -const newly_inferred = CodeInstance[] - """ The module `Core.Compiler.Timings` provides a simple implementation of nested timers that can be used to measure the exclusive time spent inferring each method instance that is @@ -264,12 +260,6 @@ function cache_result!(interp::AbstractInterpreter, result::InferenceResult) if cache_results code_cache(interp)[mi] = result.ci - if track_newly_inferred[] - m = mi.def - if isa(m, Method) && m.module != Core - ccall(:jl_push_newly_inferred, Cvoid, (Any,), result.ci) - end - end end return cache_results end diff --git a/base/loading.jl b/base/loading.jl index fe4a4770628da..2448fdba44053 100644 --- a/base/loading.jl +++ b/base/loading.jl @@ -2863,6 +2863,9 @@ function load_path_setup_code(load_path::Bool=true) return code end +# Const global for GC root +const newly_inferred = CodeInstance[] + # this is called in the external process that generates precompiled package files function include_package_for_output(pkg::PkgId, input::String, depot_path::Vector{String}, dl_load_path::Vector{String}, load_path::Vector{String}, concrete_deps::typeof(_concrete_dependencies), source::Union{Nothing,String}) @@ -2882,8 +2885,7 @@ function include_package_for_output(pkg::PkgId, input::String, depot_path::Vecto task_local_storage()[:SOURCE_PATH] = source end - ccall(:jl_set_newly_inferred, Cvoid, (Any,), Core.Compiler.newly_inferred) - Core.Compiler.track_newly_inferred.x = true + ccall(:jl_set_newly_inferred, Cvoid, (Any,), newly_inferred) try Base.include(Base.__toplevel__, input) catch ex @@ -2891,10 +2893,15 @@ function include_package_for_output(pkg::PkgId, input::String, depot_path::Vecto @debug "Aborting `create_expr_cache'" exception=(ErrorException("Declaration of __precompile__(false) not allowed"), catch_backtrace()) exit(125) # we define status = 125 means PrecompileableError finally - Core.Compiler.track_newly_inferred.x = false + ccall(:jl_set_newly_inferred, Cvoid, (Any,), nothing) end # check that the package defined the expected module so we can give a nice error message if not Base.check_package_module_loaded(pkg) + + # Re-populate the runtime's newly-inferred array, which will be included + # in the output. We removed it above to avoid including any code we may + # have compiled for error handling and validation. + ccall(:jl_set_newly_inferred, Cvoid, (Any,), newly_inferred) end function check_package_module_loaded(pkg::PkgId) diff --git a/src/staticdata_utils.c b/src/staticdata_utils.c index 8eb223d3cfbde..5f1095fec9168 100644 --- a/src/staticdata_utils.c +++ b/src/staticdata_utils.c @@ -91,12 +91,16 @@ extern jl_mutex_t world_counter_lock; // This gets called as the first step of Base.include_package_for_output JL_DLLEXPORT void jl_set_newly_inferred(jl_value_t* _newly_inferred) { - assert(_newly_inferred == NULL || jl_is_array(_newly_inferred)); + assert(_newly_inferred == NULL || _newly_inferred == jl_nothing || jl_is_array(_newly_inferred)); + if (_newly_inferred == jl_nothing) + _newly_inferred = NULL; newly_inferred = (jl_array_t*) _newly_inferred; } JL_DLLEXPORT void jl_push_newly_inferred(jl_value_t* ci) { + if (!newly_inferred) + return; JL_LOCK(&newly_inferred_mutex); size_t end = jl_array_nrows(newly_inferred); jl_array_grow_end(newly_inferred, 1); From a060824a2cdf93bd05df91fe3a2c833378222ff3 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Wed, 16 Oct 2024 10:43:03 +0000 Subject: [PATCH 2/2] Go back to notifying for all new CIs (not just the internal ones) --- base/compiler/cicache.jl | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/base/compiler/cicache.jl b/base/compiler/cicache.jl index 6bc6e45f9b1a3..bf32e8f12f085 100644 --- a/base/compiler/cicache.jl +++ b/base/compiler/cicache.jl @@ -13,11 +13,9 @@ end function setindex!(cache::InternalCodeCache, ci::CodeInstance, mi::MethodInstance) @assert ci.owner === cache.owner - if cache.owner === nothing - m = mi.def - if isa(m, Method) && m.module != Core - ccall(:jl_push_newly_inferred, Cvoid, (Any,), ci) - end + m = mi.def + if isa(m, Method) && m.module != Core + ccall(:jl_push_newly_inferred, Cvoid, (Any,), ci) end ccall(:jl_mi_cache_insert, Cvoid, (Any, Any), mi, ci) return cache