Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Revert "improve performance of disabling finalizers in locks" #39334

Closed
wants to merge 1 commit into from
Closed

Revert "improve performance of disabling finalizers in locks" #39334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 1 addition & 14 deletions base/gcutils.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,20 +114,7 @@ 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) = 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
enable_finalizers(on::Bool) = ccall(:jl_gc_enable_finalizers, Cvoid, (Ptr{Cvoid}, Int32,), C_NULL, on)

"""
GC.@preserve x1 x2 ... xn expr
Expand Down
8 changes: 4 additions & 4 deletions base/lock.jl
View file
Open in desktop
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.disable_finalizers()
GC.enable_finalizers(false)
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.disable_finalizers()
GC.enable_finalizers(false)
break
end
try
Expand Down Expand Up @@ -135,7 +135,7 @@ function unlock(rl::ReentrantLock)
rethrow()
end
end
GC.enable_finalizers()
GC.enable_finalizers(true)
unlock(rl.cond_wait)
end
return
Expand All @@ -157,7 +157,7 @@ function unlockall(rl::ReentrantLock)
rethrow()
end
end
GC.enable_finalizers()
GC.enable_finalizers(true)
unlock(rl.cond_wait)
return n
end
Expand Down
10 changes: 5 additions & 5 deletions base/locks-mt.jl
View file
Open in desktop
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.disable_finalizers()
GC.enable_finalizers(false)
p = _xchg!(l, 1)
if p == 0
return
end
GC.enable_finalizers()
GC.enable_finalizers(true)
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.disable_finalizers()
GC.enable_finalizers(false)
p = _xchg!(l, 1)
if p == 0
return true
end
GC.enable_finalizers()
GC.enable_finalizers(true)
end
return false
end

function unlock(l::SpinLock)
_get(l) == 0 && error("unlock count must match lock count")
_set!(l, 0)
GC.enable_finalizers()
GC.enable_finalizers(true)
ccall(:jl_cpu_wake, Cvoid, ())
return
end
Expand Down
22 changes: 0 additions & 22 deletions src/ccall.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1481,28 +1481,6 @@ 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: 4 additions & 32 deletions src/gc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,6 @@ 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 @@ -262,7 +261,6 @@ 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 @@ -388,47 +386,19 @@ 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 @@ -451,8 +421,10 @@ JL_DLLEXPORT void jl_gc_enable_finalizers(jl_ptls_t ptls, int on)
return;
}
ptls->finalizers_inhibited = new_val;
if (jl_gc_have_pending_finalizers) {
jl_gc_run_pending_finalizers(ptls);
if (!new_val && old_val && !ptls->in_finalizer && ptls->locks.len == 0) {
ptls->in_finalizer = 1;
run_finalizers(ptls);
ptls->in_finalizer = 0;
}
}

Expand Down
4 changes: 0 additions & 4 deletions src/julia_threads.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,10 +341,6 @@ 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: 3 additions & 2 deletions src/locks.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,9 @@ static inline void jl_mutex_unlock(jl_mutex_t *lock)
jl_mutex_unlock_nogc(lock);
jl_lock_frame_pop();
JL_SIGATOMIC_END();
if (jl_gc_have_pending_finalizers) {
jl_gc_run_pending_finalizers(ptls); // may GC
if (ptls->locks.len == 0 && ptls->finalizers_inhibited == 0) {
ptls->finalizers_inhibited = 1;
jl_gc_enable_finalizers(ptls, 1); // call run_finalizers (may GC)
}
}

Expand Down
6 changes: 4 additions & 2 deletions src/rtutils.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,8 +265,10 @@ JL_DLLEXPORT void jl_eh_restore_state(jl_handler_t *eh)
if (old_defer_signal && !eh->defer_signal) {
jl_sigint_safepoint(ptls);
}
if (jl_gc_have_pending_finalizers && unlocks && eh->locks_len == 0) {
jl_gc_run_pending_finalizers(ptls);
if (unlocks && eh->locks_len == 0 && ptls->finalizers_inhibited == 0) {
// call run_finalizers
ptls->finalizers_inhibited = 1;
jl_gc_enable_finalizers(ptls, 1);
}
}

Expand Down