From 0d054f2694120d70e3bce3c675df3ec0b62a10f6 Mon Sep 17 00:00:00 2001 From: Jake Hughes Date: Wed, 21 Aug 2024 20:42:05 +0100 Subject: [PATCH 1/4] Remove pointless CI step BDWGC is now a submodule in Alloy, so this step (which builds and run's Alloys test suite) will never actually test a BDWGC PRs changes. Instead, this would just grab the current Alloy submodule commit. --- .buildbot.sh | 6 ------ 1 file changed, 6 deletions(-) diff --git a/.buildbot.sh b/.buildbot.sh index afab8e50c..bbab8c910 100644 --- a/.buildbot.sh +++ b/.buildbot.sh @@ -22,9 +22,3 @@ sh rustup.sh --default-host x86_64-unknown-linux-gnu \ --profile minimal \ -y export PATH=`pwd`/.cargo/bin/:$PATH -BDWGC_SRC=`pwd` - -git clone https://github.com/softdevteam/alloy -cd alloy -BDWGC=${BDWGC_SRC} ENABLE_GC_ASSERTIONS=true /usr/bin/time -v python3 x.py test --stage 2 \ - --config .buildbot.config.toml --exclude rustdoc-json --exclude debuginfo From 773862b3335790245f0057324a1e7ad8a3995c4d Mon Sep 17 00:00:00 2001 From: Jake Hughes Date: Wed, 21 Aug 2024 21:08:18 +0100 Subject: [PATCH 2/4] Remove old POSIX TLS APIs We have now switched to using fast TLS support, so this old POSIX get/set_specific support for tracking GC roots in TLS is no longer required. --- alloc.c | 12 ------------ include/gc/gc.h | 3 --- pthread_stop_world.c | 2 -- 3 files changed, 17 deletions(-) diff --git a/alloc.c b/alloc.c index 167309226..4896fbcd5 100644 --- a/alloc.c +++ b/alloc.c @@ -1861,15 +1861,3 @@ GC_INNER ptr_t GC_allocobj(size_t lg, int k) GC_fail_count = 0; return (ptr_t)(*flh); } - -STATIC __thread void* tls_rootset = NULL; - -GC_INNER void GC_init_tls_rootset(void * rootset) -{ - tls_rootset = rootset; -} - -GC_INNER void* GC_tls_rootset() -{ - return tls_rootset; -} diff --git a/include/gc/gc.h b/include/gc/gc.h index b52443620..41882ca86 100644 --- a/include/gc/gc.h +++ b/include/gc/gc.h @@ -2358,7 +2358,4 @@ GC_API void GC_CALL GC_win32_free_heap(void); } /* extern "C" */ #endif -GC_API void GC_init_tls_rootset(void * rootset); -GC_API void * GC_tls_rootset(); - #endif /* GC_H */ diff --git a/pthread_stop_world.c b/pthread_stop_world.c index b8bf0d173..5c617ba06 100644 --- a/pthread_stop_world.c +++ b/pthread_stop_world.c @@ -398,7 +398,6 @@ STATIC void GC_suspend_handler_inner(ptr_t dummy, void *context) } crtn = me -> crtn; GC_store_stack_ptr(crtn); - crtn -> tls_rootset = GC_tls_rootset(); get_thread_local_roots(&tlr); crtn -> compiler_thread_roots = tlr; @@ -834,7 +833,6 @@ GC_INNER void GC_push_all_stacks(void) if (me != NULL) { struct GC_ThreadLocalRoots tlr; get_thread_local_roots(&tlr); - me -> crtn -> tls_rootset = GC_tls_rootset(); me -> crtn -> compiler_thread_roots = tlr; } From 2aeecee23024b0637dbd8800ef92557442a0247d Mon Sep 17 00:00:00 2001 From: Jake Hughes Date: Wed, 21 Aug 2024 21:17:22 +0100 Subject: [PATCH 3/4] Revert to using old GC_finalize mechanism Using an extension of the disclaim API proved difficult because of the need to re-mark objects pointed to by finalisable objects before a finaliser is run. This would have led to complex changes to the mark-phase for no real benefit. Instead, by switching back to the GC_finalize mechanism but ensuring that finalisers are run off of mutator threads, we get this desired behaviour 'for free'. This approach uses a simple condition variable to sleep the finaliser thread when there is no finalisation work to do. This is then potentially resumed at the end of a GC cycle if that cycle discovered finalisable objects. --- include/private/gc_priv.h | 10 +++++----- reclaim.c | 40 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/include/private/gc_priv.h b/include/private/gc_priv.h index 893c6372b..74aa27825 100644 --- a/include/private/gc_priv.h +++ b/include/private/gc_priv.h @@ -341,6 +341,7 @@ typedef struct hblkhdr hdr; #include "gc/gc_inline.h" +<<<<<<< HEAD #ifdef BUFFERED_FINALIZATION typedef struct GC_finalization_buffer_hdr GC_finalization_buffer_hdr; @@ -355,6 +356,9 @@ struct GC_current_buffer { }; GC_INNER void GC_maybe_spawn_finalize_thread(); +======= +GC_INNER void GC_maybe_wake_finalizer_thread(); +>>>>>>> b44b0a3e (Revert to using old GC_finalize mechanism) #endif @@ -402,11 +406,7 @@ GC_INNER void GC_maybe_spawn_finalize_thread(); EXTERN_C_BEGIN #ifndef GC_NO_FINALIZATION -#ifdef BUFFERED_FINALIZATION -# define GC_INVOKE_FINALIZERS() GC_maybe_spawn_finalize_thread() -#else -# define GC_INVOKE_FINALIZERS() GC_notify_or_invoke_finalizers() -#endif +# define GC_INVOKE_FINALIZERS() GC_maybe_wake_finalizer_thread() GC_INNER void GC_notify_or_invoke_finalizers(void); /* If GC_finalize_on_demand is not set, invoke */ /* eligible finalizers. Otherwise: */ diff --git a/reclaim.c b/reclaim.c index ae6a420c9..1936110a4 100644 --- a/reclaim.c +++ b/reclaim.c @@ -21,6 +21,14 @@ # include "gc/gc_disclaim.h" #endif +#ifdef GC_PTHREADS +static pthread_mutex_t flzr_mtx = PTHREAD_MUTEX_INITIALIZER; +static pthread_cond_t flzr_t_has_work = PTHREAD_COND_INITIALIZER; +static int flzr_can_work = 0; +#elif +#error "This fork of BDWGC only supports POSIX threads" +#endif + GC_INNER signed_word GC_bytes_found = 0; /* Number of bytes of memory reclaimed */ /* minus the number of bytes originally */ @@ -876,3 +884,35 @@ GC_API void GC_CALL GC_enumerate_reachable_objects_inner( ed.client_data = client_data; GC_apply_to_all_blocks(GC_do_enumerate_reachable_objects, (word)&ed); } + +static void* init_finalize_thread(void *arg) +{ + while(1) { + pthread_mutex_lock(&flzr_mtx); + while (flzr_can_work == 0) { + pthread_cond_wait(&flzr_t_has_work, &flzr_mtx); + } + flzr_can_work = 0; + pthread_mutex_unlock(&flzr_mtx); + GC_invoke_finalizers(); + } + return arg; +} + +GC_INNER void GC_maybe_wake_finalizer_thread() +{ + if (!GC_finalizer_thread_exists) { + pthread_t t; + pthread_create(&t, NULL, init_finalize_thread, NULL /* arg */); + GC_finalizer_thread_exists = 1; + return; + } + + if (GC_should_invoke_finalizers() == 0) + return; + + pthread_mutex_lock(&flzr_mtx); + flzr_can_work = 1; + pthread_cond_signal(&flzr_t_has_work); + pthread_mutex_unlock(&flzr_mtx); +} From 43d97f6413a43a47a20bd06c455dada18df3fffd Mon Sep 17 00:00:00 2001 From: Jake Hughes Date: Wed, 21 Aug 2024 21:10:38 +0100 Subject: [PATCH 4/4] Remove buffered disclaim finalisation This was replaced with the GC_finalize mechanism in e093be9. --- fnlz_mlc.c | 196 -------------------------------------- include/gc/gc_disclaim.h | 39 -------- include/private/gc_priv.h | 33 +------ misc.c | 8 -- tests/gctest.c | 29 ------ 5 files changed, 2 insertions(+), 303 deletions(-) diff --git a/fnlz_mlc.c b/fnlz_mlc.c index 17aad6803..3f0b02449 100644 --- a/fnlz_mlc.c +++ b/fnlz_mlc.c @@ -131,200 +131,4 @@ GC_API GC_ATTR_MALLOC void * GC_CALL GC_finalized_malloc(size_t lb, return (word *)op + 1; } -# ifdef BUFFERED_FINALIZATION - -static void* init_finalize_thread(void *arg) -{ - while (1) { - GC_finalize_objects(); - } - return arg; -} - -GC_finalization_buffer_hdr* GC_new_buffer() { - GC_ASSERT(I_HOLD_LOCK()); - void* ptr = GC_os_get_mem(GC_page_size); - if (NULL == ptr) - ABORT("Insufficient memory for finalization buffer."); - GC_add_roots_inner(ptr, ptr + GC_page_size, FALSE); - return ptr; -} - -void GC_delete_buffer(GC_finalization_buffer_hdr* buffer) { - GC_remove_roots((void*) buffer, (void*) buffer + GC_page_size); - GC_unmap((void*)buffer, GC_page_size); - -} - -STATIC int GC_CALLBACK GC_push_object_to_fin_buffer(void *obj) -{ - GC_ASSERT(I_HOLD_LOCK()); - - word finalizer_word = *(word *)obj; - if ((finalizer_word & FINALIZER_CLOSURE_FLAG) == 0) { - return 0; - } - - if (finalizer_word & HAS_BEEN_FINALIZED_FLAG) { - /* The object has already been finalized. Return 0 ensures it is - * immediately reclaimed. - */ - return 0; - } - - if (GC_finalizer_buffer_head == NULL) { - /* This can happen for two reasons: - * 1) This is first time a finalizable object is unreachable and no - * finalization buffers have been created yet. - * - * 2) The buffer(s) have already passed to a finalization thread - * which is processing them. We must start again. */ - GC_finalizer_buffer_head = GC_new_buffer(); - GC_finalizer_buffer_current.hdr = GC_finalizer_buffer_head; - GC_finalizer_buffer_current.cursor = (void**) (GC_finalizer_buffer_head + 1); - } - - void** last = (void**) ((void *)GC_finalizer_buffer_current.hdr + GC_page_size); - if (GC_finalizer_buffer_current.cursor == last) { - GC_finalization_buffer_hdr* next = GC_new_buffer(); - GC_finalizer_buffer_current.hdr->next = next; - GC_finalizer_buffer_current.hdr = next; - GC_finalizer_buffer_current.cursor = (void**) (next + 1); - } - - *GC_finalizer_buffer_current.cursor = obj; - GC_finalizer_buffer_current.cursor++; - - return 1; -} - - -GC_API GC_ATTR_MALLOC void * GC_CALL GC_buffered_finalize_malloc(size_t lb) -{ - void *op; - - GC_ASSERT(GC_fin_q_kind != 0); - op = GC_malloc_kind(lb, (int)GC_fin_q_kind); - if (EXPECT(NULL == op, FALSE)) - return NULL; - return (word *)op; -} - -GC_API GC_ATTR_MALLOC void * GC_CALL GC_buffered_finalize_memalign(size_t align, size_t lb) -{ - size_t offset; - ptr_t result; - size_t align_m1 = align - 1; - - /* Check the alignment argument. */ - if (EXPECT(0 == align || (align & align_m1) != 0, FALSE)) return NULL; - if (align <= GC_GRANULE_BYTES) return GC_buffered_finalize_malloc(lb); - - if (align >= HBLKSIZE/2 || lb >= HBLKSIZE/2) { - return GC_clear_stack(GC_generic_malloc_aligned(lb, GC_fin_q_kind, - 0 /* flags */, align_m1)); - } - - result = (ptr_t)GC_buffered_finalize_malloc(SIZET_SAT_ADD(lb, align_m1)); - offset = (size_t)(word)result & align_m1; - if (offset != 0) { - offset = align - offset; - if (!GC_all_interior_pointers) { - GC_STATIC_ASSERT(VALID_OFFSET_SZ <= HBLKSIZE); - GC_ASSERT(offset < VALID_OFFSET_SZ); - GC_register_displacement(offset); - } - result += offset; - } - GC_ASSERT(((word)result & align_m1) == 0); - return result; -} - -GC_API int GC_CALL GC_buffered_finalize_posix_memalign(void **memptr, size_t align, size_t lb) -{ - size_t align_minus_one = align - 1; /* to workaround a cppcheck warning */ - - /* Check alignment properly. */ - if (EXPECT(align < sizeof(void *) - || (align_minus_one & align) != 0, FALSE)) { - return EINVAL; - } - - if ((*memptr = GC_buffered_finalize_memalign(align, lb)) == NULL) { - return ENOMEM; - } - return 0; -} - - -GC_API void GC_CALL GC_init_buffered_finalization(void) -{ - LOCK(); - GC_new_buffer(); - GC_fin_q_kind = GC_new_kind_inner(GC_new_free_list_inner(), - GC_DS_LENGTH, TRUE, TRUE); - GC_ASSERT(GC_fin_q_kind != 0); - - GC_register_disclaim_proc_inner(GC_fin_q_kind, GC_push_object_to_fin_buffer, TRUE); - UNLOCK(); -} - -void GC_finalize_buffer(GC_finalization_buffer_hdr* buffer) { - void** cursor = (void**) (buffer + 1); - void** last = (void**) ((void *)buffer + GC_page_size); - while (cursor != last) - { - if (*cursor == NULL) { - break; - } - void* obj = *cursor; - word finalizer_word = (*(word *)obj) & ~(word)FINALIZER_CLOSURE_FLAG; - GC_disclaim_proc finalizer = (GC_disclaim_proc) finalizer_word; - (finalizer)(obj); - GC_num_finalized++; - /* Prevent the object from being re-added to the finalization queue */ - *(word *)obj = finalizer_word | HAS_BEEN_FINALIZED_FLAG; - cursor++; - } -} - -GC_API void GC_CALL GC_finalize_objects(void) { - /* This is safe to do without locking because this global is only ever - * mutated from within a collection where all mutator threads (including - * this finalisation thread) are paused. It is not possible for them to race. - * - * In addition, a memory barrier synchronises threads at the end of a - * collection, so the finalisation thread will always load the up-to-date - * version of this global. */ - GC_disable(); - GC_finalization_buffer_hdr* buffer = GC_finalizer_buffer_head; - GC_finalizer_buffer_head = NULL; - GC_enable(); - - while(buffer != NULL) - { - GC_finalize_buffer(buffer); - GC_finalization_buffer_hdr* next = buffer->next; - - GC_delete_buffer(buffer); - buffer = next; - } -} - -GC_INNER void GC_maybe_spawn_finalize_thread() -{ - if (GC_finalizer_thread_exists || !GC_finalizer_buffer_head) - return; - - pthread_t t; - pthread_create(&t, NULL, init_finalize_thread, NULL /* arg */); - GC_finalizer_thread_exists = 1; -} - -GC_API size_t GC_finalized_total(void) { - return GC_num_finalized; -} - -# endif - #endif /* ENABLE_DISCLAIM */ diff --git a/include/gc/gc_disclaim.h b/include/gc/gc_disclaim.h index 186f18574..e2718b388 100644 --- a/include/gc/gc_disclaim.h +++ b/include/gc/gc_disclaim.h @@ -69,45 +69,6 @@ GC_API GC_ATTR_MALLOC GC_ATTR_ALLOC_SIZE(1) void * GC_CALL const struct GC_finalizer_closure * /* fc */) GC_ATTR_NONNULL(2); -#ifdef BUFFERED_FINALIZATION -/* This API is defined only if the library has been suitably compiled */ -/* (i.e. with ENABLE_DISCLAIM defined). */ - -/* Prepare the object kind used by GC_buffered_finalize_malloc. Call */ -/* it from your initialization code or, at least, at some point before */ -/* finalized allocations. The function is thread-safe. */ -GC_API void GC_CALL GC_init_buffered_finalization(void); - -/* Allocate an object which is to be finalized. */ -/* This function assumes the first word in the allocated block will */ -/* store the function pointer to the finalizer. */ -/* Allocations of this kind are added to a buffer which, when full, is */ -/* passed to a user supplied closure to invoke their finalizers. It is */ -/* the responsibility of the user to ensure these objects are finalized.*/ -/* This uses a dedicated object kind with a disclaim procedure, and is */ -/* more efficient than GC_register_finalizer and friends. */ -/* GC_init_buffered_finalization must be called before using this. */ -/* The collector will not reclaim the object during the GC cycle where */ -/* it was considered unreachable. In addition, objects reachable from */ -/* the finalizer will be protected from collection until the finalizer */ -/* has been run. */ -/* The disclaim procedure is not invoked in the leak-finding mode. */ -/* There is no debugging version of this allocation API. */ -GC_API GC_ATTR_MALLOC GC_ATTR_ALLOC_SIZE(1) void * GC_CALL - GC_buffered_finalize_malloc(size_t); - -GC_API GC_ATTR_MALLOC GC_ATTR_ALLOC_SIZE(2) void * GC_CALL - GC_buffered_finalize_memalign(size_t /* align */, size_t /* lb */); - -GC_API int GC_CALL GC_buffered_finalize_posix_memalign(void ** /* memptr */, size_t /* align */, - size_t /* lb */) GC_ATTR_NONNULL(1); - -GC_API void GC_CALL GC_finalize_objects(void); - -GC_API size_t GC_finalized_total(void); - -#endif - #ifdef __cplusplus } /* extern "C" */ #endif diff --git a/include/private/gc_priv.h b/include/private/gc_priv.h index 74aa27825..dbd73720f 100644 --- a/include/private/gc_priv.h +++ b/include/private/gc_priv.h @@ -341,26 +341,7 @@ typedef struct hblkhdr hdr; #include "gc/gc_inline.h" -<<<<<<< HEAD -#ifdef BUFFERED_FINALIZATION - -typedef struct GC_finalization_buffer_hdr GC_finalization_buffer_hdr; - -struct GC_finalization_buffer_hdr { - GC_finalization_buffer_hdr* next; -}; - -struct GC_current_buffer { - GC_finalization_buffer_hdr* hdr; - void** cursor; -}; - -GC_INNER void GC_maybe_spawn_finalize_thread(); -======= GC_INNER void GC_maybe_wake_finalizer_thread(); ->>>>>>> b44b0a3e (Revert to using old GC_finalize mechanism) - -#endif /*********************************/ /* */ @@ -1597,21 +1578,11 @@ struct _GC_arrays { # define GC_trace_buf_ptr GC_arrays._trace_buf_ptr int _trace_buf_ptr; # endif +# define GC_finalizer_thread_exists GC_arrays._fin_thread_exists + int _fin_thread_exists; # ifdef ENABLE_DISCLAIM # define GC_finalized_kind GC_arrays._finalized_kind unsigned _finalized_kind; -# ifdef BUFFERED_FINALIZATION -# define GC_fin_q_kind GC_arrays._fin_q_kind - unsigned _fin_q_kind; -# define GC_finalizer_buffer_head GC_arrays._fin_buffer_head - GC_finalization_buffer_hdr* _fin_buffer_head; -# define GC_finalizer_buffer_current GC_arrays._fin_buffer_current - struct GC_current_buffer _fin_buffer_current; -# define GC_finalizer_thread_exists GC_arrays._fin_thread_exists - int _fin_thread_exists; -# define GC_num_finalized GC_arrays._fin_total - unsigned _fin_total; -# endif # endif # define n_root_sets GC_arrays._n_root_sets # define GC_excl_table_entries GC_arrays._excl_table_entries diff --git a/misc.c b/misc.c index 4b78789eb..30058d0d1 100644 --- a/misc.c +++ b/misc.c @@ -1481,9 +1481,6 @@ GC_API void GC_CALL GC_init(void) GC_ASSERT(GC_bytes_allocd + GC_bytes_allocd_before_gc == 0); # endif -# ifdef BUFFERED_FINALIZATION - GC_init_buffered_finalization(); -# endif } GC_API void GC_CALL GC_enable_incremental(void) @@ -2136,11 +2133,6 @@ GC_API void GC_CALL GC_enable(void) LOCK(); GC_ASSERT(GC_dont_gc != 0); /* ensure no counter underflow */ GC_dont_gc--; -#ifndef BUFFERED_FINALIZATION - if (!GC_dont_gc && GC_heapsize > GC_heapsize_on_gc_disable) - WARN("Heap grown by %" WARN_PRIuPTR " KiB while GC was disabled\n", - (GC_heapsize - GC_heapsize_on_gc_disable) >> 10); -#endif UNLOCK(); } diff --git a/tests/gctest.c b/tests/gctest.c index 41c51f3d5..d64f9d55b 100644 --- a/tests/gctest.c +++ b/tests/gctest.c @@ -192,10 +192,6 @@ # define INIT_PERF_MEASUREMENT GC_start_performance_measurement() #endif -# ifdef BUFFERED_FINALIZATION -# include "gc/gc_disclaim.h" -# endif - #define GC_COND_INIT() \ INIT_FORK_SUPPORT; INIT_MANUAL_VDB_ALLOWED; INIT_PAGES_EXECUTABLE; \ GC_OPT_INIT; CHECK_GCLIB_VERSION; \ @@ -1674,31 +1670,6 @@ static void run_one_test(void) NOOP1_PTR(p); AO_fetch_and_add1(&collectable_count); } -# ifdef BUFFERED_FINALIZATION - { - size_t i; - void *p; - - GC_init(); - - p = GC_buffered_finalize_malloc(17); - CHECK_OUT_OF_MEMORY(p); - AO_fetch_and_add1(&collectable_count); - - for (i = sizeof(GC_word); i <= HBLKSIZE * 4; i *= 2) { - p = checkOOM(GC_buffered_finalize_memalign(i, 17)); - AO_fetch_and_add1(&collectable_count); - if ((GC_word)p % i != 0 || *(int *)p != 0) { - GC_printf("GC_buffered_finalize_posix_memalign(%u,17) produced incorrect result: %p\n", - (unsigned)i, p); - FAIL; - } - } - (void)GC_buffered_finalize_posix_memalign(&p, 64, 1); - CHECK_OUT_OF_MEMORY(p); - AO_fetch_and_add1(&collectable_count); - } -# endif # ifndef GC_NO_VALLOC { void *p = checkOOM(GC_valloc(78));