From c5ab7c9c3d0715e86777ddda0235ca8fbabe2bf2 Mon Sep 17 00:00:00 2001 From: Diogo Netto <61364108+d-netto@users.noreply.github.com> Date: Thu, 12 Sep 2024 05:56:19 -0300 Subject: [PATCH] [release-1.10] dont reset maxsize in jl_array_to_string (#55689) Let's change `jl_array_to_string` so that we make the consequences of calling it in a thread-unsafe way less disastrous (i.e. let's avoid corrupting GC internal metrics). Strictly speaking, this is not a bug-fix because calling `jl_array_to_string` concurrently from two threads is UB in any case. To see how a race here may lead to negative `live_bytes`, consider this MWE from @NHDaly: - 1.10: ```Julia julia> GC.gc(true); Base.gc_live_bytes() 1842370 julia> g_vecs = Any[ UInt8['a' for _ in 1:1000000000] for _ in 1:10 ]; julia> GC.gc(true); Base.gc_live_bytes() 10001774906 julia> Threads.@threads for _ in 1:1000 for v in g_vecs String(v) end end julia> GC.gc(true); Base.gc_live_bytes() -1997600207 ``` - This patch: ```Julia julia> GC.gc(true); Base.gc_live_bytes() 1862440 julia> g_vecs = Any[ UInt8['a' for _ in 1:1000000000] for _ in 1:10 ]; julia> GC.gc(true); Base.gc_live_bytes() 10001796440 julia> Threads.@threads for _ in 1:1000 for v in g_vecs String(v) end end julia> GC.gc(true); Base.gc_live_bytes() 10002390952 ``` --- src/array.c | 2 -- src/gc.c | 7 ------- src/julia_internal.h | 1 - 3 files changed, 10 deletions(-) diff --git a/src/array.c b/src/array.c index c7f71c9695e87..47a4d28e10f76 100644 --- a/src/array.c +++ b/src/array.c @@ -479,10 +479,8 @@ JL_DLLEXPORT jl_value_t *jl_array_to_string(jl_array_t *a) return o; } } - jl_gc_count_freed(jl_array_nbytes(a)); a->nrows = 0; a->length = 0; - a->maxsize = 0; return jl_pchar_to_string((const char*)jl_array_data(a), len); } diff --git a/src/gc.c b/src/gc.c index 98a36992d70ea..2f9dec27e045a 100644 --- a/src/gc.c +++ b/src/gc.c @@ -1128,13 +1128,6 @@ void jl_gc_count_allocd(size_t sz) JL_NOTSAFEPOINT jl_atomic_load_relaxed(&ptls->gc_num.allocd) + sz); } -void jl_gc_count_freed(size_t sz) JL_NOTSAFEPOINT -{ - jl_ptls_t ptls = jl_current_task->ptls; - jl_atomic_store_relaxed(&ptls->gc_num.freed, - jl_atomic_load_relaxed(&ptls->gc_num.freed) + sz); -} - static void combine_thread_gc_counts(jl_gc_num_t *dest) JL_NOTSAFEPOINT { int gc_n_threads; diff --git a/src/julia_internal.h b/src/julia_internal.h index 07a1ab628fadf..45193782f009f 100644 --- a/src/julia_internal.h +++ b/src/julia_internal.h @@ -573,7 +573,6 @@ JL_DLLEXPORT int64_t jl_gc_sync_total_bytes(int64_t offset) JL_NOTSAFEPOINT; void jl_gc_track_malloced_array(jl_ptls_t ptls, jl_array_t *a) JL_NOTSAFEPOINT; size_t jl_array_nbytes(jl_array_t *a) JL_NOTSAFEPOINT; void jl_gc_count_allocd(size_t sz) JL_NOTSAFEPOINT; -void jl_gc_count_freed(size_t sz) JL_NOTSAFEPOINT; void jl_gc_run_all_finalizers(jl_task_t *ct); void jl_release_task_stack(jl_ptls_t ptls, jl_task_t *task); void jl_gc_add_finalizer_(jl_ptls_t ptls, void *v, void *f) JL_NOTSAFEPOINT;