[Git][ghc/ghc][wip/T22264-9.2] 8 commits: nonmoving: Don't show occupancy if we didn't collect live words
Ben Gamari (@bgamari)
gitlab at gitlab.haskell.org
Sun Oct 23 03:10:22 UTC 2022
Ben Gamari pushed to branch wip/T22264-9.2 at Glasgow Haskell Compiler / GHC
Commits:
5847390c by Ben Gamari at 2022-10-22T19:36:47+00:00
nonmoving: Don't show occupancy if we didn't collect live words
- - - - -
fe49be7b by Ben Gamari at 2022-10-22T19:36:47+00:00
rts/BlockAlloc: Allow disabling of internal assertions
These can be quite expensive and it is sometimes useful to compile a
DEBUG RTS without them.
- - - - -
ae910ecd by Ben Gamari at 2022-10-22T19:36:47+00:00
nonmoving: Fix style
- - - - -
9546afa3 by Ben Gamari at 2022-10-22T19:36:47+00:00
nonmoving: Fix innocuous warning in sweep
- - - - -
fd2421aa by Ben Gamari at 2022-10-22T19:36:47+00:00
Refactor nonmoving weak marking
- - - - -
999f5c4c by Ben Gamari at 2022-10-22T19:36:47+00:00
Weak shutdown
- - - - -
64b397ca by Ben Gamari at 2022-10-22T19:36:47+00:00
nonmoving: Handle new closures in nonmovingIsNowAlive
- - - - -
279279db by Ben Gamari at 2022-10-22T19:39:50+00:00
Revert "nonmoving: Handle new closures in nonmovingIsNowAlive"
This reverts commit 64b397cac504370d98580a8f482c972cf186d029.
- - - - -
8 changed files:
- rts/RtsStartup.c
- rts/sm/BlockAlloc.c
- rts/sm/NonMoving.c
- rts/sm/NonMovingCensus.c
- rts/sm/NonMovingCensus.h
- rts/sm/NonMovingMark.c
- rts/sm/NonMovingMark.h
- rts/sm/NonMovingSweep.c
Changes:
=====================================
rts/RtsStartup.c
=====================================
@@ -472,6 +472,7 @@ hs_exit_(bool wait_foreign)
for (g = 0; g < RtsFlags.GcFlags.generations; g++) {
runAllCFinalizers(generations[g].weak_ptr_list);
}
+ runAllCFinalizers(nonmoving_weak_ptr_list);
#if defined(RTS_USER_SIGNALS)
if (RtsFlags.MiscFlags.install_signal_handlers) {
=====================================
rts/sm/BlockAlloc.c
=====================================
@@ -27,6 +27,16 @@
static void initMBlock(void *mblock, uint32_t node);
+/*
+ * By default the DEBUG RTS is built with block allocator assertions
+ * enabled. However, these are quite expensive and consequently it can
+ * sometimes be useful to disable them if debugging an issue known to be
+ * elsewhere
+ */
+#if defined(DEBUG)
+#define BLOCK_ALLOC_DEBUG
+#endif
+
/* -----------------------------------------------------------------------------
Implementation notes
@@ -234,7 +244,7 @@ initGroup(bdescr *head)
last->link = head;
}
-#if defined(DEBUG)
+#if defined(BLOCK_ALLOC_DEBUG)
for (uint32_t i=0; i < head->blocks; i++) {
head[i].flags = 0;
}
@@ -568,7 +578,7 @@ allocAlignedGroupOnNode (uint32_t node, W_ n)
ASSERT(slop_low_blocks + slop_high_blocks + n == num_blocks);
-#if defined(DEBUG)
+#if defined(BLOCK_ALLOC_DEBUG)
checkFreeListSanity();
W_ free_before = countFreeList();
#endif
@@ -578,7 +588,7 @@ allocAlignedGroupOnNode (uint32_t node, W_ n)
ASSERT(countBlocks(bd) == num_blocks - slop_low_blocks);
}
-#if defined(DEBUG)
+#if defined(BLOCK_ALLOC_DEBUG)
ASSERT(countFreeList() == free_before + slop_low_blocks);
checkFreeListSanity();
#endif
@@ -586,7 +596,7 @@ allocAlignedGroupOnNode (uint32_t node, W_ n)
// At this point the bd should be aligned, but we may have slop on the high side
ASSERT((uintptr_t)bd->start % group_size == 0);
-#if defined(DEBUG)
+#if defined(BLOCK_ALLOC_DEBUG)
free_before = countFreeList();
#endif
@@ -595,7 +605,7 @@ allocAlignedGroupOnNode (uint32_t node, W_ n)
ASSERT(bd->blocks == n);
}
-#if defined(DEBUG)
+#if defined(BLOCK_ALLOC_DEBUG)
ASSERT(countFreeList() == free_before + slop_high_blocks);
checkFreeListSanity();
#endif
@@ -818,7 +828,7 @@ freeGroup(bdescr *p)
ASSERT(RELAXED_LOAD(&p->free) != (P_)-1);
-#if defined(DEBUG)
+#if defined(BLOCK_ALLOC_DEBUG)
for (uint32_t i=0; i < p->blocks; i++) {
p[i].flags = 0;
}
=====================================
rts/sm/NonMoving.c
=====================================
@@ -26,7 +26,7 @@
#include "NonMovingCensus.h"
#include "StablePtr.h" // markStablePtrTable
#include "Schedule.h" // markScheduler
-#include "Weak.h" // dead_weak_ptr_list
+#include "Weak.h" // scheduleFinalizers
struct NonmovingHeap nonmovingHeap;
@@ -891,37 +891,6 @@ static void nonmovingPrepareMark(void)
#endif
}
-// Mark weak pointers in the non-moving heap. They'll either end up in
-// dead_weak_ptr_list or stay in weak_ptr_list. Either way they need to be kept
-// during sweep. See `MarkWeak.c:markWeakPtrList` for the moving heap variant
-// of this.
-static void nonmovingMarkWeakPtrList(MarkQueue *mark_queue, StgWeak *dead_weak_ptr_list)
-{
- for (StgWeak *w = oldest_gen->weak_ptr_list; w; w = w->link) {
- markQueuePushClosure_(mark_queue, (StgClosure*)w);
- // Do not mark finalizers and values here, those fields will be marked
- // in `nonmovingMarkDeadWeaks` (for dead weaks) or
- // `nonmovingTidyWeaks` (for live weaks)
- }
-
- // We need to mark dead_weak_ptr_list too. This is subtle:
- //
- // - By the beginning of this GC we evacuated all weaks to the non-moving
- // heap (in `markWeakPtrList`)
- //
- // - During the scavenging of the moving heap we discovered that some of
- // those weaks are dead and moved them to `dead_weak_ptr_list`. Note that
- // because of the fact above _all weaks_ are in the non-moving heap at
- // this point.
- //
- // - So, to be able to traverse `dead_weak_ptr_list` and run finalizers we
- // need to mark it.
- for (StgWeak *w = dead_weak_ptr_list; w; w = w->link) {
- markQueuePushClosure_(mark_queue, (StgClosure*)w);
- nonmovingMarkDeadWeak(mark_queue, w);
- }
-}
-
void nonmovingCollect(StgWeak **dead_weaks, StgTSO **resurrected_threads)
{
#if defined(THREADED_RTS)
@@ -955,9 +924,16 @@ void nonmovingCollect(StgWeak **dead_weaks, StgTSO **resurrected_threads)
capabilities[n], true/*don't mark sparks*/);
}
markScheduler((evac_fn)markQueueAddRoot, mark_queue);
- nonmovingMarkWeakPtrList(mark_queue, *dead_weaks);
markStablePtrTable((evac_fn)markQueueAddRoot, mark_queue);
+ // The dead weak pointer list shouldn't contain any weaks in the
+ // nonmoving heap
+#if defined(DEBUG)
+ for (StgWeak *w = *dead_weaks; w; w = w->link) {
+ ASSERT(Bdescr((StgPtr) w)->gen != oldest_gen);
+ }
+#endif
+
// Mark threads resurrected during moving heap scavenging
for (StgTSO *tso = *resurrected_threads; tso != END_TSO_QUEUE; tso = tso->global_link) {
markQueuePushClosure_(mark_queue, (StgClosure*)tso);
@@ -983,8 +959,23 @@ void nonmovingCollect(StgWeak **dead_weaks, StgTSO **resurrected_threads)
// alive).
ASSERT(oldest_gen->old_weak_ptr_list == NULL);
ASSERT(nonmoving_old_weak_ptr_list == NULL);
- nonmoving_old_weak_ptr_list = oldest_gen->weak_ptr_list;
- oldest_gen->weak_ptr_list = NULL;
+ {
+ // Move both oldest_gen->weak_ptr_list and nonmoving_weak_ptr_list to
+ // nonmoving_old_weak_ptr_list
+ StgWeak **weaks = &oldest_gen->weak_ptr_list;
+ uint32_t n = 0;
+ while (*weaks) {
+ weaks = &(*weaks)->link;
+ n++;
+ }
+ debugTrace(DEBUG_nonmoving_gc, "%d new nonmoving weaks", n);
+ *weaks = nonmoving_weak_ptr_list;
+ nonmoving_old_weak_ptr_list = oldest_gen->weak_ptr_list;
+ nonmoving_weak_ptr_list = NULL;
+ oldest_gen->weak_ptr_list = NULL;
+ // At this point all weaks in the nonmoving generation are on
+ // nonmoving_old_weak_ptr_list
+ }
trace(TRACE_nonmoving_gc, "Finished nonmoving GC preparation");
// We are now safe to start concurrent marking
@@ -1046,7 +1037,6 @@ static void* nonmovingConcurrentMark(void *data)
return NULL;
}
-// TODO: Not sure where to put this function.
// Append w2 to the end of w1.
static void appendWeakList( StgWeak **w1, StgWeak *w2 )
{
@@ -1074,10 +1064,11 @@ static void nonmovingMark_(MarkQueue *mark_queue, StgWeak **dead_weaks, StgTSO *
// Set snapshot
nonmovingSegmentInfo(seg)->next_free_snap = seg->next_free;
n_filled++;
- if (seg->link)
+ if (seg->link) {
seg = seg->link;
- else
+ } else {
break;
+ }
}
// add filled segments to sweep_list
SET_SEGMENT_STATE(seg, FILLED_SWEEPING);
@@ -1086,6 +1077,9 @@ static void nonmovingMark_(MarkQueue *mark_queue, StgWeak **dead_weaks, StgTSO *
}
}
+ // Mark Weak#s
+ nonmovingMarkWeakPtrList(mark_queue);
+
// Do concurrent marking; most of the heap will get marked here.
nonmovingMarkThreadsWeaks(mark_queue);
@@ -1096,21 +1090,13 @@ static void nonmovingMark_(MarkQueue *mark_queue, StgWeak **dead_weaks, StgTSO *
if (sched_state > SCHED_RUNNING) {
// Note that we break our invariants here and leave segments in
// nonmovingHeap.sweep_list, don't free nonmoving_large_objects etc.
- // However because we won't be running mark-sweep in the final GC this
+ // However because we won't be running sweep in the final GC this
// is OK.
-
- // This is a RTS shutdown so we need to move our copy (snapshot) of
- // weaks (nonmoving_old_weak_ptr_list and nonmoving_weak_ptr_list) to
- // oldest_gen->threads to be able to run C finalizers in hs_exit_. Note
- // that there may be more weaks added to oldest_gen->threads since we
- // started mark, so we need to append our list to the tail of
- // oldest_gen->threads.
- appendWeakList(&nonmoving_old_weak_ptr_list, nonmoving_weak_ptr_list);
- appendWeakList(&oldest_gen->weak_ptr_list, nonmoving_old_weak_ptr_list);
- // These lists won't be used again so this is not necessary, but still
- nonmoving_old_weak_ptr_list = NULL;
- nonmoving_weak_ptr_list = NULL;
-
+ //
+ // However, we must move any weak pointers remaining on
+ // nonmoving_old_weak_ptr_list back to nonmoving_weak_ptr_list
+ // such that their C finalizers can be run by hs_exit_.
+ appendWeakList(&nonmoving_weak_ptr_list, nonmoving_old_weak_ptr_list);
goto finish;
}
@@ -1182,15 +1168,8 @@ static void nonmovingMark_(MarkQueue *mark_queue, StgWeak **dead_weaks, StgTSO *
nonmoving_old_threads = END_TSO_QUEUE;
}
- {
- StgWeak **weaks = &oldest_gen->weak_ptr_list;
- while (*weaks) {
- weaks = &(*weaks)->link;
- }
- *weaks = nonmoving_weak_ptr_list;
- nonmoving_weak_ptr_list = NULL;
- nonmoving_old_weak_ptr_list = NULL;
- }
+ nonmoving_weak_ptr_list = nonmoving_old_weak_ptr_list;
+ nonmoving_old_weak_ptr_list = NULL;
// Prune spark lists
// See Note [Spark management under the nonmoving collector].
@@ -1233,7 +1212,7 @@ static void nonmovingMark_(MarkQueue *mark_queue, StgWeak **dead_weaks, StgTSO *
traceConcSweepEnd();
#if defined(DEBUG)
if (RtsFlags.DebugFlags.nonmoving_gc)
- nonmovingPrintAllocatorCensus();
+ nonmovingPrintAllocatorCensus(true);
#endif
#if defined(TRACING)
if (RtsFlags.TraceFlags.nonmoving_gc)
=====================================
rts/sm/NonMovingCensus.c
=====================================
@@ -23,7 +23,7 @@
static struct NonmovingAllocCensus
nonmovingAllocatorCensus_(struct NonmovingAllocator *alloc, bool collect_live_words)
{
- struct NonmovingAllocCensus census = {0, 0, 0, 0};
+ struct NonmovingAllocCensus census = {collect_live_words, 0, 0, 0, 0};
for (struct NonmovingSegment *seg = alloc->filled;
seg != NULL;
@@ -47,7 +47,7 @@ nonmovingAllocatorCensus_(struct NonmovingAllocator *alloc, bool collect_live_wo
census.n_active_segs++;
unsigned int n = nonmovingSegmentBlockCount(seg);
for (unsigned int i=0; i < n; i++) {
- if (nonmovingGetMark(seg, i)) {
+ if (nonmovingGetMark(seg, i) == nonmovingMarkEpoch) {
StgClosure *c = (StgClosure *) nonmovingSegmentGetBlock(seg, i);
if (collect_live_words)
census.n_live_words += closure_sizeW(c);
@@ -88,28 +88,51 @@ nonmovingAllocatorCensus(struct NonmovingAllocator *alloc)
}
-void nonmovingPrintAllocatorCensus()
+static void print_alloc_census(int i, struct NonmovingAllocCensus census)
{
- if (!RtsFlags.GcFlags.useNonmoving)
- return;
+ uint32_t blk_size = 1 << (i + NONMOVING_ALLOCA0);
+ int sz_min = 1 << (i + NONMOVING_ALLOCA0 - 1);
+ int sz_max = 1 << (i + NONMOVING_ALLOCA0);
+ (void) sz_min; (void) sz_max;
- for (int i=0; i < NONMOVING_ALLOCA_CNT; i++) {
- struct NonmovingAllocCensus census =
- nonmovingAllocatorCensus(nonmovingHeap.allocators[i]);
-
- uint32_t blk_size = 1 << (i + NONMOVING_ALLOCA0);
+ if (census.collected_live_words) {
// We define occupancy as the fraction of space that is used for useful
// data (that is, live and not slop).
double occupancy = 100.0 * census.n_live_words * sizeof(W_)
/ (census.n_live_blocks * blk_size);
if (census.n_live_blocks == 0) occupancy = 100;
(void) occupancy; // silence warning if !DEBUG
- debugTrace(DEBUG_nonmoving_gc, "Allocator %d (%d bytes - %d bytes): "
- "%d active segs, %d filled segs, %d live blocks, %d live words "
- "(%2.1f%% occupancy)",
- i, 1 << (i + NONMOVING_ALLOCA0 - 1), 1 << (i + NONMOVING_ALLOCA0),
- census.n_active_segs, census.n_filled_segs, census.n_live_blocks, census.n_live_words,
+ debugTrace(DEBUG_nonmoving_gc,
+ "Allocator %d (%d bytes - %d bytes): "
+ "%"PRIu32" active segs, %"PRIu32" filled segs, %"PRIu32" live blocks, "
+ "%"PRIu32" live words (%2.1f%% occupancy)",
+ i, sz_min, sz_max,
+ census.n_active_segs,
+ census.n_filled_segs,
+ census.n_live_blocks,
+ census.n_live_words,
occupancy);
+ } else {
+ debugTrace(DEBUG_nonmoving_gc,
+ "Allocator %d (%d bytes - %d bytes): "
+ "%"PRIu32" active segs, %"PRIu32" filled segs, %"PRIu32" live blocks",
+ i, sz_min, sz_max,
+ census.n_active_segs,
+ census.n_filled_segs,
+ census.n_live_blocks);
+ }
+}
+
+void nonmovingPrintAllocatorCensus(bool collect_live_words)
+{
+ if (!RtsFlags.GcFlags.useNonmoving)
+ return;
+
+ for (int i=0; i < NONMOVING_ALLOCA_CNT; i++) {
+ struct NonmovingAllocCensus census =
+ nonmovingAllocatorCensus_(nonmovingHeap.allocators[i], collect_live_words);
+
+ print_alloc_census(i, census);
}
}
=====================================
rts/sm/NonMovingCensus.h
=====================================
@@ -11,6 +11,7 @@
#include "NonMoving.h"
struct NonmovingAllocCensus {
+ bool collected_live_words;
uint32_t n_active_segs;
uint32_t n_filled_segs;
uint32_t n_live_blocks;
@@ -24,5 +25,5 @@ nonmovingAllocatorCensusWithWords(struct NonmovingAllocator *alloc);
struct NonmovingAllocCensus
nonmovingAllocatorCensus(struct NonmovingAllocator *alloc);
-void nonmovingPrintAllocatorCensus(void);
+void nonmovingPrintAllocatorCensus(bool collect_live_words);
void nonmovingTraceAllocatorCensus(void);
=====================================
rts/sm/NonMovingMark.c
=====================================
@@ -1824,6 +1824,15 @@ static bool nonmovingIsNowAlive (StgClosure *p)
}
}
+// Mark all Weak#s on nonmoving_old_weak_ptr_list.
+void nonmovingMarkWeakPtrList (struct MarkQueue_ *queue)
+{
+ ASSERT(nonmoving_weak_ptr_list == NULL);
+ for (StgWeak *w = nonmoving_old_weak_ptr_list; w != NULL; w = w->link) {
+ mark_closure(queue, (StgClosure *) w, NULL);
+ }
+}
+
// Non-moving heap variant of `tidyWeakList`
bool nonmovingTidyWeaks (struct MarkQueue_ *queue)
{
@@ -1832,6 +1841,9 @@ bool nonmovingTidyWeaks (struct MarkQueue_ *queue)
StgWeak **last_w = &nonmoving_old_weak_ptr_list;
StgWeak *next_w;
for (StgWeak *w = nonmoving_old_weak_ptr_list; w != NULL; w = next_w) {
+ // This should have been marked by nonmovingMarkWeaks
+ ASSERT(nonmovingIsNowAlive((StgClosure *) w));
+
if (w->header.info == &stg_DEAD_WEAK_info) {
// finalizeWeak# was called on the weak
next_w = w->link;
@@ -1875,7 +1887,8 @@ void nonmovingMarkDeadWeak (struct MarkQueue_ *queue, StgWeak *w)
void nonmovingMarkLiveWeak (struct MarkQueue_ *queue, StgWeak *w)
{
- ASSERT(nonmovingIsNowAlive((P_)w));
+ ASSERT(nonmovingIsNowAlive((StgClosure *) w));
+ ASSERT(nonmovingIsNowAlive((StgClosure *) w->key));
markQueuePushClosure_(queue, w->value);
markQueuePushClosure_(queue, w->finalizer);
markQueuePushClosure_(queue, w->cfinalizers);
@@ -1889,7 +1902,7 @@ void nonmovingMarkDeadWeaks (struct MarkQueue_ *queue, StgWeak **dead_weaks)
{
StgWeak *next_w;
for (StgWeak *w = nonmoving_old_weak_ptr_list; w; w = next_w) {
- ASSERT(!nonmovingIsNowAlive((P_)(w->key)));
+ ASSERT(!nonmovingIsNowAlive(w->key));
nonmovingMarkDeadWeak(queue, w);
next_w = w->link;
w->link = *dead_weaks;
=====================================
rts/sm/NonMovingMark.h
=====================================
@@ -157,6 +157,7 @@ void initMarkQueue(MarkQueue *queue);
void freeMarkQueue(MarkQueue *queue);
void nonmovingMark(struct MarkQueue_ *restrict queue);
+void nonmovingMarkWeakPtrList(struct MarkQueue_ *queue);
bool nonmovingTidyWeaks(struct MarkQueue_ *queue);
void nonmovingTidyThreads(void);
void nonmovingMarkDeadWeaks(struct MarkQueue_ *queue, StgWeak **dead_weak_ptr_list);
=====================================
rts/sm/NonMovingSweep.c
=====================================
@@ -286,7 +286,7 @@ void nonmovingSweepMutLists()
for (bdescr *bd = old_mut_list; bd; bd = bd->link) {
for (StgPtr p = bd->start; p < bd->free; p++) {
StgClosure **q = (StgClosure**)p;
- ASSERT(Bdescr(*q)->gen == oldest_gen);
+ ASSERT(Bdescr((StgPtr) *q)->gen == oldest_gen);
if (nonmovingIsAlive(*q) && !is_closure_clean(*q)) {
recordMutableCap(*q, cap, oldest_gen->no);
}
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/5674bd0a899b9406e2a9349afb54244002e79611...279279db1bc106c3e0008cb056d7421b55aac4e4
--
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/5674bd0a899b9406e2a9349afb54244002e79611...279279db1bc106c3e0008cb056d7421b55aac4e4
You're receiving this email because of your account on gitlab.haskell.org.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-commits/attachments/20221022/b75fe975/attachment-0001.html>
More information about the ghc-commits
mailing list