From 6eef3087e54505340281089b074d26e2c1d7e642 Mon Sep 17 00:00:00 2001 From: Kristoffer Carlsson Date: Mon, 20 Jul 2026 16:28:00 +0200 Subject: [PATCH] reenable global ref lookup optimization --- benchmark/benchmarks.jl | 42 ++++++++++++++++++- src/commands.jl | 26 ++++++------ src/optimize.jl | 93 ++++++++++++++++++++++++++++++----------- src/types.jl | 10 ++--- test/interpret.jl | 55 ++++++++++++++++++++++-- 5 files changed, 180 insertions(+), 46 deletions(-) diff --git a/benchmark/benchmarks.jl b/benchmark/benchmarks.jl index 80da56ac..eea15885 100644 --- a/benchmark/benchmarks.jl +++ b/benchmark/benchmarks.jl @@ -65,4 +65,44 @@ SUITE["ccall ptr"] = @benchmarkable @interpret ccall_ptr(ptr, 1, 5) function powf(a, b) ccall(("powf", Base.Math.libm), Float32, (Float32,Float32), a, b) end -SUITE["ccall library"] = @benchmarkable @interpret powf(2, 3) \ No newline at end of file +SUITE["ccall library"] = @benchmarkable @interpret powf(2, 3) + +# Global reads in a tight loop: `const` globals can be folded at framecode construction, +# non-`const` globals must be looked up on every execution +module BenchGlobals +const CONSTGLOBAL = 1 +NONCONST = 1 +end + +function readconst(n) + s = 0 + for i in 1:n + s += BenchGlobals.CONSTGLOBAL + end + return s +end +SUITE["const global read 10_000"] = @benchmarkable @interpret readconst(10_000) + +function readnonconst(n) + s = 0 + for i in 1:n + s += BenchGlobals.NONCONST + end + return s +end +SUITE["nonconst global read 10_000"] = @benchmarkable @interpret readnonconst(10_000) + +# Calls resolved through globals: every call statement's callee is a `GlobalRef` +function callglobals(x) + s = zero(x) + for i in 1:1_000 + s += sin(x) + cos(x) + abs(x) + min(x, one(x)) + end + return s +end +SUITE["global callees 1_000"] = @benchmarkable @interpret callglobals(0.5) + +# Frame creation: uncached measures the framecode build (including the `optimize!` pass), +# cached measures the per-call frame setup for an already-built framecode +SUITE["frame creation uncached"] = @benchmarkable JuliaInterpreter.enter_call(callglobals, 0.5) setup=(JuliaInterpreter.clear_caches()) evals=1 +SUITE["frame creation cached"] = @benchmarkable JuliaInterpreter.enter_call(callglobals, 0.5) diff --git a/src/commands.jl b/src/commands.jl index 1e5e39e8..a4056c2c 100644 --- a/src/commands.jl +++ b/src/commands.jl @@ -490,15 +490,13 @@ end maybe_step_through_kwprep!(frame::Frame, istoplevel::Bool=false) = maybe_step_through_kwprep!(RecursiveInterpreter(), frame, istoplevel) -@static if isbindingresolved_deprecated - function is_empty_namedtuple(stmt) - isexpr(stmt, :call) && length(stmt.args) == 1 || return false - arg1 = stmt.args[1] - isa(arg1, GlobalRef) || return false - return arg1.name === :NamedTuple - end -else - is_empty_namedtuple(stmt) = isexpr(stmt, :call) && is_quoted_type(stmt.args[1], :NamedTuple) && length(stmt.args) == 1 +# The `NamedTuple` callee is a `QuoteNode` when `optimize!` folded the const (method scope), +# or a `GlobalRef` when it didn't (toplevel scope on 1.12+); accept both forms. +function is_empty_namedtuple(stmt) + isexpr(stmt, :call) && length(stmt.args) == 1 || return false + arg1 = stmt.args[1] + is_quoted_type(arg1, :NamedTuple) && return true + return isa(arg1, GlobalRef) && arg1.name === :NamedTuple end """ @@ -699,11 +697,13 @@ function debug_command(interp::Interpreter, frame::Frame, cmd::Symbol, rootistop # On Julia 1.12 a method body may start with bare global loads # (e.g. the `+` of `f(x) = g(x) + 1`); pausing there shows the # user an internal-looking statement that is not even the next - # call. Advance to the first call or return, like frame entry - # does. Keyword/closure bodies (gensym `#` names) keep their - # exact entry point. + # call. `optimize!` folds `const` globals to `QuoteNode`s, so the + # leading load is either a `GlobalRef` or a `QuoteNode`. Advance + # to the first call or return, like frame entry does. + # Keyword/closure bodies (gensym `#` names) keep their exact entry point. scope = scopeof(newframe) - normalize_entry = pc_expr(newframe) isa GlobalRef && + entrystmt = pc_expr(newframe) + normalize_entry = (entrystmt isa GlobalRef || entrystmt isa QuoteNode) && !(scope isa Method && startswith(string(scope.name), "#")) if !is_si && normalize_entry pc = maybe_next_until!(interp, newframe, istoplevel) do fr::Frame diff --git a/src/optimize.jl b/src/optimize.jl index 40e5c6ef..160b4fca 100644 --- a/src/optimize.jl +++ b/src/optimize.jl @@ -10,11 +10,29 @@ const compiled_calls = Dict{Any,Any}() function record_world_dep!(world_deps::Union{Nothing,Vector{BindingPartition}}, world::UInt, gr::GlobalRef) @static if isbindingresolved_deprecated world_deps === nothing && return nothing - push!(world_deps, Base.lookup_binding_partition(world, gr)) + # KNOWN HOLE: only the named binding's partition is recorded. For an explicitly + # imported binding (`import M: x` / `using M: x`) that partition delegates to the + # source binding and survives a rebinding of the source const — only the source + # partition (where the value lives) gets split. `lookup_global_ref` therefore never + # folds such bindings, but the compiled `ccall`/`llvmcall` wrapper paths still bake + # values reached through them and would miss the rebinding (see the + # "imported const invalidation" `@test_broken`). Whether importer partitions should + # be split upstream, or the delegation chain recorded here, is under discussion. + record_world_dep!(world_deps, Base.lookup_binding_partition(world, gr)) end return nothing end +@static if isbindingresolved_deprecated +function record_world_dep!(world_deps::Union{Nothing,Vector{BindingPartition}}, bpart::BindingPartition) + world_deps === nothing && return nothing + # The same binding is typically referenced many times within one method; store each + # partition once so `framecode_valid_world` stays cheap. + any(p -> p === bpart, world_deps) || push!(world_deps, bpart) + return nothing +end +end + # Record every `GlobalRef` reachable in `arg` (recursing through `Expr`s, but not chasing # `SSAValue`s). Used when a whole expression's value is baked at framecode-build time, e.g. a # `(name, lib)` tuple evaluated for a compiled `ccall` wrapper or the feeder statements of an @@ -79,32 +97,55 @@ function smallest_ref(stmts, arg, idmin) return idmin end -function lookup_global_ref(a::GlobalRef, world::UInt) - isbindingresolved_deprecated && return a # TODO: reenable this optimization once we can invalidate Frames - if Base.isbindingresolved(a.mod, a.name) && - (invoke_in_world(world, isdefinedglobal, a.mod, a.name)) && - (invoke_in_world(world, isconst, a.mod, a.name)) - return QuoteNode(invoke_in_world(world, getglobal, a.mod, a.name)) +function lookup_global_ref(a::GlobalRef, world::UInt, + world_deps::Union{Nothing,Vector{BindingPartition}}=nothing) + @static if isbindingresolved_deprecated + # On 1.12+ a `const` can be rebound, making the folded value world-dependent. Folding + # is therefore allowed only when the caller supplies `world_deps`: the binding + # partition recorded there lets `framecode_valid_world` reject the cached framecode + # for worlds in which the binding was redefined. + world_deps === nothing && return a + bpart = Base.lookup_binding_partition(world, a) + # Fold only when the partition itself holds the constant value (a directly defined + # or implicit-`using` const): then this partition must be split for the value to + # change, so recording it makes the invalidation exact. Explicitly imported bindings + # (`import M: x` / `using M: x`) delegate to the source binding and their partition + # survives a rebinding of the source const, so a folded value could go stale + # undetected — leave those as `GlobalRef`s (resolved per execution in the frame's + # world, which is always correct). + if Base.is_defined_const_binding(Base.binding_kind(bpart)) + record_world_dep!(world_deps, bpart) + return QuoteNode(invoke_in_world(world, getglobal, a.mod, a.name)) + end + return a + else + if Base.isbindingresolved(a.mod, a.name) && + (invoke_in_world(world, isdefinedglobal, a.mod, a.name)) && + (invoke_in_world(world, isconst, a.mod, a.name)) + return QuoteNode(invoke_in_world(world, getglobal, a.mod, a.name)) + end + return a end - return a end -function lookup_global_refs!(ex::Expr, world::UInt) +function lookup_global_refs!(ex::Expr, world::UInt, + world_deps::Union{Nothing,Vector{BindingPartition}}=nothing) if isexpr(ex, (:isdefined, :thunk, :toplevel, :method, :global, :const, :globaldecl)) return nothing end for (i, a) in enumerate(ex.args) ex.head === :(=) && i == 1 && continue # Don't look up globalrefs on the LHS of an assignment (issue #98) if isa(a, GlobalRef) - ex.args[i] = lookup_global_ref(a, world) + ex.args[i] = lookup_global_ref(a, world, world_deps) elseif isa(a, Expr) - lookup_global_refs!(a, world) + lookup_global_refs!(a, world, world_deps) end end return nothing end -function lookup_getproperties(code::Vector{Any}, @nospecialize(a), world::UInt) +function lookup_getproperties(code::Vector{Any}, @nospecialize(a), world::UInt, + world_deps::Union{Nothing,Vector{BindingPartition}}=nothing) isexpr(a, :call) || return a length(a.args) == 3 || return a arg1 = lookup_stmt(code, a.args[1], world) @@ -113,17 +154,16 @@ function lookup_getproperties(code::Vector{Any}, @nospecialize(a), world::UInt) arg2 isa Module || return a arg3 = lookup_stmt(code, a.args[3], world) arg3 isa Symbol || return a - return lookup_global_ref(GlobalRef(arg2, arg3), world) + return lookup_global_ref(GlobalRef(arg2, arg3), world, world_deps) end # HACK This isn't optimization really, but necessary to bypass llvmcall and foreigncall # TODO This "optimization" should be refactored into a "minimum compilation" necessary to # execute `llvmcall` and `foreigncall` and pure optimizations on the lowered code representation. -# On Julia 1.12+ the GlobalRef -> QuoteNode folding is disabled (`lookup_global_ref` returns the -# ref unchanged) because a redefinable `const` makes the folded value world-dependent; values -# that *must* be resolved at build time (library names and llvmcall ingredients baked into the -# compiled wrappers below) record their bindings in `world_deps` so the framecode can be -# invalidated when a binding is redefined. +# On Julia 1.12+ a redefinable `const` makes a folded value world-dependent, so every value +# resolved at build time — folded `const` globals as well as library names and llvmcall +# ingredients baked into the compiled wrappers below — records its binding in `world_deps`, +# and `framecode_valid_world` rejects the cached framecode once any of them is redefined. """ optimize!(code::CodeInfo, scope, world::UInt) -> code, methodtables, world_deps @@ -142,10 +182,15 @@ function optimize!(code::CodeInfo, scope, world::UInt) sparams = scope isa Method ? sparam_syms(scope) : Symbol[] replace_coretypes!(code) - # Binding partitions of globals whose values get baked into this framecode (compiled - # `ccall`/`llvmcall` wrappers); recorded so a cached `FrameCode` can be invalidated once any - # baked value goes stale (see `FrameCode.world_deps`). + # Binding partitions of globals whose values get baked into this framecode (folded `const` + # globals and compiled `ccall`/`llvmcall` wrappers); recorded so a cached `FrameCode` can be + # invalidated once any baked value goes stale (see `FrameCode.world_deps`). world_deps = BindingPartition[] + # On 1.12+, fold `const` globals only for method scope: the framecode cache is guarded by + # `framecode_valid_world`, and a method frame's world is fixed at construction. Toplevel + # frames advance `frame.world` mid-execution (see `step_toplevel!`), so a value folded in + # the build world could go stale within the frame; leave their `GlobalRef`s unresolved. + fold_deps = scope isa Method ? world_deps : nothing # TODO: because of builtins.jl, for CodeInfos like # %1 = Core.apply_type # %2 = (%1)(args...) @@ -153,13 +198,13 @@ function optimize!(code::CodeInfo, scope, world::UInt) ## Replace GlobalRefs with QuoteNodes for (i, stmt) in enumerate(code.code) if isa(stmt, GlobalRef) - code.code[i] = lookup_global_ref(stmt, world) + code.code[i] = lookup_global_ref(stmt, world, fold_deps) elseif isa(stmt, Expr) if stmt.head === :call && stmt.args[1] === :cglobal # cglobal requires literals continue else - lookup_global_refs!(stmt, world) - code.code[i] = lookup_getproperties(code.code, stmt, world) + lookup_global_refs!(stmt, world, fold_deps) + code.code[i] = lookup_getproperties(code.code, stmt, world, fold_deps) end end end diff --git a/src/types.jl b/src/types.jl index 9821af64..492b9661 100644 --- a/src/types.jl +++ b/src/types.jl @@ -189,11 +189,11 @@ function is_breakpoint_expr(ex::Expr) return isa(q, QuoteNode) && q.value === :__BREAKPOINT_MARKER__ end -@static if isbindingresolved_deprecated - is_breakpoint_marker(stmt) = is_global_ref(stmt, JuliaInterpreter, :__BREAK_POINT_MARKER__) -else - is_breakpoint_marker(stmt) = stmt === __BREAK_POINT_MARKER__ -end +# `@bp` lowers to a `GlobalRef` of the `__BREAK_POINT_MARKER__` const. `optimize!` folds it +# to its value in method scope (unwrapped from the `QuoteNode` by `lookup_stmt`), while +# toplevel or unoptimized code keeps the `GlobalRef`, so accept both forms. +is_breakpoint_marker(@nospecialize(stmt)) = + stmt === __BREAK_POINT_MARKER__ || is_global_ref(stmt, JuliaInterpreter, :__BREAK_POINT_MARKER__) @static if VERSION ≥ v"1.12.0-DEV.173" function pushuniquefiles!(unique_files::Set{Symbol}, lt::Core.DebugInfo) diff --git a/test/interpret.jl b/test/interpret.jl index 699e3609..cfe357ad 100644 --- a/test/interpret.jl +++ b/test/interpret.jl @@ -1344,14 +1344,16 @@ end @test !isempty(fcchain.world_deps) # the `Base.Math.libm` getproperty-chain resolution # Rebinding the library const (e.g. after dlclosing one library and dlopening another) - # invalidates the framecode; the rebuild resolves the new value, so the call fails on the - # bogus path instead of silently calling into the stale library. + # invalidates the framecode; a rebuild in a world that sees the new value resolves it, so + # the call fails on the bogus path instead of silently calling into the stale library. + # (In worlds predating the rebinding — like this testset's task world — the old value is + # still the correct resolution, matching compiled-code semantics.) Core.eval(WrapperDepTest, :(const MYLIB = "/nonexistent_library_path")) w2 = Base.get_world_counter() @test !JuliaInterpreter.framecode_valid_world(fc, w2) fc2, _ = JuliaInterpreter.prepare_framecode(mlib, Tuple{typeof(WrapperDepTest.powf_constlib), Float32, Float32}; world=w2) @test fc2 !== fc - @test_throws "nonexistent_library_path" @interpret(WrapperDepTest.powf_constlib(2f0, 3f0)) + @test_throws "nonexistent_library_path" @interpret world=w2 WrapperDepTest.powf_constlib(2f0, 3f0) # An unrelated rebinding must not invalidate the chain-library framecode. @test JuliaInterpreter.framecode_valid_world(fcchain, w2) @@ -1371,6 +1373,53 @@ end end end +module ImportedConstSource + const XI = 1 + const XU = 1 + const LIB = Base.Math.libm +end +module ImportedConstConsumer + import ..ImportedConstSource: XI, LIB + using ..ImportedConstSource: XU + fi() = XI + fu() = XU + powf_importedlib(a, b) = ccall(("powf", LIB), Float32, (Float32, Float32), a, b) +end + +@static if JuliaInterpreter.isbindingresolved_deprecated +@testset "imported const invalidation" begin + # Explicitly imported consts (`import M: x` / `using M: x`) are never folded: the + # importing module's binding partition delegates to the source binding and is NOT split + # when the source const is rebound (only the source partition is), so a folded value + # could go stale without invalidating the framecode. The `GlobalRef` is left in place + # and resolved per execution in the frame's world, which is correct in every world. + for (f, src_name) in ((ImportedConstConsumer.fi, :XI), (ImportedConstConsumer.fu, :XU)) + m = only(methods(f)) + w = Base.get_world_counter() + fc, _ = JuliaInterpreter.prepare_framecode(m, Tuple{typeof(f)}; world=w) + @test any(x -> isa(x, GlobalRef), fc.src.code) # not folded + Core.eval(ImportedConstSource, :(const $src_name = 2)) + w2 = Base.get_world_counter() + @test JuliaInterpreter.framecode_valid_world(fc, w2) # nothing baked, still valid + @test (@interpret world=w2 f()) == 2 # matches native execution in w2 + @test (@interpret world=w f()) == 1 # pre-rebinding world sees the old value + end + + # KNOWN HOLE: the compiled-ccall wrapper path *bakes* values at build time regardless of + # binding kind, and `record_world_dep!` records only the importing partition, which + # survives the source rebinding — so the framecode wrongly stays valid. Whether importer + # partitions should be split upstream, or the delegation chain recorded in + # `record_world_dep!`, is still under discussion. + mlib = only(methods(ImportedConstConsumer.powf_importedlib)) + w = Base.get_world_counter() + @test (@interpret world=w ImportedConstConsumer.powf_importedlib(2f0, 3f0)) == 8f0 + fc, _ = JuliaInterpreter.prepare_framecode(mlib, Tuple{typeof(ImportedConstConsumer.powf_importedlib), Float32, Float32}; world=w) + Core.eval(ImportedConstSource, :(const LIB = "/nonexistent_library_path")) + w2 = Base.get_world_counter() + @test_broken !JuliaInterpreter.framecode_valid_world(fc, w2) +end +end # @static if + @testset "Empty varargs are visible to locals" begin empty_vararg(x...) = x fr = JuliaInterpreter.enter_call(empty_vararg)