From 8a157bc616dc42407860b5da9e0b25e910ba0b47 Mon Sep 17 00:00:00 2001 From: Andy Wingo Date: Fri, 16 May 2025 21:57:10 +0200 Subject: [PATCH 1/7] Add allocation counter to prepare_gc event Adapt all users --- api/gc-basic-stats.h | 5 ++++- api/gc-event-listener-chain.h | 7 ++++--- api/gc-event-listener.h | 3 ++- api/gc-lttng.h | 5 +++-- api/gc-null-event-listener.h | 3 ++- src/bdw.c | 2 +- src/mmc.c | 2 +- src/pcc.c | 2 +- src/semi.c | 4 +++- 9 files changed, 21 insertions(+), 12 deletions(-) diff --git a/api/gc-basic-stats.h b/api/gc-basic-stats.h index 0f8c68aee..69a98cdf3 100644 --- a/api/gc-basic-stats.h +++ b/api/gc-basic-stats.h @@ -26,6 +26,7 @@ struct gc_basic_stats { size_t max_heap_size; size_t live_data_size; size_t max_live_data_size; + uint64_t allocation_counter_at_last_gc; struct gc_latency pause_times; }; @@ -68,12 +69,14 @@ static inline void gc_basic_stats_waiting_for_stop(void *data) {} static inline void gc_basic_stats_mutators_stopped(void *data) {} static inline void gc_basic_stats_prepare_gc(void *data, - enum gc_collection_kind kind) { + enum gc_collection_kind kind, + uint64_t allocation_counter) { struct gc_basic_stats *stats = data; if (kind == GC_COLLECTION_MINOR) stats->minor_collection_count++; else stats->major_collection_count++; + stats->allocation_counter_at_last_gc = allocation_counter; } static inline void gc_basic_stats_roots_traced(void *data) {} diff --git a/api/gc-event-listener-chain.h b/api/gc-event-listener-chain.h index 27b56d5c6..380439f2c 100644 --- a/api/gc-event-listener-chain.h +++ b/api/gc-event-listener-chain.h @@ -36,10 +36,11 @@ static inline void gc_event_listener_chain_mutators_stopped(void *data) { chain->tail.mutators_stopped(chain->tail_data); } static inline void -gc_event_listener_chain_prepare_gc(void *data, enum gc_collection_kind kind) { +gc_event_listener_chain_prepare_gc(void *data, enum gc_collection_kind kind, + uint64_t counter) { struct gc_event_listener_chain *chain = data; - chain->head.prepare_gc(chain->head_data, kind); - chain->tail.prepare_gc(chain->tail_data, kind); + chain->head.prepare_gc(chain->head_data, kind, counter); + chain->tail.prepare_gc(chain->tail_data, kind, counter); } static inline void gc_event_listener_chain_roots_traced(void *data) { struct gc_event_listener_chain *chain = data; diff --git a/api/gc-event-listener.h b/api/gc-event-listener.h index 66801a52c..da4ab5809 100644 --- a/api/gc-event-listener.h +++ b/api/gc-event-listener.h @@ -8,7 +8,8 @@ struct gc_event_listener { void (*requesting_stop)(void *data); void (*waiting_for_stop)(void *data); void (*mutators_stopped)(void *data); - void (*prepare_gc)(void *data, enum gc_collection_kind kind); + void (*prepare_gc)(void *data, enum gc_collection_kind kind, + uint64_t allocation_counter); void (*roots_traced)(void *data); void (*heap_traced)(void *data); void (*ephemerons_traced)(void *data); diff --git a/api/gc-lttng.h b/api/gc-lttng.h index d192be4ed..24612f4a8 100644 --- a/api/gc-lttng.h +++ b/api/gc-lttng.h @@ -45,9 +45,10 @@ LTTNG_UST_TRACEPOINT_EVENT_INSTANCE( whippet, tracepoint, whippet, mutators_stopped, LTTNG_UST_TP_ARGS()) LTTNG_UST_TRACEPOINT_EVENT( whippet, prepare_gc, - LTTNG_UST_TP_ARGS(int, gc_kind), + LTTNG_UST_TP_ARGS(int, gc_kind, uint64_t, allocation_counter), LTTNG_UST_TP_FIELDS( - lttng_ust_field_enum(whippet, gc_kind, int, gc_kind, gc_kind))) + lttng_ust_field_enum(whippet, gc_kind, int, gc_kind, gc_kind) + lttng_ust_field_integer(uint64_t, allocation_counter, allocation_counter))) LTTNG_UST_TRACEPOINT_EVENT_INSTANCE( whippet, tracepoint, whippet, roots_traced, LTTNG_UST_TP_ARGS()) LTTNG_UST_TRACEPOINT_EVENT_INSTANCE( diff --git a/api/gc-null-event-listener.h b/api/gc-null-event-listener.h index 9c032ffc2..63ca29eb1 100644 --- a/api/gc-null-event-listener.h +++ b/api/gc-null-event-listener.h @@ -8,7 +8,8 @@ static inline void gc_null_event_listener_requesting_stop(void *data) {} static inline void gc_null_event_listener_waiting_for_stop(void *data) {} static inline void gc_null_event_listener_mutators_stopped(void *data) {} static inline void gc_null_event_listener_prepare_gc(void *data, - enum gc_collection_kind) {} + enum gc_collection_kind, + uint64_t) {} static inline void gc_null_event_listener_roots_traced(void *data) {} static inline void gc_null_event_listener_heap_traced(void *data) {} static inline void gc_null_event_listener_ephemerons_traced(void *data) {} diff --git a/src/bdw.c b/src/bdw.c index fc228c15e..8cee58a73 100644 --- a/src/bdw.c +++ b/src/bdw.c @@ -505,7 +505,7 @@ static void on_collection_event(GC_EventType event) { } case GC_EVENT_MARK_START: HEAP_EVENT(mutators_stopped); - HEAP_EVENT(prepare_gc, GC_COLLECTION_MAJOR); + HEAP_EVENT(prepare_gc, GC_COLLECTION_MAJOR, GC_get_total_bytes()); break; case GC_EVENT_MARK_END: HEAP_EVENT(roots_traced); diff --git a/src/mmc.c b/src/mmc.c index 4e79675cc..b6b83396b 100644 --- a/src/mmc.c +++ b/src/mmc.c @@ -792,7 +792,7 @@ collect(struct gc_mutator *mut, enum gc_collection_kind requested_kind) { enum gc_collection_kind gc_kind = determine_collection_kind(heap, requested_kind); int is_minor = gc_kind == GC_COLLECTION_MINOR; - HEAP_EVENT(heap, prepare_gc, gc_kind); + HEAP_EVENT(heap, prepare_gc, gc_kind, heap->total_allocated_bytes_at_last_gc); nofl_space_prepare_gc(nofl_space, gc_kind); large_object_space_start_gc(lospace, is_minor); gc_extern_space_start_gc(exspace, is_minor); diff --git a/src/pcc.c b/src/pcc.c index 6902b3412..c0aef49fd 100644 --- a/src/pcc.c +++ b/src/pcc.c @@ -905,11 +905,11 @@ collect(struct gc_mutator *mut, enum gc_collection_kind requested_kind) { heap->is_minor_collection = #endif GC_GENERATIONAL ? gc_kind == GC_COLLECTION_MINOR : 0; - HEAP_EVENT(heap, prepare_gc, gc_kind); uint64_t *counter_loc = &heap->total_allocated_bytes_at_last_gc; copy_space_add_to_allocation_counter(heap_allocation_space(heap), counter_loc); large_object_space_add_to_allocation_counter(lospace, counter_loc); + HEAP_EVENT(heap, prepare_gc, gc_kind, *counter_loc); copy_spaces_start_gc(heap, is_minor_gc); large_object_space_start_gc(lospace, is_minor_gc); gc_extern_space_start_gc(exspace, is_minor_gc); diff --git a/src/semi.c b/src/semi.c index a1526cec4..0cca01868 100644 --- a/src/semi.c +++ b/src/semi.c @@ -395,7 +395,6 @@ static void collect(struct gc_mutator *mut) { HEAP_EVENT(heap, requesting_stop); HEAP_EVENT(heap, waiting_for_stop); HEAP_EVENT(heap, mutators_stopped); - HEAP_EVENT(heap, prepare_gc, GC_COLLECTION_COMPACTING); struct semi_space *semi = heap_semi_space(heap); struct large_object_space *large = heap_large_object_space(heap); @@ -403,6 +402,9 @@ static void collect(struct gc_mutator *mut) { uint64_t *counter_loc = &heap->total_allocated_bytes_at_last_gc; semi_space_add_to_allocation_counter(semi, counter_loc); large_object_space_add_to_allocation_counter(large, counter_loc); + + HEAP_EVENT(heap, prepare_gc, GC_COLLECTION_COMPACTING, *counter_loc); + large_object_space_start_gc(large, 0); gc_extern_space_start_gc(heap->extern_space, 0); flip(semi); From 9512c8b8060c0c203a37b42c46a8d96527a1c34f Mon Sep 17 00:00:00 2001 From: Andy Wingo Date: Fri, 16 May 2025 21:59:27 +0200 Subject: [PATCH 2/7] Simplify mmc-attrs.h --- api/mmc-attrs.h | 1 - 1 file changed, 1 deletion(-) diff --git a/api/mmc-attrs.h b/api/mmc-attrs.h index 49ff5630b..6e9b6487d 100644 --- a/api/mmc-attrs.h +++ b/api/mmc-attrs.h @@ -42,7 +42,6 @@ static inline uint8_t gc_allocator_alloc_table_begin_pattern(enum gc_allocation_ case GC_ALLOCATION_UNTAGGED_CONSERVATIVE: return young | trace_conservatively; case GC_ALLOCATION_TAGGED_POINTERLESS: - return young | trace_none; case GC_ALLOCATION_UNTAGGED_POINTERLESS: return young | trace_none; default: From 7010b4fce0374a7c3a5eb2d75ad0a4e2c8b84398 Mon Sep 17 00:00:00 2001 From: Andy Wingo Date: Fri, 16 May 2025 21:59:40 +0200 Subject: [PATCH 3/7] Fix --with-gc-debug in whippet.m4 --- whippet.m4 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/whippet.m4 b/whippet.m4 index f364d1973..9ff7a81bc 100644 --- a/whippet.m4 +++ b/whippet.m4 @@ -176,7 +176,7 @@ END fi]) AC_DEFUN([WHIPPET_PKG_DEBUG], - [AC_ARG_WITH(whippet-debug, + [AC_ARG_WITH(gc-debug, AS_HELP_STRING([--with-gc-debug], [Compile GC library with debugging support (default: no)]), [whippet_with_debug=$withval], From e59fde2edf58e8421a1267b9a937b429817f9761 Mon Sep 17 00:00:00 2001 From: Andy Wingo Date: Fri, 16 May 2025 22:00:02 +0200 Subject: [PATCH 4/7] Fix build issue with mmc in Guile For some reason we need to include gc-api with GC_IMPL already defined, otherwise the attrs don't get picked up. --- src/mmc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/mmc.c b/src/mmc.c index b6b83396b..02f51c776 100644 --- a/src/mmc.c +++ b/src/mmc.c @@ -4,9 +4,10 @@ #include #include +#define GC_IMPL 1 + #include "gc-api.h" -#define GC_IMPL 1 #include "gc-internal.h" #include "background-thread.h" From 2018a77f3631bffe2e8d4cebc302cbedde51b338 Mon Sep 17 00:00:00 2001 From: Andy Wingo Date: Fri, 16 May 2025 22:00:48 +0200 Subject: [PATCH 5/7] Fix bogus static debug check in mmc init --- src/mmc.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/mmc.c b/src/mmc.c index 02f51c776..a5743fb5d 100644 --- a/src/mmc.c +++ b/src/mmc.c @@ -1216,16 +1216,18 @@ gc_init(const struct gc_options *options, struct gc_stack_addr stack_base, GC_ASSERT_EQ(gc_allocator_allocation_limit_offset(), offsetof(struct nofl_allocator, sweep)); GC_ASSERT_EQ(gc_allocator_alloc_table_alignment(), NOFL_SLAB_SIZE); - GC_ASSERT_EQ(gc_allocator_alloc_table_begin_pattern(GC_ALLOCATION_TAGGED), - NOFL_METADATA_BYTE_YOUNG | NOFL_METADATA_BYTE_TRACE_PRECISELY); GC_ASSERT_EQ(gc_allocator_alloc_table_begin_pattern(GC_ALLOCATION_TAGGED_POINTERLESS), NOFL_METADATA_BYTE_YOUNG | NOFL_METADATA_BYTE_TRACE_NONE); if (GC_CONSERVATIVE_TRACE) { + GC_ASSERT_EQ(gc_allocator_alloc_table_begin_pattern(GC_ALLOCATION_TAGGED), + NOFL_METADATA_BYTE_YOUNG | NOFL_METADATA_BYTE_TRACE_CONSERVATIVELY); GC_ASSERT_EQ(gc_allocator_alloc_table_begin_pattern(GC_ALLOCATION_UNTAGGED_CONSERVATIVE), NOFL_METADATA_BYTE_YOUNG | NOFL_METADATA_BYTE_TRACE_CONSERVATIVELY); GC_ASSERT_EQ(gc_allocator_alloc_table_begin_pattern(GC_ALLOCATION_UNTAGGED_POINTERLESS), NOFL_METADATA_BYTE_YOUNG | NOFL_METADATA_BYTE_TRACE_NONE); } else { + GC_ASSERT_EQ(gc_allocator_alloc_table_begin_pattern(GC_ALLOCATION_TAGGED), + NOFL_METADATA_BYTE_YOUNG | NOFL_METADATA_BYTE_TRACE_PRECISELY); GC_ASSERT_EQ(gc_allocator_alloc_table_begin_pattern(GC_ALLOCATION_UNTAGGED_POINTERLESS), NOFL_METADATA_BYTE_YOUNG | NOFL_METADATA_BYTE_TRACE_NONE | NOFL_METADATA_BYTE_PINNED); From fbcdffdc620494d524e1c80785bdeadc579c86d3 Mon Sep 17 00:00:00 2001 From: Andy Wingo Date: Fri, 16 May 2025 22:01:06 +0200 Subject: [PATCH 6/7] Fix bogus assert in mmc.c:deactivate_mutator --- src/mmc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mmc.c b/src/mmc.c index a5743fb5d..375548701 100644 --- a/src/mmc.c +++ b/src/mmc.c @@ -1293,7 +1293,7 @@ gc_finish_for_thread(struct gc_mutator *mut) { static void deactivate_mutator(struct gc_heap *heap, struct gc_mutator *mut) { - GC_ASSERT(mut->next == NULL); + GC_ASSERT(mut->active); nofl_allocator_finish(&mut->allocator, heap_nofl_space(heap)); if (GC_GENERATIONAL) gc_field_set_writer_release_buffer(&mut->logger); From b794e4663505a98d728943cb21edc735cd7d174b Mon Sep 17 00:00:00 2001 From: Andy Wingo Date: Fri, 16 May 2025 22:01:55 +0200 Subject: [PATCH 7/7] mmc: Grow the heap if collection fails to find space for large alloc For a pending large allocation, we will try to page out blocks from the nofl space. However sometimes we are not able to do so, especially if evacuation is unavailable, as in a heap-conservative configuration. In that case, if the heap is growable, grow the heap after GC if there are still bytes pending to page out. --- src/mmc.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/mmc.c b/src/mmc.c index 375548701..79e6a709a 100644 --- a/src/mmc.c +++ b/src/mmc.c @@ -531,6 +531,23 @@ compute_progress(struct gc_heap *heap, uintptr_t allocation_since_last_gc) { return allocation_since_last_gc > nofl_space_fragmentation(nofl); } +static void +grow_heap_for_large_allocation_if_necessary(struct gc_heap *heap, + enum gc_collection_kind gc_kind, + int progress) +{ + if (progress || heap->sizer.policy == GC_HEAP_SIZE_FIXED) + return; + + struct nofl_space *nofl = heap_nofl_space(heap); + if (nofl_space_shrink (nofl, 0)) + return; + + ssize_t pending = nofl_space_request_release_memory(nofl, 0); + GC_ASSERT (pending > 0); + resize_heap(heap, heap->size + pending); +} + static int compute_success(struct gc_heap *heap, enum gc_collection_kind gc_kind, int progress) { @@ -833,6 +850,7 @@ collect(struct gc_mutator *mut, enum gc_collection_kind requested_kind) { DEBUG("--- total live bytes estimate: %zu\n", live_bytes_estimate); gc_heap_sizer_on_gc(heap->sizer, heap->size, live_bytes_estimate, pause_ns, resize_heap); + grow_heap_for_large_allocation_if_necessary(heap, gc_kind, progress); heap->size_at_last_gc = heap->size; HEAP_EVENT(heap, restarting_mutators); allow_mutators_to_continue(heap);