From 5163fe718b8b0908029c2c06d021a5ccdb57c860 Mon Sep 17 00:00:00 2001 From: Diogo Netto <61364108+d-netto@users.noreply.github.com> Date: Sat, 29 Jun 2024 15:24:32 -0300 Subject: [PATCH 1/4] address a TODO in gc_sweep_page (i.e. save an unnecessary fetch-add) (#54976) --- src/gc.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/gc.c b/src/gc.c index 3411575dd4c16..a1d9cc9e8132f 100644 --- a/src/gc.c +++ b/src/gc.c @@ -1528,9 +1528,16 @@ static void gc_sweep_page(gc_page_profiler_serializer_t *s, jl_gc_pool_t *p, jl_ gc_page_profile_write_to_file(s); gc_update_page_fragmentation_data(pg); gc_time_count_page(freedall, pg_skpd); - jl_ptls_t ptls = gc_all_tls_states[pg->thread_n]; - jl_atomic_fetch_add(&ptls->gc_num.pool_live_bytes, GC_PAGE_SZ - GC_PAGE_OFFSET - nfree * osize); - jl_atomic_fetch_add((_Atomic(int64_t) *)&gc_num.freed, (nfree - old_nfree) * osize); + jl_ptls_t ptls = jl_current_task->ptls; + // Note that we aggregate the `pool_live_bytes` over all threads before returning this + // value to the user. It doesn't matter how the `pool_live_bytes` are partitioned among + // the threads as long as the sum is correct. Let's add the `pool_live_bytes` to the current thread + // instead of adding it to the thread that originally allocated the page, so we can avoid + // an atomic-fetch-add here. + size_t delta = (GC_PAGE_SZ - GC_PAGE_OFFSET - nfree * osize); + jl_atomic_store_relaxed(&ptls->gc_num.pool_live_bytes, + jl_atomic_load_relaxed(&ptls->gc_num.pool_live_bytes) + delta); + jl_atomic_fetch_add_relaxed((_Atomic(int64_t) *)&gc_num.freed, (nfree - old_nfree) * osize); } // the actual sweeping over all allocated pages in a memory pool From 02745186530b0f8226bcf6c4021334ecf4369875 Mon Sep 17 00:00:00 2001 From: Diogo Netto <61364108+d-netto@users.noreply.github.com> Date: Mon, 1 Jul 2024 16:28:23 -0300 Subject: [PATCH 2/4] NFC: create an actual set of functions to manipulate GC thread ids (#54984) Also adds a bunch of integrity constraint checks to ensure we don't repeat the bug from https://github.com/JuliaLang/julia/pull/54645. --- src/gc.c | 52 +++++++++++++++++++++++++++++---------------- src/gc.h | 48 +++++++++++++++++++++++++++++++++++++++++ src/julia_threads.h | 6 ++++++ src/partr.c | 13 ++++++------ 4 files changed, 95 insertions(+), 24 deletions(-) diff --git a/src/gc.c b/src/gc.c index a1d9cc9e8132f..42d682b496e98 100644 --- a/src/gc.c +++ b/src/gc.c @@ -1639,9 +1639,11 @@ void gc_sweep_wake_all(jl_ptls_t ptls, jl_gc_padded_page_stack_t *new_gc_allocd_ if (parallel_sweep_worthwhile && !page_profile_enabled) { jl_atomic_store(&gc_allocd_scratch, new_gc_allocd_scratch); uv_mutex_lock(&gc_threads_lock); - for (int i = gc_first_tid; i < gc_first_tid + jl_n_markthreads; i++) { + int first = gc_first_parallel_collector_thread_id(); + int last = gc_last_parallel_collector_thread_id(); + for (int i = first; i <= last; i++) { jl_ptls_t ptls2 = gc_all_tls_states[i]; - assert(ptls2 != NULL); // should be a GC thread + gc_check_ptls_of_parallel_collector_thread(ptls2); jl_atomic_fetch_add(&ptls2->gc_sweeps_requested, 1); } uv_cond_broadcast(&gc_threads_cond); @@ -1653,9 +1655,11 @@ void gc_sweep_wake_all(jl_ptls_t ptls, jl_gc_padded_page_stack_t *new_gc_allocd_ // collecting a page profile. // wait for all to leave in order to ensure that a straggler doesn't // try to enter sweeping after we set `gc_allocd_scratch` below. - for (int i = gc_first_tid; i < gc_first_tid + jl_n_markthreads; i++) { + int first = gc_first_parallel_collector_thread_id(); + int last = gc_last_parallel_collector_thread_id(); + for (int i = first; i <= last; i++) { jl_ptls_t ptls2 = gc_all_tls_states[i]; - assert(ptls2 != NULL); // should be a GC thread + gc_check_ptls_of_parallel_collector_thread(ptls2); while (jl_atomic_load_acquire(&ptls2->gc_sweeps_requested) != 0) { jl_cpu_pause(); } @@ -3006,10 +3010,14 @@ void gc_mark_and_steal(jl_ptls_t ptls) // since we know chunks will likely expand into a lot // of work for the mark loop steal : { + int first = gc_first_parallel_collector_thread_id(); + int last = gc_last_parallel_collector_thread_id(); // Try to steal chunk from random GC thread for (int i = 0; i < 4 * jl_n_markthreads; i++) { - uint32_t v = gc_first_tid + cong(UINT64_MAX, UINT64_MAX, &ptls->rngseed) % jl_n_markthreads; - jl_gc_markqueue_t *mq2 = &gc_all_tls_states[v]->mark_queue; + int v = gc_random_parallel_collector_thread_id(ptls); + jl_ptls_t ptls2 = gc_all_tls_states[v]; + gc_check_ptls_of_parallel_collector_thread(ptls2); + jl_gc_markqueue_t *mq2 = &ptls2->mark_queue; c = gc_chunkqueue_steal_from(mq2); if (c.cid != GC_empty_chunk) { gc_mark_chunk(ptls, mq, &c); @@ -3017,8 +3025,10 @@ void gc_mark_and_steal(jl_ptls_t ptls) } } // Sequentially walk GC threads to try to steal chunk - for (int i = gc_first_tid; i < gc_first_tid + jl_n_markthreads; i++) { - jl_gc_markqueue_t *mq2 = &gc_all_tls_states[i]->mark_queue; + for (int i = first; i <= last; i++) { + jl_ptls_t ptls2 = gc_all_tls_states[i]; + gc_check_ptls_of_parallel_collector_thread(ptls2); + jl_gc_markqueue_t *mq2 = &ptls2->mark_queue; c = gc_chunkqueue_steal_from(mq2); if (c.cid != GC_empty_chunk) { gc_mark_chunk(ptls, mq, &c); @@ -3035,15 +3045,19 @@ void gc_mark_and_steal(jl_ptls_t ptls) } // Try to steal pointer from random GC thread for (int i = 0; i < 4 * jl_n_markthreads; i++) { - uint32_t v = gc_first_tid + cong(UINT64_MAX, UINT64_MAX, &ptls->rngseed) % jl_n_markthreads; - jl_gc_markqueue_t *mq2 = &gc_all_tls_states[v]->mark_queue; + int v = gc_random_parallel_collector_thread_id(ptls); + jl_ptls_t ptls2 = gc_all_tls_states[v]; + gc_check_ptls_of_parallel_collector_thread(ptls2); + jl_gc_markqueue_t *mq2 = &ptls2->mark_queue; new_obj = gc_ptr_queue_steal_from(mq2); if (new_obj != NULL) goto mark; } // Sequentially walk GC threads to try to steal pointer - for (int i = gc_first_tid; i < gc_first_tid + jl_n_markthreads; i++) { - jl_gc_markqueue_t *mq2 = &gc_all_tls_states[i]->mark_queue; + for (int i = first; i <= last; i++) { + jl_ptls_t ptls2 = gc_all_tls_states[i]; + gc_check_ptls_of_parallel_collector_thread(ptls2); + jl_gc_markqueue_t *mq2 = &ptls2->mark_queue; new_obj = gc_ptr_queue_steal_from(mq2); if (new_obj != NULL) goto mark; @@ -3103,12 +3117,13 @@ int gc_should_mark(void) } int tid = jl_atomic_load_relaxed(&gc_master_tid); assert(tid != -1); + assert(gc_all_tls_states != NULL); size_t work = gc_count_work_in_queue(gc_all_tls_states[tid]); - for (tid = gc_first_tid; tid < gc_first_tid + jl_n_markthreads; tid++) { - jl_ptls_t ptls2 = gc_all_tls_states[tid]; - if (ptls2 == NULL) { - continue; - } + int first = gc_first_parallel_collector_thread_id(); + int last = gc_last_parallel_collector_thread_id(); + for (int i = first; i <= last; i++) { + jl_ptls_t ptls2 = gc_all_tls_states[i]; + gc_check_ptls_of_parallel_collector_thread(ptls2); work += gc_count_work_in_queue(ptls2); } // if there is a lot of work left, enter the mark loop @@ -3486,7 +3501,8 @@ static int _jl_gc_collect(jl_ptls_t ptls, jl_gc_collection_t collection) jl_ptls_t ptls_dest = ptls; jl_gc_markqueue_t *mq_dest = mq; if (!single_threaded_mark) { - ptls_dest = gc_all_tls_states[gc_first_tid + t_i % jl_n_markthreads]; + int dest_tid = gc_ith_parallel_collector_thread_id(t_i % jl_n_markthreads); + ptls_dest = gc_all_tls_states[dest_tid]; mq_dest = &ptls_dest->mark_queue; } if (ptls2 != NULL) { diff --git a/src/gc.h b/src/gc.h index bcba63a613b1f..b8da8b81a5423 100644 --- a/src/gc.h +++ b/src/gc.h @@ -439,6 +439,54 @@ extern int gc_first_tid; extern int gc_n_threads; extern jl_ptls_t* gc_all_tls_states; +STATIC_INLINE int gc_first_parallel_collector_thread_id(void) JL_NOTSAFEPOINT +{ + if (jl_n_markthreads == 0) { + return 0; + } + return gc_first_tid; +} + +STATIC_INLINE int gc_last_parallel_collector_thread_id(void) JL_NOTSAFEPOINT +{ + if (jl_n_markthreads == 0) { + return -1; + } + return gc_first_tid + jl_n_markthreads - 1; +} + +STATIC_INLINE int gc_ith_parallel_collector_thread_id(int i) JL_NOTSAFEPOINT +{ + assert(i >= 0 && i < jl_n_markthreads); + return gc_first_tid + i; +} + +STATIC_INLINE int gc_is_parallel_collector_thread(int tid) JL_NOTSAFEPOINT +{ + return tid >= gc_first_tid && tid <= gc_last_parallel_collector_thread_id(); +} + +STATIC_INLINE int gc_random_parallel_collector_thread_id(jl_ptls_t ptls) JL_NOTSAFEPOINT +{ + assert(jl_n_markthreads > 0); + int v = gc_first_tid + (int)cong(jl_n_markthreads, UINT64_MAX, &ptls->rngseed); + assert(v >= gc_first_tid && v <= gc_last_parallel_collector_thread_id()); + return v; +} + +STATIC_INLINE int gc_parallel_collector_threads_enabled(void) JL_NOTSAFEPOINT +{ + return jl_n_markthreads > 0; +} + +STATIC_INLINE void gc_check_ptls_of_parallel_collector_thread(jl_ptls_t ptls) JL_NOTSAFEPOINT +{ + (void)ptls; + assert(gc_parallel_collector_threads_enabled()); + assert(ptls != NULL); + assert(jl_atomic_load_relaxed(&ptls->gc_state) == JL_GC_PARALLEL_COLLECTOR_THREAD); +} + STATIC_INLINE bigval_t *bigval_header(jl_taggedvalue_t *o) JL_NOTSAFEPOINT { return container_of(o, bigval_t, header); diff --git a/src/julia_threads.h b/src/julia_threads.h index a80084a86f4b3..f6dbb7763b63e 100644 --- a/src/julia_threads.h +++ b/src/julia_threads.h @@ -215,6 +215,10 @@ typedef struct _jl_tls_states_t { #define JL_GC_STATE_SAFE 2 // gc_state = 2 means the thread is running unmanaged code that can be // execute at the same time with the GC. +#define JL_GC_PARALLEL_COLLECTOR_THREAD 3 + // gc_state = 3 means the thread is a parallel collector thread (i.e. never runs Julia code) +#define JL_GC_CONCURRENT_COLLECTOR_THREAD 4 + // gc_state = 4 means the thread is a concurrent collector thread (background sweeper thread that never runs Julia code) _Atomic(int8_t) gc_state; // read from foreign threads // execution of certain certain impure // statements is prohibited from certain @@ -343,6 +347,8 @@ void jl_sigint_safepoint(jl_ptls_t tls); STATIC_INLINE int8_t jl_gc_state_set(jl_ptls_t ptls, int8_t state, int8_t old_state) { + assert(old_state != JL_GC_PARALLEL_COLLECTOR_THREAD); + assert(old_state != JL_GC_CONCURRENT_COLLECTOR_THREAD); jl_atomic_store_release(&ptls->gc_state, state); // A safe point is required if we transition from GC-safe region to // non GC-safe region. diff --git a/src/partr.c b/src/partr.c index f8a87ac9bbc1b..b0b4ee77a9ec5 100644 --- a/src/partr.c +++ b/src/partr.c @@ -131,7 +131,7 @@ void jl_parallel_gc_threadfun(void *arg) jl_task_t *ct = jl_init_root_task(ptls, stack_lo, stack_hi); JL_GC_PROMISE_ROOTED(ct); // wait for all threads - jl_gc_state_set(ptls, JL_GC_STATE_WAITING, 0); + jl_gc_state_set(ptls, JL_GC_PARALLEL_COLLECTOR_THREAD, 0); uv_barrier_wait(targ->barrier); // free the thread argument here @@ -143,10 +143,10 @@ void jl_parallel_gc_threadfun(void *arg) uv_cond_wait(&gc_threads_cond, &gc_threads_lock); } uv_mutex_unlock(&gc_threads_lock); - if (may_mark()) { - gc_mark_loop_parallel(ptls, 0); - } - if (may_sweep(ptls)) { // not an else! + assert(jl_atomic_load_relaxed(&ptls->gc_state) == JL_GC_PARALLEL_COLLECTOR_THREAD); + gc_mark_loop_parallel(ptls, 0); + if (may_sweep(ptls)) { + assert(jl_atomic_load_relaxed(&ptls->gc_state) == JL_GC_PARALLEL_COLLECTOR_THREAD); gc_sweep_pool_parallel(ptls); jl_atomic_fetch_add(&ptls->gc_sweeps_requested, -1); } @@ -166,13 +166,14 @@ void jl_concurrent_gc_threadfun(void *arg) jl_task_t *ct = jl_init_root_task(ptls, stack_lo, stack_hi); JL_GC_PROMISE_ROOTED(ct); // wait for all threads - jl_gc_state_set(ptls, JL_GC_STATE_WAITING, 0); + jl_gc_state_set(ptls, JL_GC_CONCURRENT_COLLECTOR_THREAD, 0); uv_barrier_wait(targ->barrier); // free the thread argument here free(targ); while (1) { + assert(jl_atomic_load_relaxed(&ptls->gc_state) == JL_GC_CONCURRENT_COLLECTOR_THREAD); uv_sem_wait(&gc_sweep_assists_needed); gc_free_pages(); } From 8e949b602eeb140450210d8fe3468dc5713f4f11 Mon Sep 17 00:00:00 2001 From: Diogo Netto <61364108+d-netto@users.noreply.github.com> Date: Tue, 2 Jul 2024 16:32:22 -0300 Subject: [PATCH 3/4] remove stale objprofile (#54991) As mentioned in https://github.com/JuliaLang/julia/issues/54968, `OBJPROFILE` exposes a functionality which is quite similar to what the heap snapshot does, but has a considerably worse visualization tool (i.e. raw printf's compared to the snapshot viewer from Chrome). Closes https://github.com/JuliaLang/julia/issues/54968. --- src/gc-debug.c | 92 -------------------------------------------------- src/gc.c | 24 ------------- src/gc.h | 18 ---------- src/options.h | 3 -- 4 files changed, 137 deletions(-) diff --git a/src/gc-debug.c b/src/gc-debug.c index 1f1aca1a467f5..6faee6b1de97f 100644 --- a/src/gc-debug.c +++ b/src/gc-debug.c @@ -603,91 +603,6 @@ void jl_gc_debug_print_status(void) } #endif -#ifdef OBJPROFILE -static htable_t obj_counts[3]; -static htable_t obj_sizes[3]; -void objprofile_count(void *ty, int old, int sz) -{ - if (gc_verifying) return; - if ((intptr_t)ty <= 0x10) { - ty = (void*)jl_buff_tag; - } - else if (ty != (void*)jl_buff_tag && ty != jl_malloc_tag && - jl_typeof(ty) == (jl_value_t*)jl_datatype_type && - ((jl_datatype_t*)ty)->instance) { - ty = jl_singleton_tag; - } - void **bp = ptrhash_bp(&obj_counts[old], ty); - if (*bp == HT_NOTFOUND) - *bp = (void*)2; - else - (*((intptr_t*)bp))++; - bp = ptrhash_bp(&obj_sizes[old], ty); - if (*bp == HT_NOTFOUND) - *bp = (void*)(intptr_t)(1 + sz); - else - *((intptr_t*)bp) += sz; -} - -void objprofile_reset(void) -{ - for (int g = 0; g < 3; g++) { - htable_reset(&obj_counts[g], 0); - htable_reset(&obj_sizes[g], 0); - } -} - -static void objprofile_print(htable_t nums, htable_t sizes) -{ - for(int i=0; i < nums.size; i+=2) { - if (nums.table[i+1] != HT_NOTFOUND) { - void *ty = nums.table[i]; - int num = (intptr_t)nums.table[i + 1] - 1; - size_t sz = (uintptr_t)ptrhash_get(&sizes, ty) - 1; - static const int ptr_hex_width = 2 * sizeof(void*); - if (sz > 2e9) { - jl_safe_printf(" %6d : %*.1f GB of (%*p) ", - num, 6, ((double)sz) / 1024 / 1024 / 1024, - ptr_hex_width, ty); - } - else if (sz > 2e6) { - jl_safe_printf(" %6d : %*.1f MB of (%*p) ", - num, 6, ((double)sz) / 1024 / 1024, - ptr_hex_width, ty); - } - else if (sz > 2e3) { - jl_safe_printf(" %6d : %*.1f kB of (%*p) ", - num, 6, ((double)sz) / 1024, - ptr_hex_width, ty); - } - else { - jl_safe_printf(" %6d : %*d B of (%*p) ", - num, 6, (int)sz, ptr_hex_width, ty); - } - if (ty == (void*)jl_buff_tag) - jl_safe_printf("#"); - else if (ty == jl_malloc_tag) - jl_safe_printf("#"); - else if (ty == jl_singleton_tag) - jl_safe_printf("#"); - else - jl_static_show(JL_STDERR, (jl_value_t*)ty); - jl_safe_printf("\n"); - } - } -} - -void objprofile_printall(void) -{ - jl_safe_printf("Transient mark :\n"); - objprofile_print(obj_counts[0], obj_sizes[0]); - jl_safe_printf("Perm mark :\n"); - objprofile_print(obj_counts[1], obj_sizes[1]); - jl_safe_printf("Remset :\n"); - objprofile_print(obj_counts[2], obj_sizes[2]); -} -#endif - #if defined(GC_TIME) || defined(GC_FINAL_STATS) STATIC_INLINE double jl_ns2ms(int64_t t) { @@ -996,13 +911,6 @@ void jl_gc_debug_init(void) arraylist_new(&lostval_parents_done, 0); #endif -#ifdef OBJPROFILE - for (int g = 0; g < 3; g++) { - htable_new(&obj_counts[g], 0); - htable_new(&obj_sizes[g], 0); - } -#endif - #ifdef GC_FINAL_STATS process_t0 = jl_hrtime(); #endif diff --git a/src/gc.c b/src/gc.c index 42d682b496e98..a3b18effc3b8a 100644 --- a/src/gc.c +++ b/src/gc.c @@ -861,8 +861,6 @@ STATIC_INLINE void gc_setmark_big(jl_ptls_t ptls, jl_taggedvalue_t *o, gc_queue_big_marked(ptls, hdr, 1); } } - objprofile_count(jl_typeof(jl_valueof(o)), - mark_mode == GC_OLD_MARKED, hdr->sz); } // This function should be called exactly once during marking for each pool @@ -884,8 +882,6 @@ STATIC_INLINE void gc_setmark_pool_(jl_ptls_t ptls, jl_taggedvalue_t *o, page->has_young = 1; } } - objprofile_count(jl_typeof(jl_valueof(o)), - mark_mode == GC_OLD_MARKED, page->osize); page->has_marked = 1; #endif } @@ -2677,8 +2673,6 @@ FORCE_INLINE void gc_mark_outrefs(jl_ptls_t ptls, jl_gc_markqueue_t *mq, void *_ size_t dtsz = l * sizeof(void *) + sizeof(jl_svec_t); if (update_meta) gc_setmark(ptls, o, bits, dtsz); - else if (foreign_alloc) - objprofile_count(jl_simplevector_type, bits == GC_OLD_MARKED, dtsz); jl_value_t *objary_parent = new_obj; jl_value_t **objary_begin = data; jl_value_t **objary_end = data + l; @@ -2689,8 +2683,6 @@ FORCE_INLINE void gc_mark_outrefs(jl_ptls_t ptls, jl_gc_markqueue_t *mq, void *_ else if (vtag == jl_module_tag << 4) { if (update_meta) gc_setmark(ptls, o, bits, sizeof(jl_module_t)); - else if (foreign_alloc) - objprofile_count(jl_module_type, bits == GC_OLD_MARKED, sizeof(jl_module_t)); jl_module_t *mb_parent = (jl_module_t *)new_obj; uintptr_t nptr = ((mb_parent->usings.len + 1) << 2) | (bits & GC_OLD); gc_mark_module_binding(ptls, mb_parent, nptr, bits); @@ -2698,8 +2690,6 @@ FORCE_INLINE void gc_mark_outrefs(jl_ptls_t ptls, jl_gc_markqueue_t *mq, void *_ else if (vtag == jl_task_tag << 4) { if (update_meta) gc_setmark(ptls, o, bits, sizeof(jl_task_t)); - else if (foreign_alloc) - objprofile_count(jl_task_type, bits == GC_OLD_MARKED, sizeof(jl_task_t)); jl_task_t *ta = (jl_task_t *)new_obj; gc_scrub_record_task(ta); if (gc_cblist_task_scanner) { @@ -2768,16 +2758,12 @@ FORCE_INLINE void gc_mark_outrefs(jl_ptls_t ptls, jl_gc_markqueue_t *mq, void *_ size_t dtsz = jl_string_len(new_obj) + sizeof(size_t) + 1; if (update_meta) gc_setmark(ptls, o, bits, dtsz); - else if (foreign_alloc) - objprofile_count(jl_string_type, bits == GC_OLD_MARKED, dtsz); } else { jl_datatype_t *vt = ijl_small_typeof[vtag / sizeof(*ijl_small_typeof)]; size_t dtsz = jl_datatype_size(vt); if (update_meta) gc_setmark(ptls, o, bits, dtsz); - else if (foreign_alloc) - objprofile_count(vt, bits == GC_OLD_MARKED, dtsz); } return; } @@ -2796,9 +2782,6 @@ FORCE_INLINE void gc_mark_outrefs(jl_ptls_t ptls, jl_gc_markqueue_t *mq, void *_ else gc_setmark_big(ptls, o, bits); } - else if (foreign_alloc) { - objprofile_count(vt, bits == GC_OLD_MARKED, sizeof(jl_array_t)); - } if (flags.how == 0) { void *data_ptr = (char*)a + sizeof(jl_array_t) +jl_array_ndimwords(a->flags.ndims) * sizeof(size_t); gc_heap_snapshot_record_hidden_edge(new_obj, data_ptr, jl_array_nbytes(a), 2); @@ -2813,8 +2796,6 @@ FORCE_INLINE void gc_mark_outrefs(jl_ptls_t ptls, jl_gc_markqueue_t *mq, void *_ } else if (flags.how == 2) { if (update_meta || foreign_alloc) { - objprofile_count(jl_malloc_tag, bits == GC_OLD_MARKED, - jl_array_nbytes(a)); gc_heap_snapshot_record_hidden_edge(new_obj, a->data, jl_array_nbytes(a), flags.pooled); if (bits == GC_OLD_MARKED) { ptls->gc_cache.perm_scanned_bytes += jl_array_nbytes(a); @@ -2881,8 +2862,6 @@ FORCE_INLINE void gc_mark_outrefs(jl_ptls_t ptls, jl_gc_markqueue_t *mq, void *_ size_t dtsz = jl_datatype_size(vt); if (update_meta) gc_setmark(ptls, o, bits, dtsz); - else if (foreign_alloc) - objprofile_count(vt, bits == GC_OLD_MARKED, dtsz); if (vt == jl_weakref_type) return; const jl_datatype_layout_t *layout = vt->layout; @@ -3221,7 +3200,6 @@ static void gc_premark(jl_ptls_t ptls2) void **items = remset->items; for (size_t i = 0; i < len; i++) { jl_value_t *item = (jl_value_t *)items[i]; - objprofile_count(jl_typeof(item), 2, 0); jl_astaggedvalue(item)->bits.gc = GC_OLD_MARKED; } } @@ -3586,8 +3564,6 @@ static int _jl_gc_collect(jl_ptls_t ptls, jl_gc_collection_t collection) gc_stats_all_pool(); gc_stats_big_obj(); - objprofile_printall(); - objprofile_reset(); gc_num.total_allocd += gc_num.allocd; if (!prev_sweep_full) promoted_bytes += perm_scanned_bytes - last_perm_scanned_bytes; diff --git a/src/gc.h b/src/gc.h index b8da8b81a5423..1a74d334c98f4 100644 --- a/src/gc.h +++ b/src/gc.h @@ -720,24 +720,6 @@ static inline void gc_scrub(void) } #endif -#ifdef OBJPROFILE -void objprofile_count(void *ty, int old, int sz) JL_NOTSAFEPOINT; -void objprofile_printall(void); -void objprofile_reset(void); -#else -static inline void objprofile_count(void *ty, int old, int sz) JL_NOTSAFEPOINT -{ -} - -static inline void objprofile_printall(void) -{ -} - -static inline void objprofile_reset(void) -{ -} -#endif - #ifdef MEMPROFILE void gc_stats_all_pool(void); void gc_stats_big_obj(void); diff --git a/src/options.h b/src/options.h index f4a052ddc1cb4..285f849eee216 100644 --- a/src/options.h +++ b/src/options.h @@ -75,9 +75,6 @@ // GC_TIME prints time taken by each phase of GC // #define GC_TIME -// OBJPROFILE counts objects by type -// #define OBJPROFILE - // pool allocator configuration options // GC_SMALL_PAGE allocates objects in 4k pages From 932eabed94841f1ec19669740aa2f0e9e774a7aa Mon Sep 17 00:00:00 2001 From: Diogo Netto <61364108+d-netto@users.noreply.github.com> Date: Wed, 3 Jul 2024 20:00:43 -0300 Subject: [PATCH 4/4] delete possibly stale reset_gc_stats (#55015) See discussion in https://github.com/JuliaLang/julia/issues/55014. Doesn't seem breaking, but I can close the PR if it is. Closes https://github.com/JuliaLang/julia/issues/55014. --- base/timing.jl | 1 - src/gc.c | 7 ------- 2 files changed, 8 deletions(-) diff --git a/base/timing.jl b/base/timing.jl index 4b14161aa89d2..bdbb32936b56f 100644 --- a/base/timing.jl +++ b/base/timing.jl @@ -29,7 +29,6 @@ struct GC_Num end gc_num() = ccall(:jl_gc_num, GC_Num, ()) -reset_gc_stats() = ccall(:jl_gc_reset_stats, Cvoid, ()) # This type is to represent differences in the counters, so fields may be negative struct GC_Diff diff --git a/src/gc.c b/src/gc.c index a3b18effc3b8a..2b2c0f132f622 100644 --- a/src/gc.c +++ b/src/gc.c @@ -3399,13 +3399,6 @@ JL_DLLEXPORT jl_gc_num_t jl_gc_num(void) return num; } -JL_DLLEXPORT void jl_gc_reset_stats(void) -{ - gc_num.max_pause = 0; - gc_num.max_memory = 0; - gc_num.max_time_to_safepoint = 0; -} - // TODO: these were supposed to be thread local JL_DLLEXPORT int64_t jl_gc_diff_total_bytes(void) JL_NOTSAFEPOINT {