Fix JuliaLang#10208 - Unnecessary boxing to call jl_object_id (JuliaL…

…ang#31771)

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.

src/builtins.c

@@ -236,7 +236,7 @@ typedef struct _varidx {
     struct _varidx *prev;
 } jl_varidx_t;
 
-static uintptr_t jl_object_id_(jl_value_t *tv, jl_value_t *v) JL_NOTSAFEPOINT;
+JL_DLLEXPORT uintptr_t jl_object_id_(jl_value_t *tv, jl_value_t *v) JL_NOTSAFEPOINT;
 
 static uintptr_t type_object_id_(jl_value_t *v, jl_varidx_t *env) JL_NOTSAFEPOINT
 {
@@ -277,7 +277,7 @@ static uintptr_t type_object_id_(jl_value_t *v, jl_varidx_t *env) JL_NOTSAFEPOIN
     return jl_object_id_((jl_value_t*)tv, v);
 }
 
-static uintptr_t jl_object_id_(jl_value_t *tv, jl_value_t *v) JL_NOTSAFEPOINT
+JL_DLLEXPORT uintptr_t jl_object_id_(jl_value_t *tv, jl_value_t *v) JL_NOTSAFEPOINT
 {
     if (tv == (jl_value_t*)jl_sym_type)
         return ((jl_sym_t*)v)->hash;

src/ccall.cpp

@@ -1713,6 +1713,27 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
         JL_GC_POP();
         return ghostValue(jl_void_type);
     }
+    else if (is_libjulia_func(jl_object_id) && nargt == 1 &&
+            rt == (jl_value_t*)jl_ulong_type) {
+        jl_cgval_t val = argv[0];
+        if (!val.isboxed) {
+            // If the value is not boxed, try to compute the object id without
+            // reboxing it.
+            auto T_pint8_derived = PointerType::get(T_int8, AddressSpace::Derived);
+            if (!val.isghost && !val.ispointer())
+                val = value_to_pointer(ctx, val);
+            Value *args[] = {
+                emit_typeof_boxed(ctx, val),
+                val.isghost ? ConstantPointerNull::get(T_pint8_derived) :
+                    ctx.builder.CreateBitCast(
+                        decay_derived(data_pointer(ctx, val)),
+                        T_pint8_derived)
+            };
+            Value *ret = ctx.builder.CreateCall(prepare_call(jl_object_id__func), makeArrayRef(args));
+            JL_GC_POP();
+            return mark_or_box_ccall_result(ctx, ret, retboxed, rt, unionall, static_rt);
+        }
+    }
 
     jl_cgval_t retval = sig.emit_a_ccall(
             ctx,

src/cgutils.cpp

@@ -1362,6 +1362,7 @@ static Value *julia_bool(jl_codectx_t &ctx, Value *cond)
 static Constant *julia_const_to_llvm(jl_value_t *e);
 static Value *data_pointer(jl_codectx_t &ctx, const jl_cgval_t &x)
 {
+    assert(x.ispointer());
     Value *data = x.V;
     if (x.constant) {
         Constant *val = julia_const_to_llvm(x.constant);

src/codegen.cpp

@@ -298,6 +298,7 @@ static Function *jl_write_barrier_func;
 static Function *jlisa_func;
 static Function *jlsubtype_func;
 static Function *jlapplytype_func;
+static Function *jl_object_id__func;
 static Function *setjmp_func;
 static Function *memcmp_derived_func;
 static Function *box_int8_func;
@@ -7434,6 +7435,15 @@ static void init_julia_llvm_env(Module *m)
     add_return_attr(jlapplytype_func, Attribute::NonNull);
     add_named_global(jlapplytype_func, &jl_instantiate_type_in_env);
 
+    std::vector<Type *> objectid__args(0);
+    objectid__args.push_back(T_prjlvalue);
+    objectid__args.push_back(T_pint8_derived);
+    jl_object_id__func =
+        Function::Create(FunctionType::get(T_size, objectid__args, false),
+                         Function::ExternalLinkage,
+                         "jl_object_id_", m);
+    add_named_global(jl_object_id__func, &jl_object_id_);
+
     std::vector<Type*> gc_alloc_args(0);
     gc_alloc_args.push_back(T_pint8);
     gc_alloc_args.push_back(T_size);

src/julia_internal.h

@@ -868,6 +868,8 @@ JL_DLLEXPORT int jl_array_isassigned(jl_array_t *a, size_t i);
 
 JL_DLLEXPORT void jl_uv_stop(uv_loop_t* loop);
 
+JL_DLLEXPORT uintptr_t jl_object_id_(jl_value_t *tv, jl_value_t *v) JL_NOTSAFEPOINT;
+
 // -- synchronization utilities -- //
 
 extern jl_mutex_t typecache_lock;

test/compiler/codegen.jl

@@ -364,3 +364,27 @@ str = String(take!(io))
 @test occursin("alias.scope", str)
 @test occursin("aliasscope", str)
 @test occursin("noalias", str)
+
+# Issue #10208 - Unnecessary boxing for calling objectid
+struct FooDictHash{T}
+    x::T
+end
+
+function f_dict_hash_alloc()
+    d = Dict{FooDictHash{Int},Int}()
+    for i in 1:10000
+        d[FooDictHash(i)] = i+1
+    end
+    d
+end
+
+function g_dict_hash_alloc()
+    d = Dict{Int,Int}()
+    for i in 1:10000
+        d[i] = i+1
+    end
+    d
+end
+# Warm up
+f_dict_hash_alloc(); g_dict_hash_alloc();
+@test (@allocated f_dict_hash_alloc()) == (@allocated g_dict_hash_alloc())