Skip to content

Commit ec102eb

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 1916d35 commit ec102eb

14 files changed

+90
-25
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
@@ -1986,7 +1986,7 @@ function abstract_eval_special_value(interp::AbstractInterpreter, @nospecialize(
19861986
return sv.argtypes[e.n]
19871987
end
19881988
elseif isa(e, GlobalRef)
1989-
return abstract_eval_global(interp, e.mod, e.name, sv)
1989+
return abstract_eval_globalref(interp, e, sv)
19901990
end
19911991

19921992
return Const(e)
@@ -2260,17 +2260,24 @@ function abstract_eval_statement(interp::AbstractInterpreter, @nospecialize(e),
22602260
return rt
22612261
end
22622262

2263-
function abstract_eval_global(M::Module, s::Symbol)
2264-
if isdefined(M, s) && isconst(M, s)
2265-
return Const(getglobal(M, s))
2263+
function isdefined_globalref(g::GlobalRef)
2264+
g.binding != C_NULL && return ccall(:jl_binding_boundp, Cint, (Ptr{Cvoid},), g.binding) != 0
2265+
return isdefined(g.mod, g.name)
2266+
end
2267+
2268+
function abstract_eval_globalref(g::GlobalRef)
2269+
if isdefined_globalref(g) && isconst(g)
2270+
g.binding != C_NULL && return Const(ccall(:jl_binding_value, Any, (Ptr{Cvoid},), g.binding))
2271+
return Const(getglobal(g.mod, g.name))
22662272
end
2267-
ty = ccall(:jl_binding_type, Any, (Any, Any), M, s)
2273+
ty = ccall(:jl_binding_type, Any, (Any, Any), g.mod, g.name)
22682274
ty === nothing && return Any
22692275
return ty
22702276
end
2277+
abstract_eval_global(M::Module, s::Symbol) = abstract_eval_globalref(GlobalRef(M, s))
22712278

2272-
function abstract_eval_global(interp::AbstractInterpreter, M::Module, s::Symbol, frame::Union{InferenceState, IRCode})
2273-
rt = abstract_eval_global(M, s)
2279+
function abstract_eval_globalref(interp::AbstractInterpreter, g::GlobalRef, frame::Union{InferenceState, IRCode})
2280+
rt = abstract_eval_globalref(g)
22742281
consistent = inaccessiblememonly = ALWAYS_FALSE
22752282
nothrow = false
22762283
if isa(rt, Const)
@@ -2281,7 +2288,7 @@ function abstract_eval_global(interp::AbstractInterpreter, M::Module, s::Symbol,
22812288
else
22822289
nothrow = true
22832290
end
2284-
elseif isdefined(M,s)
2291+
elseif isdefined_globalref(g)
22852292
nothrow = true
22862293
end
22872294
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
@@ -336,7 +336,7 @@ function argextype(
336336
elseif isa(x, QuoteNode)
337337
return Const(x.value)
338338
elseif isa(x, GlobalRef)
339-
return abstract_eval_global(x.mod, x.name)
339+
return abstract_eval_globalref(x)
340340
elseif isa(x, PhiNode)
341341
return Any
342342
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/essentials.jl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ length(a::Array) = arraylen(a)
1313
eval(:(getindex(A::Array, i1::Int) = arrayref($(Expr(:boundscheck)), A, i1)))
1414
eval(:(getindex(A::Array, i1::Int, i2::Int, I::Int...) = (@inline; arrayref($(Expr(:boundscheck)), A, i1, i2, I...))))
1515

16+
==(a::GlobalRef, b::GlobalRef) = a.mod === b.mod && a.name === b.name
17+
1618
"""
1719
AbstractSet{T}
1820

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

base/show.jl

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1838,9 +1838,10 @@ function allow_macroname(ex)
18381838
end
18391839
end
18401840

1841-
function is_core_macro(arg, macro_name::AbstractString)
1842-
arg === GlobalRef(Core, Symbol(macro_name))
1841+
function is_core_macro(arg::GlobalRef, macro_name::AbstractString)
1842+
arg == GlobalRef(Core, Symbol(macro_name))
18431843
end
1844+
is_core_macro(@nospecialize(arg), macro_name::AbstractString) = false
18441845

18451846
# symbol for IOContext flag signaling whether "begin" is treated
18461847
# as an ordinary symbol, which is true in indexing expressions.

src/clangsa/GCChecker.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -745,6 +745,7 @@ bool GCChecker::isGCTrackedType(QualType QT) {
745745
Name.endswith_lower("jl_method_match_t") ||
746746
Name.endswith_lower("jl_vararg_t") ||
747747
Name.endswith_lower("jl_opaque_closure_t") ||
748+
Name.endswith_lower("jl_globalref_t") ||
748749
// Probably not technically true for these, but let's allow it
749750
Name.endswith_lower("typemap_intersection_env") ||
750751
Name.endswith_lower("interpreter_state") ||

src/dump.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2084,6 +2084,10 @@ 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+
jl_binding_t *b = jl_get_binding(r->mod, r->name);
2090+
r->bnd_cache = b->value ? b : NULL;
20872091
}
20882092
}
20892093

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: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -410,17 +410,29 @@ JL_DLLEXPORT jl_binding_t *jl_get_binding_or_error(jl_module_t *m, jl_sym_t *var
410410
return b;
411411
}
412412

413+
JL_DLLEXPORT jl_globalref_t *jl_new_globalref(jl_module_t *mod, jl_sym_t *name, jl_binding_t *b)
414+
{
415+
jl_task_t *ct = jl_current_task;
416+
jl_globalref_t *g = (jl_globalref_t *)jl_gc_alloc(ct->ptls, sizeof(jl_globalref_t), jl_globalref_type);
417+
g->mod = mod;
418+
jl_gc_wb(g, g->mod);
419+
g->name = name;
420+
g->bnd_cache = b;
421+
return g;
422+
}
423+
413424
JL_DLLEXPORT jl_value_t *jl_module_globalref(jl_module_t *m, jl_sym_t *var)
414425
{
415426
JL_LOCK(&m->lock);
416-
jl_binding_t *b = (jl_binding_t*)ptrhash_get(&m->bindings, var);
427+
jl_binding_t *b = _jl_get_module_binding(m, var);
417428
if (b == HT_NOTFOUND) {
418429
JL_UNLOCK(&m->lock);
419-
return jl_new_struct(jl_globalref_type, m, var);
430+
return (jl_value_t *)jl_new_globalref(m, var, NULL);
420431
}
421432
jl_value_t *globalref = jl_atomic_load_relaxed(&b->globalref);
422433
if (globalref == NULL) {
423-
jl_value_t *newref = jl_new_struct(jl_globalref_type, m, var);
434+
jl_value_t *newref = (jl_value_t *)jl_new_globalref(m, var,
435+
!b->owner ? NULL : b->owner == m ? b : _jl_get_module_binding(b->owner, b->name));
424436
if (jl_atomic_cmpswap_relaxed(&b->globalref, &globalref, newref)) {
425437
JL_GC_PROMISE_ROOTED(newref);
426438
globalref = newref;
@@ -662,12 +674,18 @@ JL_DLLEXPORT jl_binding_t *jl_get_module_binding(jl_module_t *m JL_PROPAGATES_RO
662674
return b == HT_NOTFOUND ? NULL : b;
663675
}
664676

677+
678+
JL_DLLEXPORT jl_value_t *jl_binding_value(jl_binding_t *b JL_PROPAGATES_ROOT)
679+
{
680+
return b->value;
681+
}
682+
665683
JL_DLLEXPORT jl_value_t *jl_get_global(jl_module_t *m, jl_sym_t *var)
666684
{
667685
jl_binding_t *b = jl_get_binding(m, var);
668686
if (b == NULL) return NULL;
669687
if (b->deprecated) jl_binding_deprecation_warning(m, b);
670-
return b->value;
688+
return jl_binding_value(b);
671689
}
672690

673691
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 +714,22 @@ JL_DLLEXPORT void jl_set_const(jl_module_t *m JL_ROOTING_ARGUMENT, jl_sym_t *var
696714
jl_symbol_name(bp->name));
697715
}
698716

717+
JL_DLLEXPORT int jl_binding_is_const(jl_binding_t *b)
718+
{
719+
assert(b);
720+
return b->constp;
721+
}
722+
723+
JL_DLLEXPORT int jl_binding_boundp(jl_binding_t *b)
724+
{
725+
assert(b);
726+
return b->value != 0;
727+
}
728+
699729
JL_DLLEXPORT int jl_is_const(jl_module_t *m, jl_sym_t *var)
700730
{
701731
jl_binding_t *b = jl_get_binding(m, var);
702-
return b && b->constp;
732+
return b && jl_binding_is_const(b);
703733
}
704734

705735
// set the deprecated flag for a binding:

src/staticdata.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -576,6 +576,10 @@ static void jl_serialize_value__(jl_serializer_state *s, jl_value_t *v, int recu
576576
jl_serialize_value(s, tn->partial);
577577
}
578578
else if (t->layout->nfields > 0) {
579+
if (jl_typeis(v, jl_globalref_type)) {
580+
// Don't save the cached binding reference in staticdata
581+
((jl_globalref_t*)v)->bnd_cache = NULL;
582+
}
579583
char *data = (char*)jl_data_ptr(v);
580584
size_t i, np = t->layout->npointers;
581585
for (i = 0; i < np; i++) {

0 commit comments

Comments
 (0)