From: Ivan Maidanski Date: Tue, 16 Apr 2019 21:52:59 +0000 (+0300) Subject: Enable true incremental collection even if parallel marker is on X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=3c571c7ad66a90e33e4701afe3dc4d2113c60adc;p=gc Enable true incremental collection even if parallel marker is on Issue #151 (bdwgc). Because of the current limitation of the parallel marker implementation, it is not possible to interrupt the collection when performed by the parallel marker. This change allows to have the true incremental mode at the expense of disabling the parallel marker during most collection phases. By default, the old behavior (a generational collection with the parallel marker enabled) is preserved unless the client sets GC_time_limit to a value other than GC_TIME_UNLIMITED. * alloc.c [(!GC_TIME_LIMIT || CPPCHECK) && PARALLEL_MARK] (GC_time_limit): Set to GC_TIME_UNLIMITED; add comment. * alloc.c [PARALLEL_MARK] (GC_collect_a_little_inner): Temporarily set GC_parallel_mark_disabled to TRUE before GC_mark_some repeated invocation if GC_time_limit is not GC_TIME_UNLIMITED. * alloc.c [PARALLEL_MARK] (GC_stopped_mark): Temporarily set GC_parallel_mark_disabled to TRUE before GC_mark_some repeated invocation if stop_func is not GC_never_stop_func; add verbose logging if parallel marking is disabled temporarily. * doc/README.environment (GC_PAUSE_TIME_TARGET): Update the description (remove the limitation for the case when parallel marking is on). * doc/scale.md (The Parallel Marking Algorithm): Update the documentation regarding incremental mode. * include/gc.h (GC_parallel, GC_enable_incremental): Update the comment (remove the limitation on the incremental mode when parallel marking is on). * include/private/gc_priv.h [PARALLEL_MARK] (GC_parallel_mark_disabled): Declare global variable. * mark.c [PARALLEL_MARK] (GC_parallel_mark_disabled): Define. * mark.c [PARALLEL_MARK] (GC_mark_some_inner): Do not call GC_do_parallel_mark() if GC_parallel_mark_disabled; update comment. * pthread_support.c [PARALLEL_MARK] (GC_thr_init): Do not set GC_time_limit to GC_TIME_UNLIMITED if available_markers_m1 > 0; remove comment. * win32_threads.c [PARALLEL_MARK] (GC_thr_init): Likewise. --- diff --git a/alloc.c b/alloc.c index 3e005cd9..a0c2a1c3 100644 --- a/alloc.c +++ b/alloc.c @@ -168,6 +168,10 @@ GC_INNER int GC_CALLBACK GC_never_stop_func(void) unsigned long GC_time_limit = GC_TIME_LIMIT; /* We try to keep pause times from exceeding */ /* this by much. In milliseconds. */ +#elif defined(PARALLEL_MARK) + unsigned long GC_time_limit = GC_TIME_UNLIMITED; + /* The parallel marker cannot be interrupted for */ + /* now, so the time limit is absent by default. */ #else unsigned long GC_time_limit = 50; #endif @@ -657,10 +661,17 @@ GC_INNER void GC_collect_a_little_inner(int n) int i; int max_deficit = GC_rate * n; +# ifdef PARALLEL_MARK + if (GC_time_limit != GC_TIME_UNLIMITED) + GC_parallel_mark_disabled = TRUE; +# endif for (i = GC_deficit; i < max_deficit; i++) { if (GC_mark_some(NULL)) break; } +# ifdef PARALLEL_MARK + GC_parallel_mark_disabled = FALSE; +# endif if (i < max_deficit) { /* Need to finish a collection. */ @@ -797,12 +808,25 @@ STATIC GC_bool GC_stopped_mark(GC_stop_func stop_func) GC_noop6(0,0,0,0,0,0); GC_initiate_gc(); +# ifdef PARALLEL_MARK + if (stop_func != GC_never_stop_func) + GC_parallel_mark_disabled = TRUE; +# endif for (i = 0; !(*stop_func)(); i++) { if (GC_mark_some(GC_approx_sp())) { +# ifdef PARALLEL_MARK + if (GC_parallel && GC_parallel_mark_disabled) { + GC_COND_LOG_PRINTF("Stopped marking done after %d iterations" + " with disabled parallel marker\n", i); + } +# endif i = -1; break; } } +# ifdef PARALLEL_MARK + GC_parallel_mark_disabled = FALSE; +# endif if (i >= 0) { GC_COND_LOG_PRINTF("Abandoned stopped marking after" diff --git a/doc/README.environment b/doc/README.environment index 1806f331..16df12f5 100644 --- a/doc/README.environment +++ b/doc/README.environment @@ -135,10 +135,9 @@ GC_PAUSE_TIME_TARGET - Set the desired garbage collector pause time in work to compensate. The special value "999999" indicates that pause time is unlimited, and the incremental collector will behave completely like a simple - generational collector. If the collector is configured - for parallel marking, and run on a multiprocessor, - incremental collection should only be used with unlimited - pause time. + generational collector. Any value, except for the given + special one, disables parallel marker (almost fully) for + now. GC_FULL_FREQUENCY - Set the desired number of partial collections between full collections. Matters only if GC_incremental is set. diff --git a/doc/scale.md b/doc/scale.md index 67336848..7bf9eb15 100644 --- a/doc/scale.md +++ b/doc/scale.md @@ -86,10 +86,10 @@ It does require synchronization, but should be relatively rare. The sequential marking code is reused to process local mark stacks. Hence the amount of additional code required for parallel marking is minimal. -It should be possible to use generational collection in the presence of the -parallel collector, by calling `GC_enable_incremental`. This does not result -in fully incremental collection, since parallel mark phases cannot currently -be interrupted, and doing so may be too expensive. +It should be possible to use incremental/generational collection in the +presence of the parallel collector by calling `GC_enable_incremental`, but +the current implementation does not allow interruption of the parallel marker, +so the latter is mostly avoided if the client sets the collection time limit. Gcj-style mark descriptors do not currently mix with the combination of local allocation and incremental collection. They should work correctly with one or diff --git a/include/gc.h b/include/gc.h index 0778ad91..1866c3a6 100644 --- a/include/gc.h +++ b/include/gc.h @@ -94,10 +94,7 @@ GC_API GC_word GC_CALL GC_get_gc_no(void); /* PARALLEL_MARK defined, and if either */ /* GC_MARKERS (or GC_NPROCS) environment */ /* variable is set to > 1, or multiple cores */ - /* (processors) are available. */ - /* If GC_parallel is on (non-zero), incremental */ - /* collection is only partially functional, */ - /* and may not be desirable. The getter does */ + /* (processors) are available. The getter does */ /* not use or need synchronization (i.e. */ /* acquiring the GC lock). GC_parallel value */ /* is equal to the number of marker threads */ @@ -840,12 +837,11 @@ GC_API int GC_CALL GC_get_manual_vdb_allowed(void); /* dirty bits are available or most heap objects are pointer-free */ /* (atomic) or immutable. Don't use in leak finding mode. Ignored if */ /* GC_dont_gc is non-zero. Only the generational piece of this is */ -/* functional if GC_parallel is non-zero or if GC_time_limit is */ -/* GC_TIME_UNLIMITED. Causes thread-local variant of GC_gcj_malloc() */ -/* to revert to locked allocation. Must be called before any such */ -/* GC_gcj_malloc() calls. For best performance, should be called as */ -/* early as possible. On some platforms, calling it later may have */ -/* adverse effects. */ +/* functional if GC_time_limit is set to GC_TIME_UNLIMITED. Causes */ +/* thread-local variant of GC_gcj_malloc() to revert to locked */ +/* allocation. Must be called before any such GC_gcj_malloc() calls. */ +/* For best performance, should be called as early as possible. */ +/* On some platforms, calling it later may have adverse effects. */ /* Safe to call before GC_INIT(). Includes a GC_init() call. */ GC_API void GC_CALL GC_enable_incremental(void); diff --git a/include/private/gc_priv.h b/include/private/gc_priv.h index c084897c..0324a379 100644 --- a/include/private/gc_priv.h +++ b/include/private/gc_priv.h @@ -2569,6 +2569,9 @@ GC_INNER void *GC_store_debug_info_inner(void *p, word sz, const char *str, /* Number of mark threads we would like to have */ /* excluding the initiating thread. */ + GC_EXTERN GC_bool GC_parallel_mark_disabled; + /* A flag to temporarily avoid parallel marking.*/ + /* The mark lock and condition variable. If the GC lock is also */ /* acquired, the GC lock must be acquired first. The mark lock is */ /* used to both protect some variables used by the parallel */ diff --git a/mark.c b/mark.c index 41790621..7bf17d29 100644 --- a/mark.c +++ b/mark.c @@ -111,6 +111,8 @@ GC_INNER size_t GC_mark_stack_size = 0; /* that may be nonempty. */ /* Updated only by initiating */ /* thread. */ + + GC_INNER GC_bool GC_parallel_mark_disabled = FALSE; #endif GC_INNER mark_state_t GC_mark_state = MS_NONE; @@ -354,15 +356,13 @@ static void alloc_mark_stack(size_t); case MS_ROOTS_PUSHED: # ifdef PARALLEL_MARK - /* In the incremental GC case, this currently doesn't */ - /* quite do the right thing, since it runs to */ - /* completion. On the other hand, starting a */ - /* parallel marker is expensive, so perhaps it is */ - /* the right thing? */ /* Eventually, incremental marking should run */ /* asynchronously in multiple threads, without grabbing */ /* the allocation lock. */ - if (GC_parallel) { + /* For now, parallel marker is disabled if there is */ + /* a chance that marking could be interrupted by */ + /* a client-supplied time limit or custom stop function. */ + if (GC_parallel && !GC_parallel_mark_disabled) { GC_do_parallel_mark(); GC_ASSERT((word)GC_mark_stack_top < (word)GC_first_nonempty); GC_mark_stack_top = GC_mark_stack - 1; diff --git a/pthread_support.c b/pthread_support.c index 6f4f51dd..f33467e9 100644 --- a/pthread_support.c +++ b/pthread_support.c @@ -1302,8 +1302,6 @@ GC_INNER void GC_thr_init(void) GC_COND_LOG_PRINTF( "Single marker thread, turning off parallel marking\n"); } else { - /* Disable true incremental collection, but generational is OK. */ - GC_time_limit = GC_TIME_UNLIMITED; setup_mark_lock(); } # endif diff --git a/win32_threads.c b/win32_threads.c index 52f8a26c..f5a7209c 100644 --- a/win32_threads.c +++ b/win32_threads.c @@ -2582,7 +2582,6 @@ GC_INNER void GC_thr_init(void) } /* Check whether parallel mode could be enabled. */ - { if (GC_win32_dll_threads || available_markers_m1 <= 0) { /* Disable parallel marking. */ GC_parallel = FALSE; @@ -2603,10 +2602,7 @@ GC_INNER void GC_thr_init(void) || mark_cv == (HANDLE)0) ABORT("CreateEvent failed"); # endif - /* Disable true incremental collection, but generational is OK. */ - GC_time_limit = GC_TIME_UNLIMITED; } - } # endif /* PARALLEL_MARK */ GC_ASSERT(0 == GC_lookup_thread_inner(GC_main_thread));