Skip to content

Commit f3eece6

Browse files
committed
Cache binding pointer in GlobalRef
On certain workloads, profiling shows a surprisingly high fraction of inference time spent looking up bindings in modules. Bindings use a hash table, so they're expected to be quite fast, but certainly not zero. A big contributor to the problem is that we do basically treat it as zero, looking up bindings for GlobalRefs multiple times for each statement (e.g. in `isconst`, `isdefined`, to get the types, etc). This PR attempts to improve the situation by adding an extra field to GlobalRef that caches this lookup. This field is not serialized and if not set, we fallback to the previous behavior. I would expect the memory overhead to be relatively small, since we do intern GlobalRefs in memory to only have one per binding (rather than one per use). # Benchmarks The benchmarks look quite promising. Consider this artifical example (though it's actually not all that far fetched, given some of the generated code we see): ``` using Core.Intrinsics: add_int const ONE = 1 @eval function f(x, y) z = 0 $([:(z = add_int(x, add_int(z, ONE))) for _ = 1:10000]...) return add_int(z, y) end g(y) = f(ONE, y) ``` On master: ``` julia> @time @code_typed g(1) 1.427227 seconds (1.31 M allocations: 58.809 MiB, 5.73% gc time, 99.96% compilation time) CodeInfo( 1 ─ %1 = invoke Main.f(Main.ONE::Int64, y::Int64)::Int64 └── return %1 ) => Int64 ``` On this PR: ``` julia> @time @code_typed g(1) 0.503151 seconds (1.19 M allocations: 53.641 MiB, 5.10% gc time, 33.59% compilation time) CodeInfo( 1 ─ %1 = invoke Main.f(Main.ONE::Int64, y::Int64)::Int64 └── return %1 ) => Int64 ``` I don't expect the same speedup on other workloads, but there should be a few % speedup on most workloads still. # Future extensions The other motivation for this is that with a view towards #40399, we will want to more clearly define when bindings get resolved. The idea here would then be that binding resolution replaces generic `GlobalRefs` by GlobalRefs with a set binding cache, and any unresolved bindings would be treated conservatively by inference and optimization.
1 parent 7eacf1b commit f3eece6

File tree

11 files changed

+74
-23
lines changed

11 files changed

+74
-23
lines changed

base/boot.jl

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,7 @@ eval(Core, quote
412412
end
413413
LineInfoNode(mod::Module, @nospecialize(method), file::Symbol, line::Int32, inlined_at::Int32) =
414414
$(Expr(:new, :LineInfoNode, :mod, :method, :file, :line, :inlined_at))
415-
GlobalRef(m::Module, s::Symbol) = $(Expr(:new, :GlobalRef, :m, :s))
415+
GlobalRef(m::Module, s::Symbol, binding::Ptr{Nothing}) = $(Expr(:new, :GlobalRef, :m, :s, :binding))
416416
SlotNumber(n::Int) = $(Expr(:new, :SlotNumber, :n))
417417
TypedSlot(n::Int, @nospecialize(t)) = $(Expr(:new, :TypedSlot, :n, :t))
418418
PhiNode(edges::Array{Int32, 1}, values::Array{Any, 1}) = $(Expr(:new, :PhiNode, :edges, :values))
@@ -812,6 +812,8 @@ Unsigned(x::Union{Float16, Float32, Float64, Bool}) = UInt(x)
812812
Integer(x::Integer) = x
813813
Integer(x::Union{Float16, Float32, Float64}) = Int(x)
814814

815+
GlobalRef(m::Module, s::Symbol) = GlobalRef(m, s, bitcast(Ptr{Nothing}, 0))
816+
815817
# Binding for the julia parser, called as
816818
#
817819
# Core._parse(text, filename, lineno, offset, options)

base/compiler/abstractinterpretation.jl

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1982,7 +1982,7 @@ function abstract_eval_special_value(interp::AbstractInterpreter, @nospecialize(
19821982
return sv.argtypes[e.n]
19831983
end
19841984
elseif isa(e, GlobalRef)
1985-
return abstract_eval_global(interp, e.mod, e.name, sv)
1985+
return abstract_eval_globalref(interp, e, sv)
19861986
end
19871987

19881988
return Const(e)
@@ -2256,17 +2256,24 @@ function abstract_eval_statement(interp::AbstractInterpreter, @nospecialize(e),
22562256
return rt
22572257
end
22582258

2259-
function abstract_eval_global(M::Module, s::Symbol)
2260-
if isdefined(M, s) && isconst(M, s)
2261-
return Const(getglobal(M, s))
2259+
function isdefined_globalref(g::GlobalRef)
2260+
g.binding != C_NULL && return ccall(:jl_binding_boundp, Cint, (Ptr{Cvoid},), g.binding) != 0
2261+
return isdefined(g.mod, g.name)
2262+
end
2263+
2264+
function abstract_eval_globalref(g::GlobalRef)
2265+
if isdefined_globalref(g) && isconst(g)
2266+
g.binding != C_NULL && return Const(ccall(:jl_binding_value, Any, (Ptr{Cvoid},), g.binding))
2267+
return Const(getglobal(g.mod, g.name))
22622268
end
2263-
ty = ccall(:jl_binding_type, Any, (Any, Any), M, s)
2269+
ty = ccall(:jl_binding_type, Any, (Any, Any), g.mod, g.name)
22642270
ty === nothing && return Any
22652271
return ty
22662272
end
2273+
abstract_eval_global(M::Module, s::Symbol) = abstract_eval_globalref(GlobalRef(M, s))
22672274

2268-
function abstract_eval_global(interp::AbstractInterpreter, M::Module, s::Symbol, frame::Union{InferenceState, IRCode})
2269-
rt = abstract_eval_global(M, s)
2275+
function abstract_eval_globalref(interp::AbstractInterpreter, g::GlobalRef, frame::Union{InferenceState, IRCode})
2276+
rt = abstract_eval_globalref(g)
22702277
consistent = inaccessiblememonly = ALWAYS_FALSE
22712278
nothrow = false
22722279
if isa(rt, Const)
@@ -2277,7 +2284,7 @@ function abstract_eval_global(interp::AbstractInterpreter, M::Module, s::Symbol,
22772284
else
22782285
nothrow = true
22792286
end
2280-
elseif isdefined(M,s)
2287+
elseif isdefined_globalref(g)
22812288
nothrow = true
22822289
end
22832290
merge_effects!(interp, frame, Effects(EFFECTS_TOTAL; consistent, nothrow, inaccessiblememonly))

base/compiler/optimize.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,7 @@ function argextype(
335335
elseif isa(x, QuoteNode)
336336
return Const(x.value)
337337
elseif isa(x, GlobalRef)
338-
return abstract_eval_global(x.mod, x.name)
338+
return abstract_eval_globalref(x)
339339
elseif isa(x, PhiNode)
340340
return Any
341341
elseif isa(x, PiNode)

base/compiler/ssair/slot2ssa.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ function typ_for_val(@nospecialize(x), ci::CodeInfo, sptypes::Vector{Any}, idx::
216216
end
217217
return (ci.ssavaluetypes::Vector{Any})[idx]
218218
end
219-
isa(x, GlobalRef) && return abstract_eval_global(x.mod, x.name)
219+
isa(x, GlobalRef) && return abstract_eval_globalref(x)
220220
isa(x, SSAValue) && return (ci.ssavaluetypes::Vector{Any})[x.id]
221221
isa(x, Argument) && return slottypes[x.n]
222222
isa(x, NewSSAValue) && return DelayedTyp(x)

base/compiler/utilities.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,7 @@ function is_throw_call(e::Expr)
378378
if e.head === :call
379379
f = e.args[1]
380380
if isa(f, GlobalRef)
381-
ff = abstract_eval_global(f.mod, f.name)
381+
ff = abstract_eval_globalref(f)
382382
if isa(ff, Const) && ff.val === Core.throw
383383
return true
384384
end

base/reflection.jl

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,11 @@ Determine whether a global is declared `const` in a given module `m`.
265265
isconst(m::Module, s::Symbol) =
266266
ccall(:jl_is_const, Cint, (Any, Any), m, s) != 0
267267

268+
function isconst(g::GlobalRef)
269+
g.binding != C_NULL && return ccall(:jl_binding_is_const, Cint, (Ptr{Cvoid},), g.binding) != 0
270+
return isconst(g.mod, g.name)
271+
end
272+
268273
"""
269274
isconst(t::DataType, s::Union{Int,Symbol}) -> Bool
270275

src/dump.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2084,6 +2084,9 @@ static void jl_deserialize_struct(jl_serializer_state *s, jl_value_t *v) JL_GC_D
20842084
entry->min_world = 1;
20852085
entry->max_world = 0;
20862086
}
2087+
} else if (dt == jl_globalref_type) {
2088+
jl_globalref_t *r = (jl_globalref_t*)v;
2089+
r->bnd_cache = jl_get_binding(r->mod, r->name);
20872090
}
20882091
}
20892092

src/jltypes.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2392,12 +2392,6 @@ void jl_init_types(void) JL_GC_DISABLED
23922392
jl_svec(1, jl_slotnumber_type),
23932393
jl_emptysvec, 0, 0, 1);
23942394

2395-
jl_globalref_type =
2396-
jl_new_datatype(jl_symbol("GlobalRef"), core, jl_any_type, jl_emptysvec,
2397-
jl_perm_symsvec(2, "mod", "name"),
2398-
jl_svec(2, jl_module_type, jl_symbol_type),
2399-
jl_emptysvec, 0, 0, 2);
2400-
24012395
jl_code_info_type =
24022396
jl_new_datatype(jl_symbol("CodeInfo"), core,
24032397
jl_any_type, jl_emptysvec,
@@ -2694,6 +2688,12 @@ void jl_init_types(void) JL_GC_DISABLED
26942688

26952689
jl_value_t *pointer_void = jl_apply_type1((jl_value_t*)jl_pointer_type, (jl_value_t*)jl_nothing_type);
26962690

2691+
jl_globalref_type =
2692+
jl_new_datatype(jl_symbol("GlobalRef"), core, jl_any_type, jl_emptysvec,
2693+
jl_perm_symsvec(3, "mod", "name", "binding"),
2694+
jl_svec(3, jl_module_type, jl_symbol_type, pointer_void),
2695+
jl_emptysvec, 0, 0, 3);
2696+
26972697
tv = jl_svec2(tvar("A"), tvar("R"));
26982698
jl_opaque_closure_type = (jl_unionall_t*)jl_new_datatype(jl_symbol("OpaqueClosure"), core, jl_function_type, tv,
26992699
jl_perm_symsvec(5, "captures", "world", "source", "invoke", "specptr"),

src/julia.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -603,6 +603,13 @@ typedef struct _jl_module_t {
603603
jl_mutex_t lock;
604604
} jl_module_t;
605605

606+
typedef struct {
607+
jl_module_t *mod;
608+
jl_sym_t *name;
609+
// Not serialized. Caches the value of jl_get_binding(mod, name).
610+
jl_binding_t *bnd_cache;
611+
} jl_globalref_t;
612+
606613
// one Type-to-Value entry
607614
typedef struct _jl_typemap_entry_t {
608615
JL_DATA_TYPE
@@ -1626,6 +1633,8 @@ JL_DLLEXPORT int jl_boundp(jl_module_t *m, jl_sym_t *var);
16261633
JL_DLLEXPORT int jl_defines_or_exports_p(jl_module_t *m, jl_sym_t *var);
16271634
JL_DLLEXPORT int jl_binding_resolved_p(jl_module_t *m, jl_sym_t *var);
16281635
JL_DLLEXPORT int jl_is_const(jl_module_t *m, jl_sym_t *var);
1636+
JL_DLLEXPORT int jl_binding_is_const(jl_binding_t *b);
1637+
JL_DLLEXPORT int jl_binding_boundp(jl_binding_t *b);
16291638
JL_DLLEXPORT jl_value_t *jl_get_global(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var);
16301639
JL_DLLEXPORT void jl_set_global(jl_module_t *m JL_ROOTING_ARGUMENT, jl_sym_t *var, jl_value_t *val JL_ROOTED_ARGUMENT);
16311640
JL_DLLEXPORT void jl_set_const(jl_module_t *m JL_ROOTING_ARGUMENT, jl_sym_t *var, jl_value_t *val JL_ROOTED_ARGUMENT);

src/module.c

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -413,14 +413,17 @@ JL_DLLEXPORT jl_binding_t *jl_get_binding_or_error(jl_module_t *m, jl_sym_t *var
413413
JL_DLLEXPORT jl_value_t *jl_module_globalref(jl_module_t *m, jl_sym_t *var)
414414
{
415415
JL_LOCK(&m->lock);
416-
jl_binding_t *b = (jl_binding_t*)ptrhash_get(&m->bindings, var);
416+
jl_binding_t *b = _jl_get_module_binding(m, var);
417417
if (b == HT_NOTFOUND) {
418418
JL_UNLOCK(&m->lock);
419-
return jl_new_struct(jl_globalref_type, m, var);
419+
return jl_new_struct(jl_globalref_type, m, var, jl_box_voidpointer(NULL));
420420
}
421421
jl_value_t *globalref = jl_atomic_load_relaxed(&b->globalref);
422422
if (globalref == NULL) {
423-
jl_value_t *newref = jl_new_struct(jl_globalref_type, m, var);
423+
jl_value_t *newref = jl_new_struct(jl_globalref_type, m, var, jl_box_voidpointer(NULL));
424+
if (b->owner) {
425+
((jl_globalref_t*)newref)->bnd_cache = b->owner == m ? b : _jl_get_module_binding(b->owner, var);
426+
}
424427
if (jl_atomic_cmpswap_relaxed(&b->globalref, &globalref, newref)) {
425428
JL_GC_PROMISE_ROOTED(newref);
426429
globalref = newref;
@@ -662,12 +665,18 @@ JL_DLLEXPORT jl_binding_t *jl_get_module_binding(jl_module_t *m JL_PROPAGATES_RO
662665
return b == HT_NOTFOUND ? NULL : b;
663666
}
664667

668+
669+
JL_DLLEXPORT jl_value_t *jl_binding_value(jl_binding_t *b JL_PROPAGATES_ROOT)
670+
{
671+
return b->value;
672+
}
673+
665674
JL_DLLEXPORT jl_value_t *jl_get_global(jl_module_t *m, jl_sym_t *var)
666675
{
667676
jl_binding_t *b = jl_get_binding(m, var);
668677
if (b == NULL) return NULL;
669678
if (b->deprecated) jl_binding_deprecation_warning(m, b);
670-
return b->value;
679+
return jl_binding_value(b);
671680
}
672681

673682
JL_DLLEXPORT void jl_set_global(jl_module_t *m JL_ROOTING_ARGUMENT, jl_sym_t *var, jl_value_t *val JL_ROOTED_ARGUMENT)
@@ -696,10 +705,22 @@ JL_DLLEXPORT void jl_set_const(jl_module_t *m JL_ROOTING_ARGUMENT, jl_sym_t *var
696705
jl_symbol_name(bp->name));
697706
}
698707

708+
JL_DLLEXPORT int jl_binding_is_const(jl_binding_t *b)
709+
{
710+
assert(b);
711+
return b->constp;
712+
}
713+
714+
JL_DLLEXPORT int jl_binding_boundp(jl_binding_t *b)
715+
{
716+
assert(b);
717+
return b->value != 0;
718+
}
719+
699720
JL_DLLEXPORT int jl_is_const(jl_module_t *m, jl_sym_t *var)
700721
{
701722
jl_binding_t *b = jl_get_binding(m, var);
702-
return b && b->constp;
723+
return b && jl_binding_is_const(b);
703724
}
704725

705726
// set the deprecated flag for a binding:

0 commit comments

Comments
 (0)