improve performance of disabling finalizers in locks (#39153)

helps #38947

(cherry picked from commit e8f23d7)

Author: JeffBezanson

base/gcutils.jl

@@ -114,7 +114,20 @@ the current Task. Finalizers will only run when the counter is at zero. (Set
 `true` for enabling, `false` for disabling). They may still run concurrently on
 another Task or thread.
 """
-enable_finalizers(on::Bool) = ccall(:jl_gc_enable_finalizers, Cvoid, (Ptr{Cvoid}, Int32,), C_NULL, on)
+enable_finalizers(on::Bool) = on ? enable_finalizers() : disable_finalizers()
+
+function enable_finalizers()
+    Base.@_inline_meta
+    ccall(:jl_gc_enable_finalizers_internal, Cvoid, ())
+    if unsafe_load(cglobal(:jl_gc_have_pending_finalizers, Cint)) != 0
+        ccall(:jl_gc_run_pending_finalizers, Cvoid, (Ptr{Cvoid},), C_NULL)
+    end
+end
+
+function disable_finalizers()
+    Base.@_inline_meta
+    ccall(:jl_gc_disable_finalizers_internal, Cvoid, ())
+end
 
 """
     GC.@preserve x1 x2 ... xn expr

base/lock.jl

@@ -65,7 +65,7 @@ function trylock(rl::ReentrantLock)
     if rl.reentrancy_cnt == 0
         rl.locked_by = t
         rl.reentrancy_cnt = 1
-        GC.enable_finalizers(false)
+        GC.disable_finalizers()
         got = true
     else
         got = false
@@ -93,7 +93,7 @@ function lock(rl::ReentrantLock)
             if rl.reentrancy_cnt == 0
                 rl.locked_by = t
                 rl.reentrancy_cnt = 1
-                GC.enable_finalizers(false)
+                GC.disable_finalizers()
                 break
             end
             try
@@ -135,7 +135,7 @@ function unlock(rl::ReentrantLock)
                 rethrow()
             end
         end
-        GC.enable_finalizers(true)
+        GC.enable_finalizers()
         unlock(rl.cond_wait)
     end
     return
@@ -157,7 +157,7 @@ function unlockall(rl::ReentrantLock)
             rethrow()
         end
     end
-    GC.enable_finalizers(true)
+    GC.enable_finalizers()
     unlock(rl.cond_wait)
     return n
 end

base/locks-mt.jl

@@ -61,12 +61,12 @@ Base.assert_havelock(l::SpinLock) = islocked(l) ? nothing : Base.concurrency_vio
 function lock(l::SpinLock)
     while true
         if _get(l) == 0
-            GC.enable_finalizers(false)
+            GC.disable_finalizers()
             p = _xchg!(l, 1)
             if p == 0
                 return
             end
-            GC.enable_finalizers(true)
+            GC.enable_finalizers()
         end
         ccall(:jl_cpu_pause, Cvoid, ())
         # Temporary solution before we have gc transition support in codegen.
@@ -76,20 +76,20 @@ end
 
 function trylock(l::SpinLock)
     if _get(l) == 0
-        GC.enable_finalizers(false)
+        GC.disable_finalizers()
         p = _xchg!(l, 1)
         if p == 0
             return true
         end
-        GC.enable_finalizers(true)
+        GC.enable_finalizers()
     end
     return false
 end
 
 function unlock(l::SpinLock)
     _get(l) == 0 && error("unlock count must match lock count")
     _set!(l, 0)
-    GC.enable_finalizers(true)
+    GC.enable_finalizers()
     ccall(:jl_cpu_wake, Cvoid, ())
     return
 end

src/ccall.cpp

@@ -1481,6 +1481,28 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
         tbaa_decorate(tbaa_const, tid);
         return mark_or_box_ccall_result(ctx, tid, retboxed, rt, unionall, static_rt);
     }
+    else if (is_libjulia_func(jl_gc_disable_finalizers_internal)
+#ifdef NDEBUG
+             || is_libjulia_func(jl_gc_enable_finalizers_internal)
+#endif
+             ) {
+        JL_GC_POP();
+        Value *ptls_i32 = emit_bitcast(ctx, ctx.ptlsStates, T_pint32);
+        const int finh_offset = offsetof(jl_tls_states_t, finalizers_inhibited);
+        Value *pfinh = ctx.builder.CreateInBoundsGEP(ptls_i32, ConstantInt::get(T_size, finh_offset / 4));
+        LoadInst *finh = ctx.builder.CreateAlignedLoad(pfinh, Align(sizeof(int32_t)));
+        Value *newval;
+        if (is_libjulia_func(jl_gc_disable_finalizers_internal)) {
+            newval = ctx.builder.CreateAdd(finh, ConstantInt::get(T_int32, 1));
+        }
+        else {
+            newval = ctx.builder.CreateSelect(ctx.builder.CreateICmpEQ(finh, ConstantInt::get(T_int32, 0)),
+                                              ConstantInt::get(T_int32, 0),
+                                              ctx.builder.CreateSub(finh, ConstantInt::get(T_int32, 1)));
+        }
+        ctx.builder.CreateStore(newval, pfinh);
+        return ghostValue(jl_nothing_type);
+    }
     else if (is_libjulia_func(jl_get_current_task)) {
         assert(lrt == T_prjlvalue);
         assert(!isVa && !llvmcall && nccallargs == 0);

src/gc.c

@@ -181,6 +181,7 @@ bigval_t *big_objects_marked = NULL;
 // `to_finalize` should not have tagged pointers.
 arraylist_t finalizer_list_marked;
 arraylist_t to_finalize;
+int jl_gc_have_pending_finalizers = 0;
 
 NOINLINE uintptr_t gc_get_stack_ptr(void)
 {
@@ -261,6 +262,7 @@ static void schedule_finalization(void *o, void *f) JL_NOTSAFEPOINT
 {
     arraylist_push(&to_finalize, o);
     arraylist_push(&to_finalize, f);
+    jl_gc_have_pending_finalizers = 1;
 }
 
 static void run_finalizer(jl_ptls_t ptls, jl_value_t *o, jl_value_t *ff)
@@ -386,19 +388,47 @@ static void run_finalizers(jl_ptls_t ptls)
     if (to_finalize.items == to_finalize._space) {
         copied_list.items = copied_list._space;
     }
+    jl_gc_have_pending_finalizers = 0;
     arraylist_new(&to_finalize, 0);
     // This releases the finalizers lock.
     jl_gc_run_finalizers_in_list(ptls, &copied_list);
     arraylist_free(&copied_list);
 }
 
+JL_DLLEXPORT void jl_gc_run_pending_finalizers(jl_ptls_t ptls)
+{
+    if (ptls == NULL)
+        ptls = jl_get_ptls_states();
+    if (!ptls->in_finalizer && ptls->locks.len == 0 && ptls->finalizers_inhibited == 0) {
+        ptls->in_finalizer = 1;
+        run_finalizers(ptls);
+        ptls->in_finalizer = 0;
+    }
+}
+
 JL_DLLEXPORT int jl_gc_get_finalizers_inhibited(jl_ptls_t ptls)
 {
     if (ptls == NULL)
         ptls = jl_get_ptls_states();
     return ptls->finalizers_inhibited;
 }
 
+JL_DLLEXPORT void jl_gc_disable_finalizers_internal(void)
+{
+    jl_ptls_t ptls = jl_get_ptls_states();
+    ptls->finalizers_inhibited++;
+}
+
+JL_DLLEXPORT void jl_gc_enable_finalizers_internal(void)
+{
+    jl_ptls_t ptls = jl_get_ptls_states();
+#ifdef NDEBUG
+    ptls->finalizers_inhibited--;
+#else
+    jl_gc_enable_finalizers(ptls, 1);
+#endif
+}
+
 JL_DLLEXPORT void jl_gc_enable_finalizers(jl_ptls_t ptls, int on)
 {
     if (ptls == NULL)
@@ -421,10 +451,8 @@ JL_DLLEXPORT void jl_gc_enable_finalizers(jl_ptls_t ptls, int on)
         return;
     }
     ptls->finalizers_inhibited = new_val;
-    if (!new_val && old_val && !ptls->in_finalizer && ptls->locks.len == 0) {
-        ptls->in_finalizer = 1;
-        run_finalizers(ptls);
-        ptls->in_finalizer = 0;
+    if (jl_gc_have_pending_finalizers) {
+        jl_gc_run_pending_finalizers(ptls);
     }
 }
 

src/julia_threads.h

@@ -332,6 +332,10 @@ int8_t jl_gc_safe_leave(jl_ptls_t ptls, int8_t state); // Can be a safepoint
 JL_DLLEXPORT void (jl_gc_safepoint)(void);
 
 JL_DLLEXPORT void jl_gc_enable_finalizers(jl_ptls_t ptls, int on);
+JL_DLLEXPORT void jl_gc_disable_finalizers_internal(void);
+JL_DLLEXPORT void jl_gc_enable_finalizers_internal(void);
+JL_DLLEXPORT void jl_gc_run_pending_finalizers(jl_ptls_t ptls);
+extern JL_DLLEXPORT int jl_gc_have_pending_finalizers;
 
 JL_DLLEXPORT void jl_wakeup_thread(int16_t tid);
 

src/locks.h

@@ -132,9 +132,8 @@ static inline void jl_mutex_unlock(jl_mutex_t *lock)
     jl_mutex_unlock_nogc(lock);
     jl_lock_frame_pop();
     JL_SIGATOMIC_END();
-    if (ptls->locks.len == 0 && ptls->finalizers_inhibited == 0) {
-        ptls->finalizers_inhibited = 1;
-        jl_gc_enable_finalizers(ptls, 1); // call run_finalizers (may GC)
+    if (jl_gc_have_pending_finalizers) {
+        jl_gc_run_pending_finalizers(ptls); // may GC
     }
 }
 

src/rtutils.c

@@ -265,10 +265,8 @@ JL_DLLEXPORT void jl_eh_restore_state(jl_handler_t *eh)
     if (old_defer_signal && !eh->defer_signal) {
         jl_sigint_safepoint(ptls);
     }
-    if (unlocks && eh->locks_len == 0 && ptls->finalizers_inhibited == 0) {
-        // call run_finalizers
-        ptls->finalizers_inhibited = 1;
-        jl_gc_enable_finalizers(ptls, 1);
+    if (jl_gc_have_pending_finalizers && unlocks && eh->locks_len == 0) {
+        jl_gc_run_pending_finalizers(ptls);
     }
 }