From c2468c49445c0fa029756e27fea01dcc1e701698 Mon Sep 17 00:00:00 2001 From: Jeff Bezanson Date: Tue, 19 Jan 2021 11:47:11 -0500 Subject: [PATCH] improve performance of disabling finalizers in locks (#39153) helps #38947 --- base/gcutils.jl | 15 ++++++++++++++- base/lock.jl | 8 ++++---- base/locks-mt.jl | 10 +++++----- src/ccall.cpp | 22 ++++++++++++++++++++++ src/gc.c | 36 ++++++++++++++++++++++++++++++++---- src/julia_threads.h | 4 ++++ src/locks.h | 5 ++--- src/rtutils.c | 6 ++---- 8 files changed, 85 insertions(+), 21 deletions(-) diff --git a/base/gcutils.jl b/base/gcutils.jl index 82bdf74c9a4d4..1280b4ab71afc 100644 --- a/base/gcutils.jl +++ b/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 diff --git a/base/lock.jl b/base/lock.jl index 7a8e0eaef878e..b013a593cde84 100644 --- a/base/lock.jl +++ b/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 diff --git a/base/locks-mt.jl b/base/locks-mt.jl index 6a3b68016cb81..41e1ef33c574c 100644 --- a/base/locks-mt.jl +++ b/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,12 +76,12 @@ 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 @@ -89,7 +89,7 @@ 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 diff --git a/src/ccall.cpp b/src/ccall.cpp index ed8f98d96df2c..65c0fa74c9ace 100644 --- a/src/ccall.cpp +++ b/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); diff --git a/src/gc.c b/src/gc.c index ef1ef52da26d4..94f5a80fd0cbe 100644 --- a/src/gc.c +++ b/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,12 +388,24 @@ 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) @@ -399,6 +413,22 @@ JL_DLLEXPORT int jl_gc_get_finalizers_inhibited(jl_ptls_t ptls) 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); } } diff --git a/src/julia_threads.h b/src/julia_threads.h index 4216b2012cedb..350fe9133e242 100644 --- a/src/julia_threads.h +++ b/src/julia_threads.h @@ -341,6 +341,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); diff --git a/src/locks.h b/src/locks.h index b02fc8c5011e5..262390bb718f3 100644 --- a/src/locks.h +++ b/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 } } diff --git a/src/rtutils.c b/src/rtutils.c index 09ba2aed85075..99fce51128345 100644 --- a/src/rtutils.c +++ b/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); } }