Skip to content

Commit 3923252

Browse files
committed
Fix #10208 - Unnecessary boxing to call jl_object_id
Introduce a fast path that passes the type and the data separately to jl_object_id_. Fixes the allocation performance problems noted in the issue, though the `Foo` version is still approx 4x slower, since the Int version doesn't have to go through a call to compute its hash. Fixing that is future work.
1 parent 4dd91fa commit 3923252

File tree

6 files changed

+59
-2
lines changed

6 files changed

+59
-2
lines changed

src/builtins.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ typedef struct _varidx {
236236
struct _varidx *prev;
237237
} jl_varidx_t;
238238

239-
static uintptr_t jl_object_id_(jl_value_t *tv, jl_value_t *v) JL_NOTSAFEPOINT;
239+
JL_DLLEXPORT uintptr_t jl_object_id_(jl_value_t *tv, jl_value_t *v) JL_NOTSAFEPOINT;
240240

241241
static uintptr_t type_object_id_(jl_value_t *v, jl_varidx_t *env) JL_NOTSAFEPOINT
242242
{
@@ -277,7 +277,7 @@ static uintptr_t type_object_id_(jl_value_t *v, jl_varidx_t *env) JL_NOTSAFEPOIN
277277
return jl_object_id_((jl_value_t*)tv, v);
278278
}
279279

280-
static uintptr_t jl_object_id_(jl_value_t *tv, jl_value_t *v) JL_NOTSAFEPOINT
280+
JL_DLLEXPORT uintptr_t jl_object_id_(jl_value_t *tv, jl_value_t *v) JL_NOTSAFEPOINT
281281
{
282282
if (tv == (jl_value_t*)jl_sym_type)
283283
return ((jl_sym_t*)v)->hash;

src/ccall.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1713,6 +1713,26 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
17131713
JL_GC_POP();
17141714
return ghostValue(jl_void_type);
17151715
}
1716+
else if (is_libjulia_func(jl_object_id) && nargt == 1 &&
1717+
rt == (jl_value_t*)jl_ulong_type) {
1718+
jl_cgval_t val = argv[0];
1719+
if (!val.isboxed) {
1720+
// If the value is not boxed, try to compute the object id without
1721+
// reboxing it.
1722+
auto T_pint8_derived = PointerType::get(T_int8, AddressSpace::Derived);
1723+
if (!val.isghost && !val.ispointer())
1724+
val = value_to_pointer(ctx, val);
1725+
Value *args[] = {
1726+
emit_typeof_boxed(ctx, val),
1727+
val.isghost ? ConstantPointerNull::get(T_pint8_derived) :
1728+
ctx.builder.CreateBitCast(
1729+
decay_derived(data_pointer(ctx, val)),
1730+
T_pint8_derived)
1731+
};
1732+
Value *ret = ctx.builder.CreateCall(prepare_call(jl_object_id__func), makeArrayRef(args));
1733+
return mark_or_box_ccall_result(ctx, ret, retboxed, rt, unionall, static_rt);
1734+
}
1735+
}
17161736

17171737
jl_cgval_t retval = sig.emit_a_ccall(
17181738
ctx,

src/cgutils.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1362,6 +1362,7 @@ static Value *julia_bool(jl_codectx_t &ctx, Value *cond)
13621362
static Constant *julia_const_to_llvm(jl_value_t *e);
13631363
static Value *data_pointer(jl_codectx_t &ctx, const jl_cgval_t &x)
13641364
{
1365+
assert(x.ispointer());
13651366
Value *data = x.V;
13661367
if (x.constant) {
13671368
Constant *val = julia_const_to_llvm(x.constant);

src/codegen.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,7 @@ static Function *jl_write_barrier_func;
298298
static Function *jlisa_func;
299299
static Function *jlsubtype_func;
300300
static Function *jlapplytype_func;
301+
static Function *jl_object_id__func;
301302
static Function *setjmp_func;
302303
static Function *memcmp_derived_func;
303304
static Function *box_int8_func;
@@ -7434,6 +7435,15 @@ static void init_julia_llvm_env(Module *m)
74347435
add_return_attr(jlapplytype_func, Attribute::NonNull);
74357436
add_named_global(jlapplytype_func, &jl_instantiate_type_in_env);
74367437

7438+
std::vector<Type *> objectid__args(0);
7439+
objectid__args.push_back(T_prjlvalue);
7440+
objectid__args.push_back(T_pint8_derived);
7441+
jl_object_id__func =
7442+
Function::Create(FunctionType::get(T_size, objectid__args, false),
7443+
Function::ExternalLinkage,
7444+
"jl_object_id_", m);
7445+
add_named_global(jl_object_id__func, &jl_object_id_);
7446+
74377447
std::vector<Type*> gc_alloc_args(0);
74387448
gc_alloc_args.push_back(T_pint8);
74397449
gc_alloc_args.push_back(T_size);

src/julia_internal.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -868,6 +868,8 @@ JL_DLLEXPORT int jl_array_isassigned(jl_array_t *a, size_t i);
868868

869869
JL_DLLEXPORT void jl_uv_stop(uv_loop_t* loop);
870870

871+
JL_DLLEXPORT uintptr_t jl_object_id_(jl_value_t *tv, jl_value_t *v) JL_NOTSAFEPOINT;
872+
871873
// -- synchronization utilities -- //
872874

873875
extern jl_mutex_t typecache_lock;

test/compiler/codegen.jl

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,3 +364,27 @@ str = String(take!(io))
364364
@test occursin("alias.scope", str)
365365
@test occursin("aliasscope", str)
366366
@test occursin("noalias", str)
367+
368+
# Issue #10208 - Unnecessary boxing for calling objectid
369+
struct FooDictHash{T}
370+
x::T
371+
end
372+
373+
function f_dict_hash_alloc()
374+
d = Dict{FooDictHash{Int},Int}()
375+
for i in 1:10000
376+
d[FooDictHash(i)] = i+1
377+
end
378+
d
379+
end
380+
381+
function g_dict_hash_alloc()
382+
d = Dict{Int,Int}()
383+
for i in 1:10000
384+
d[i] = i+1
385+
end
386+
d
387+
end
388+
# Warm up
389+
f_dict_hash_alloc(); g_dict_hash_alloc();
390+
@test (@allocated f_dict_hash_alloc()) == (@allocated g_dict_hash_alloc())

0 commit comments

Comments
 (0)