Skip to content

Commit

Permalink
improve performance of disabling finalizers in locks
Browse files Browse the repository at this point in the history
helps #38947
  • Loading branch information
JeffBezanson committed Jan 15, 2021
1 parent f813257 commit 58d5b00
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 21 deletions.
15 changes: 14 additions & 1 deletion base/gcutils.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions base/lock.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -135,7 +135,7 @@ function unlock(rl::ReentrantLock)
rethrow()
end
end
GC.enable_finalizers(true)
GC.enable_finalizers()
unlock(rl.cond_wait)
end
return
Expand All @@ -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
Expand Down
10 changes: 5 additions & 5 deletions base/locks-mt.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
22 changes: 22 additions & 0 deletions src/ccall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
36 changes: 32 additions & 4 deletions src/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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);
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/julia_threads.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
5 changes: 2 additions & 3 deletions src/locks.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down
6 changes: 2 additions & 4 deletions src/rtutils.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down

0 comments on commit 58d5b00

Please sign in to comment.