From 4601763a7f6e0c9a5feb0e339f271a6585eb8338 Mon Sep 17 00:00:00 2001 From: Ivan Maidanski Date: Wed, 2 Aug 2017 01:13:37 +0300 Subject: [PATCH] Use heap-allocated memory for local mark stack of non-marker thread Issue #159 (bdwgc). The memory is allocated using GET_MEM to avoid to make sure that it is not accidentally scanned by the collector. * include/private/gcconfig.h [!PARALLEL_MARK && THREADS] (MIN_STACK_SIZE): Do not define. * include/private/gcconfig.h [PARALLEL_MARK] (MIN_STACK_SIZE): Add comment. * mark.c [PARALLEL_MARK] (main_local_mark_stack): New static variable. * mark.c [PARALLEL_MARK] (GC_wait_for_markers_init): Declare bytes_to_get local variable; allocate local mark stack (using GET_MEM) and set main_local_mark_stack (if not yet allocated by the parent process). * mark [PARALLEL_MARK] (GC_do_parallel_mark): Remove local_mark_stack huge local variable; use main_local_mark_stack instead of local_mark_stack. * pthread_support.c [GC_ASSERTIONS] (WRAP_FUNC(pthread_create)): Use hard-coded value (65536) instead of MIN_STACK_SIZE in GC_ASSERT. * tests/test.c [GC_PTHREADS && DEFAULT_STACK_MAYBE_SMALL] (fork_a_thread): Do not set stack size for the created threads. * tests/test.c [GC_PTHREADS] (main): Do not check DEFAULT_STACK_MAYBE_SMALL. --- include/private/gcconfig.h | 3 +-- mark.c | 22 ++++++++++++++++++---- pthread_support.c | 2 +- tests/test.c | 21 ++------------------- 4 files changed, 22 insertions(+), 26 deletions(-) diff --git a/include/private/gcconfig.h b/include/private/gcconfig.h index 39bc96a4..8f7f42fc 100644 --- a/include/private/gcconfig.h +++ b/include/private/gcconfig.h @@ -3005,9 +3005,8 @@ #endif #ifdef PARALLEL_MARK + /* The minimum stack size for a marker thread. */ # define MIN_STACK_SIZE (8 * HBLKSIZE * sizeof(word)) -#elif defined(THREADS) -# define MIN_STACK_SIZE 65536 #endif #if defined(UNIX_LIKE) && defined(THREADS) && !defined(NO_CANCEL_SAFE) \ diff --git a/mark.c b/mark.c index 17fb511e..179cfb57 100644 --- a/mark.c +++ b/mark.c @@ -929,6 +929,8 @@ STATIC unsigned GC_active_count = 0; /* Number of active helpers. */ GC_INNER word GC_mark_no = 0; +static mse *main_local_mark_stack; + #ifdef LINT2 # define LOCAL_MARK_STACK_SIZE (HBLKSIZE / 8) #else @@ -947,6 +949,21 @@ GC_INNER void GC_wait_for_markers_init(void) if (GC_markers_m1 == 0) return; + /* Allocate the local mark stack for the thread that holds GC lock. */ +# ifndef CAN_HANDLE_FORK + GC_ASSERT(NULL == main_local_mark_stack); +# else + if (NULL == main_local_mark_stack) +# endif + { + size_t bytes_to_get = + ROUNDUP_PAGESIZE_IF_MMAP(LOCAL_MARK_STACK_SIZE * sizeof(mse)); + main_local_mark_stack = (mse *)GET_MEM(bytes_to_get); + if (NULL == main_local_mark_stack) + ABORT("Insufficient memory for main local_mark_stack"); + GC_add_to_our_memory((ptr_t)main_local_mark_stack, bytes_to_get); + } + /* Reuse marker lock and builders count to synchronize */ /* marker threads startup. */ GC_acquire_mark_lock(); @@ -1188,9 +1205,6 @@ STATIC void GC_mark_local(mse *local_mark_stack, int id) /* empty. */ STATIC void GC_do_parallel_mark(void) { - mse local_mark_stack[LOCAL_MARK_STACK_SIZE]; - /* Note: local_mark_stack is quite big (up to 128 KiB). */ - GC_acquire_mark_lock(); GC_ASSERT(I_HOLD_LOCK()); /* This could be a GC_ASSERT, but it seems safer to keep it on */ @@ -1206,7 +1220,7 @@ STATIC void GC_do_parallel_mark(void) GC_release_mark_lock(); GC_notify_all_marker(); /* Wake up potential helpers. */ - GC_mark_local(local_mark_stack, 0); + GC_mark_local(main_local_mark_stack, 0); GC_acquire_mark_lock(); GC_help_wanted = FALSE; /* Done; clean up. */ diff --git a/pthread_support.c b/pthread_support.c index 6f80a41c..cb31824a 100644 --- a/pthread_support.c +++ b/pthread_support.c @@ -1789,7 +1789,7 @@ GC_API int WRAP_FUNC(pthread_create)(pthread_t *new_thread, # endif stack_size = 1000000; } - GC_ASSERT(stack_size >= MIN_STACK_SIZE); + GC_ASSERT(stack_size >= 65536); /* Our threads may need to do some work for the GC. */ /* Ridiculously small threads won't work, and they */ /* probably wouldn't work anyway. */ diff --git a/tests/test.c b/tests/test.c index 4604ca77..be7d57a5 100644 --- a/tests/test.c +++ b/tests/test.c @@ -537,25 +537,8 @@ void check_marks_int_list(sexpr x) { pthread_t t; int code; -# ifdef DEFAULT_STACK_MAYBE_SMALL - pthread_attr_t attr; - code = pthread_attr_init(&attr); - if (code != 0) { - GC_printf("pthread_attr_init failed, error=%d\n", code); - FAIL; - } - code = pthread_attr_setstacksize(&attr, MIN_STACK_SIZE); - if (code != 0) { - GC_printf("pthread_attr_setstacksize(MIN_STACK_SIZE) failed," - " error=%d\n", code); - FAIL; - } - code = pthread_create(&t, &attr, tiny_reverse_test, 0); - (void)pthread_attr_destroy(&attr); -# else - code = pthread_create(&t, NULL, tiny_reverse_test, 0); -# endif + code = pthread_create(&t, NULL, tiny_reverse_test, 0); if (code != 0) { GC_printf("Small thread creation failed %d\n", code); FAIL; @@ -2201,7 +2184,7 @@ int main(void) } # if defined(GC_IRIX_THREADS) || defined(GC_FREEBSD_THREADS) \ || defined(GC_DARWIN_THREADS) || defined(GC_AIX_THREADS) \ - || defined(GC_OPENBSD_THREADS) || defined(DEFAULT_STACK_MAYBE_SMALL) + || defined(GC_OPENBSD_THREADS) if ((code = pthread_attr_setstacksize(&attr, 1000 * 1024)) != 0) { GC_printf("pthread_attr_setstacksize failed, error=%d\n", code); FAIL; -- 2.40.0