[Git][ghc/ghc][wip/T22264-9.2] 5 commits: nonmoving: Fix handling of weak pointers
Ben Gamari (@bgamari)
gitlab at gitlab.haskell.org
Tue Dec 6 15:59:30 UTC 2022
Ben Gamari pushed to branch wip/T22264-9.2 at Glasgow Haskell Compiler / GHC
Commits:
079ac21f by Ben Gamari at 2022-12-06T10:59:21-05:00
nonmoving: Fix handling of weak pointers
This fixes an interaction between aging and weak pointer handling which
prevented the finalization of some weak pointers. In particular, weak
pointers could have their keys incorrectly marked by the preparatory
collector, preventing their finalization by the subsequent concurrent
collection.
While in the area, we also significantly improve the assertions
regarding weak pointers.
Fixes #22327.
(cherry picked from commit cab678fc11d0a3f28fbf2210c4c0bb04eb52997d)
- - - - -
9fc7df1d by Ben Gamari at 2022-12-06T10:59:21-05:00
nonmoving: Handle new closures in nonmovingIsNowAlive
(cherry picked from commit 36ca160d0f199a688cf5fbc91d4bb92d2d4ea14e)
- - - - -
b9c641ba by Ben Gamari at 2022-12-06T10:59:21-05:00
nonmoving: Don't clobber update rem sets of old capabilities
Previously `storageAddCapabilities` (called by `setNumCapabilities`) would
clobber the update remembered sets of existing capabilities when
increasing the capability count. Fix this by only initializing the
update remembered sets of the newly-created capabilities.
(cherry picked from commit 8b64aff0fa978c762dfae8df235dd2b2a340656a)
- - - - -
16f79f44 by Ben Gamari at 2022-12-06T10:59:22-05:00
nonmoving: Add missing write barriers in selector optimisation
This fixes the selector optimisation, adding a few write barriers which
are necessary for soundness.
(cherry picked from commit dde67d6e32ecff0e400f98213d42ae790babac09)
- - - - -
7bf7a61c by Ben Gamari at 2022-12-06T10:59:22-05:00
nonmoving: Don't push if nonmoving collector isn't enabled
(cherry picked from commit 8adc1750c02e596b4014d2837b4eb3d76bd130f2)
- - - - -
8 changed files:
- rts/RtsStartup.c
- rts/sm/Evac.c
- rts/sm/MarkWeak.c
- rts/sm/NonMoving.c
- rts/sm/NonMoving.h
- rts/sm/NonMovingMark.c
- rts/sm/NonMovingMark.h
- rts/sm/Storage.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/Evac.c
=====================================
@@ -1246,13 +1246,18 @@ selector_chain:
bd = Bdescr((StgPtr)p);
if (HEAP_ALLOCED_GC(p)) {
+ uint16_t flags = RELAXED_LOAD(&bd->flags);
// If the THUNK_SELECTOR is in to-space or in a generation that we
// are not collecting, then bale out early. We won't be able to
// save any space in any case, and updating with an indirection is
// trickier in a non-collected gen: we would have to update the
// mutable list.
- if (RELAXED_LOAD(&bd->flags) & (BF_EVACUATED | BF_NONMOVING)) {
+ if (flags & (BF_EVACUATED | BF_NONMOVING)) {
unchain_thunk_selectors(prev_thunk_selector, (StgClosure *)p);
+ if (flags & BF_NONMOVING) {
+ // See Note [Non-moving GC: Marking evacuated objects].
+ markQueuePushClosureGC(&gct->cap->upd_rem_set.queue, (StgClosure*) p);
+ }
*q = (StgClosure *)p;
// shortcut, behave as for: if (evac) evacuate(q);
if (evac && bd->gen_no < gct->evac_gen_no) {
@@ -1267,7 +1272,7 @@ selector_chain:
// (scavenge_mark_stack doesn't deal with IND). BEWARE! This
// bit is very tricky to get right. If you make changes
// around here, test by compiling stage 3 with +RTS -c -RTS.
- if (bd->flags & BF_MARKED) {
+ if (flags & BF_MARKED) {
// must call evacuate() to mark this closure if evac==true
*q = (StgClosure *)p;
if (evac) evacuate(q);
@@ -1307,6 +1312,12 @@ selector_chain:
// - undo the chain we've built to point to p.
SET_INFO((StgClosure *)p, (const StgInfoTable *)info_ptr);
RELEASE_STORE(q, (StgClosure *) p);
+ if (Bdescr((StgPtr)p)->flags & BF_NONMOVING) {
+ // See Note [Non-moving GC: Marking evacuated objects].
+ // TODO: This really shouldn't be necessary since whoever won
+ // the race should have pushed
+ markQueuePushClosureGC(&gct->cap->upd_rem_set.queue, (StgClosure*) p);
+ }
if (evac) evacuate(q);
unchain_thunk_selectors(prev_thunk_selector, (StgClosure *)p);
return;
@@ -1397,6 +1408,11 @@ selector_loop:
case THUNK_SELECTOR:
// Use payload to make a list of thunk selectors, to be
// used in unchain_thunk_selectors
+ //
+ // FIXME: This seems racy; should we lock this selector to
+ // ensure that another thread doesn't clobber this node
+ // of the chain. This would result in some previous
+ // selectors not being updated when we unchain.
RELAXED_STORE(&((StgClosure*)p)->payload[0], (StgClosure *)prev_thunk_selector);
prev_thunk_selector = p;
p = (StgSelector*)val;
@@ -1421,6 +1437,12 @@ selector_loop:
// eval_thunk_selector(), because we know val is not
// a THUNK_SELECTOR.
if (evac) evacuate(q);
+
+ if (isNonmovingClosure(*q)) {
+ // See Note [Non-moving GC: Marking evacuated objects].
+ markQueuePushClosureGC(&gct->cap->upd_rem_set.queue, (StgClosure*) *q);
+ }
+
return;
}
@@ -1465,6 +1487,10 @@ selector_loop:
// recurse indefinitely, so we impose a depth bound.
// See Note [Selector optimisation depth limit].
if (gct->thunk_selector_depth >= MAX_THUNK_SELECTOR_DEPTH) {
+ if (isNonmovingClosure((StgClosure *) p)) {
+ // See Note [Non-moving GC: Marking evacuated objects].
+ markQueuePushClosureGC(&gct->cap->upd_rem_set.queue, (StgClosure*) p);
+ }
goto bale_out;
}
@@ -1511,5 +1537,9 @@ bale_out:
if (evac) {
copy(q,(const StgInfoTable *)info_ptr,(StgClosure *)p,THUNK_SELECTOR_sizeW(),bd->dest_no);
}
+ if (isNonmovingClosure(*q)) {
+ // See Note [Non-moving GC: Marking evacuated objects].
+ markQueuePushClosureGC(&gct->cap->upd_rem_set.queue, *q);
+ }
unchain_thunk_selectors(prev_thunk_selector, *q);
}
=====================================
rts/sm/MarkWeak.c
=====================================
@@ -50,7 +50,7 @@
- weak_stage == WeakPtrs
- We process all the weak pointers whos keys are alive (evacuate
+ We process all the weak pointers whose keys are alive (evacuate
their values and finalizers), and repeat until we can find no new
live keys. If no live keys are found in this pass, then we
evacuate the finalizers of all the dead weak pointers in order to
@@ -82,12 +82,46 @@ static bool tidyWeakList (generation *gen);
static bool resurrectUnreachableThreads (generation *gen, StgTSO **resurrected_threads);
static void tidyThreadList (generation *gen);
+/*
+ * Note [Weak pointer processing and the non-moving GC]
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ * When using the non-moving GC we defer weak pointer processing
+ * until the concurrent marking phase as weaks in the non-moving heap may be
+ * keyed on objects living in the non-moving generation. To accomplish this
+ * initWeakForGC keeps all weak pointers on oldest_gen->weak_ptr_list, where
+ * nonmovingCollect will find them. From there they will be moved to
+ * nonmoving_old_weak_ptr_list. During the mark loop we will move weaks with
+ * reachable keys to nonmoving_weak_ptr_list. At the end of concurrent marking
+ * we tidy the weak list (in nonmovingTidyWeakList) and perform another set of
+ * marking as necessary, just as is done in tidyWeakList.
+ *
+ * Note that this treatment takes advantage of the fact that we usually need
+ * not worry about Weak#s living in the non-moving heap but being keyed on an
+ * object in the moving heap since the Weak# must be strictly older than the
+ * key. Such objects would otherwise pose a problem since the non-moving
+ * collector would be unable to safely determine the liveness of the key.
+ * In the rare case that we *do* see such a key (e.g. in the case of a
+ * pinned ByteArray# living in a partially-filled accumulator block)
+ * the nonmoving collector assumes that it is live.
+ *
+ */
+
+/*
+ * Prepare the weak object lists for GC. Specifically, reset weak_stage
+ * and move all generations' `weak_ptr_list`s to `old_weak_ptr_list`.
+ * Weaks with live keys will later be moved back to `weak_ptr_list` by
+ * `tidyWeakList`.
+ */
void
initWeakForGC(void)
{
- uint32_t g;
+ uint32_t oldest = N;
+ if (RtsFlags.GcFlags.useNonmoving && N == oldest_gen->no) {
+ // See Note [Weak pointer processing and the non-moving GC].
+ oldest = oldest_gen->no - 1;
+ }
- for (g = 0; g <= N; g++) {
+ for (uint32_t g = 0; g <= oldest; g++) {
generation *gen = &generations[g];
gen->old_weak_ptr_list = gen->weak_ptr_list;
gen->weak_ptr_list = NULL;
@@ -96,6 +130,14 @@ initWeakForGC(void)
weak_stage = WeakThreads;
}
+/*
+ * Walk the weak pointer lists after having finished a round of scavenging,
+ * tidying the weak (and possibly thread) lists (depending upon the current
+ * weak_stage).
+ *
+ * Returns true if new live weak pointers were found, implying that another
+ * round of scavenging is necessary.
+ */
bool
traverseWeakPtrList(StgWeak **dead_weak_ptr_list, StgTSO **resurrected_threads)
{
@@ -182,6 +224,11 @@ traverseWeakPtrList(StgWeak **dead_weak_ptr_list, StgTSO **resurrected_threads)
}
}
+/*
+ * Deal with weak pointers with unreachable keys after GC has concluded.
+ * This means marking the finalizer (and possibly value) in preparation for
+ * later finalization.
+ */
static void collectDeadWeakPtrs (generation *gen, StgWeak **dead_weak_ptr_list)
{
StgWeak *w, *next_w;
@@ -198,6 +245,10 @@ static void collectDeadWeakPtrs (generation *gen, StgWeak **dead_weak_ptr_list)
}
}
+/*
+ * Deal with threads left on the old_threads list after GC has concluded,
+ * moving them onto the resurrected_threads list where appropriate.
+ */
static bool resurrectUnreachableThreads (generation *gen, StgTSO **resurrected_threads)
{
StgTSO *t, *tmp, *next;
@@ -233,8 +284,21 @@ static bool resurrectUnreachableThreads (generation *gen, StgTSO **resurrected_t
return flag;
}
+/*
+ * Walk over the `old_weak_ptr_list` of the given generation and:
+ *
+ * - remove any DEAD_WEAKs
+ * - move any weaks with reachable keys to the `weak_ptr_list` of the
+ * appropriate to-space and mark the weak's value and finalizer.
+ */
static bool tidyWeakList(generation *gen)
{
+ if (RtsFlags.GcFlags.useNonmoving && gen == oldest_gen) {
+ // See Note [Weak pointer processing and the non-moving GC].
+ ASSERT(gen->old_weak_ptr_list == NULL);
+ return false;
+ }
+
StgWeak *w, **last_w, *next_w;
const StgInfoTable *info;
StgClosure *new;
@@ -322,6 +386,10 @@ static bool tidyWeakList(generation *gen)
return flag;
}
+/*
+ * Walk over the `old_threads` list of the given generation and move any
+ * reachable threads onto the `threads` list.
+ */
static void tidyThreadList (generation *gen)
{
StgTSO *t, *tmp, *next, **prev;
@@ -381,6 +449,10 @@ static void checkWeakPtrSanity(StgWeak *hd, StgWeak *tl)
}
#endif
+/*
+ * Traverse the capabilities' local new-weak-pointer lists at the beginning of
+ * GC and move them to the nursery's weak_ptr_list.
+ */
void collectFreshWeakPtrs()
{
uint32_t i;
=====================================
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;
@@ -244,6 +244,9 @@ Mutex concurrent_coll_finished_lock;
* how we use the DIRTY flags associated with MUT_VARs and TVARs to improve
* barrier efficiency.
*
+ * - Note [Weak pointer processing and the non-moving GC] (MarkWeak.c) describes
+ * how weak pointers are handled when the non-moving GC is in use.
+ *
* [ueno 2016]:
* Katsuhiro Ueno and Atsushi Ohori. 2016. A fully concurrent garbage
* collector for functional programs on multicore processors. SIGPLAN Not. 51,
@@ -282,8 +285,8 @@ Mutex concurrent_coll_finished_lock;
* was (unsurprisingly) also found to result in significant amounts of
* unnecessary copying.
*
- * Consequently, we now allow aging. Aging allows the preparatory GC leading up
- * to a major collection to evacuate some objects into the young generation.
+ * Consequently, we now allow "aging", allows the preparatory GC leading up
+ * to a major collection to evacuate objects into the young generation.
* However, this introduces the following tricky case that might arise after
* we have finished the preparatory GC:
*
@@ -886,37 +889,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)
@@ -950,9 +922,17 @@ 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);
+ nonmovingMarkWeakPtrList(mark_queue);
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);
@@ -978,8 +958,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
@@ -1041,7 +1036,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 )
{
@@ -1081,6 +1075,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);
@@ -1091,21 +1088,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;
}
@@ -1177,15 +1166,9 @@ 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;
- }
+ // At this point point any weak that remains on nonmoving_old_weak_ptr_list
+ // has a dead key.
+ nonmoving_old_weak_ptr_list = NULL;
// Prune spark lists
// See Note [Spark management under the nonmoving collector].
=====================================
rts/sm/NonMoving.h
=====================================
@@ -289,20 +289,17 @@ INLINE_HEADER void nonmovingSetClosureMark(StgPtr p)
nonmovingSetMark(nonmovingGetSegment(p), nonmovingGetBlockIdx(p));
}
-// TODO: Audit the uses of these
-/* Was the given closure marked this major GC cycle? */
-INLINE_HEADER bool nonmovingClosureMarkedThisCycle(StgPtr p)
+INLINE_HEADER uint8_t nonmovingGetClosureMark(StgPtr p)
{
struct NonmovingSegment *seg = nonmovingGetSegment(p);
nonmoving_block_idx blk_idx = nonmovingGetBlockIdx(p);
- return nonmovingGetMark(seg, blk_idx) == nonmovingMarkEpoch;
+ return nonmovingGetMark(seg, blk_idx);
}
-INLINE_HEADER bool nonmovingClosureMarked(StgPtr p)
+/* Was the given closure marked this major GC cycle? */
+INLINE_HEADER bool nonmovingClosureMarkedThisCycle(StgPtr p)
{
- struct NonmovingSegment *seg = nonmovingGetSegment(p);
- nonmoving_block_idx blk_idx = nonmovingGetBlockIdx(p);
- return nonmovingGetMark(seg, blk_idx) != 0;
+ return nonmovingGetClosureMark(p) == nonmovingMarkEpoch;
}
// Can be called during a major collection to determine whether a particular
@@ -338,7 +335,7 @@ INLINE_HEADER bool nonmovingClosureBeingSwept(StgClosure *p)
INLINE_HEADER bool isNonmovingClosure(StgClosure *p)
{
- return !HEAP_ALLOCED_GC(p) || Bdescr((P_)p)->flags & BF_NONMOVING;
+ return RtsFlags.GcFlags.useNonmoving && (!HEAP_ALLOCED_GC(p) || Bdescr((P_)p)->flags & BF_NONMOVING);
}
#if defined(DEBUG)
=====================================
rts/sm/NonMovingMark.c
=====================================
@@ -35,6 +35,9 @@ static void trace_PAP_payload (MarkQueue *queue,
StgClosure *fun,
StgClosure **payload,
StgWord size);
+#if defined(DEBUG)
+static bool is_nonmoving_weak(StgWeak *weak);
+#endif
// How many Array# entries to add to the mark queue at once?
#define MARK_ARRAY_CHUNK_LENGTH 128
@@ -623,6 +626,16 @@ void updateRemembSetPushThunkEager(Capability *cap,
}
break;
}
+ case THUNK_SELECTOR:
+ {
+ StgSelector *sel = (StgSelector *) thunk;
+ if (check_in_nonmoving_heap(sel->selectee)) {
+ // Don't bother to push origin; it makes the barrier needlessly
+ // expensive with little benefit.
+ push_closure(queue, sel->selectee, NULL);
+ }
+ break;
+ }
case AP:
{
StgAP *ap = (StgAP *) thunk;
@@ -632,9 +645,11 @@ void updateRemembSetPushThunkEager(Capability *cap,
trace_PAP_payload(queue, ap->fun, ap->payload, ap->n_args);
break;
}
- case THUNK_SELECTOR:
+ // We may end up here if a thunk update races with another update.
+ // In this case there is nothing to do as the other thread will have
+ // already pushed the updated thunk's free variables to the update
+ // remembered set.
case BLACKHOLE:
- // TODO: This is right, right?
break;
// The selector optimization performed by the nonmoving mark may have
// overwritten a thunk which we are updating with an indirection.
@@ -1466,10 +1481,12 @@ mark_closure (MarkQueue *queue, const StgClosure *p0, StgClosure **origin)
break;
}
+ case WEAK:
+ ASSERT(is_nonmoving_weak((StgWeak*) p));
+ // fallthrough
gen_obj:
case CONSTR:
case CONSTR_NOCAF:
- case WEAK:
case PRIM:
{
for (StgWord i = 0; i < info->layout.payload.ptrs; i++) {
@@ -1522,8 +1539,15 @@ mark_closure (MarkQueue *queue, const StgClosure *p0, StgClosure **origin)
}
case THUNK_SELECTOR:
- nonmoving_eval_thunk_selector(queue, (StgSelector*)p, origin);
+ {
+ StgSelector *sel = (StgSelector *) p;
+ // We may be able to evaluate this selector which may render the
+ // selectee unreachable. However, we must mark the selectee regardless
+ // to satisfy the snapshot invariant.
+ PUSH_FIELD(sel, selectee);
+ nonmoving_eval_thunk_selector(queue, sel, origin);
break;
+ }
case AP_STACK: {
StgAP_STACK *ap = (StgAP_STACK *)p;
@@ -1820,9 +1844,45 @@ static bool nonmovingIsNowAlive (StgClosure *p)
|| (bd->flags & BF_MARKED) != 0;
// The object was marked
} else {
- return nonmovingClosureMarkedThisCycle((P_)p);
+ struct NonmovingSegment *seg = nonmovingGetSegment((StgPtr) p);
+ StgClosure *snapshot_loc =
+ (StgClosure *) nonmovingSegmentGetBlock(seg, nonmovingSegmentInfo(seg)->next_free_snap);
+ if (p >= snapshot_loc && nonmovingGetClosureMark((StgPtr) p) == 0) {
+ /*
+ * In this case we are looking at a block that wasn't allocated
+ * at the time that the snapshot was taken. As we do not mark such
+ * blocks, we must assume that it is reachable.
+ */
+ return true;
+ } else {
+ return nonmovingClosureMarkedThisCycle((P_)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);
+ }
+}
+
+#if defined(DEBUG)
+// Determine whether a weak pointer object is on one of the nonmoving
+// collector's weak pointer lists. Used for sanity checking.
+static bool is_nonmoving_weak(StgWeak *weak)
+{
+ for (StgWeak *w = nonmoving_old_weak_ptr_list; w != NULL; w = w->link) {
+ if (w == weak) return true;
}
+ for (StgWeak *w = nonmoving_weak_ptr_list; w != NULL; w = w->link) {
+ if (w == weak) return true;
+ }
+ return false;
}
+#endif
// Non-moving heap variant of `tidyWeakList`
bool nonmovingTidyWeaks (struct MarkQueue_ *queue)
@@ -1832,6 +1892,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;
@@ -1842,7 +1905,10 @@ bool nonmovingTidyWeaks (struct MarkQueue_ *queue)
// Otherwise it's a live weak
ASSERT(w->header.info == &stg_WEAK_info);
- if (nonmovingIsNowAlive(w->key)) {
+ // See Note [Weak pointer processing and the non-moving GC] in
+ // MarkWeak.c
+ bool key_in_nonmoving = Bdescr((StgPtr) w->key)->flags & BF_NONMOVING;
+ if (!key_in_nonmoving || nonmovingIsNowAlive(w->key)) {
nonmovingMarkLiveWeak(queue, w);
did_work = true;
@@ -1850,7 +1916,7 @@ bool nonmovingTidyWeaks (struct MarkQueue_ *queue)
*last_w = w->link;
next_w = w->link;
- // and put it on the weak ptr list
+ // and put it on nonmoving_weak_ptr_list
w->link = nonmoving_weak_ptr_list;
nonmoving_weak_ptr_list = w;
} else {
@@ -1872,7 +1938,8 @@ void nonmovingMarkDeadWeak (struct MarkQueue_ *queue, StgWeak *w)
void nonmovingMarkLiveWeak (struct MarkQueue_ *queue, StgWeak *w)
{
- ASSERT(nonmovingClosureMarkedThisCycle((P_)w));
+ ASSERT(nonmovingIsNowAlive((StgClosure *) w));
+ ASSERT(nonmovingIsNowAlive((StgClosure *) w->key));
markQueuePushClosure_(queue, w->value);
markQueuePushClosure_(queue, w->finalizer);
markQueuePushClosure_(queue, w->cfinalizers);
@@ -1886,9 +1953,9 @@ void nonmovingMarkDeadWeaks (struct MarkQueue_ *queue, StgWeak **dead_weaks)
{
StgWeak *next_w;
for (StgWeak *w = nonmoving_old_weak_ptr_list; w; w = next_w) {
- ASSERT(!nonmovingClosureMarkedThisCycle((P_)(w->key)));
+ ASSERT(!nonmovingIsNowAlive(w->key));
nonmovingMarkDeadWeak(queue, w);
- next_w = w ->link;
+ next_w = w->link;
w->link = *dead_weaks;
*dead_weaks = w;
}
=====================================
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/Storage.c
=====================================
@@ -323,7 +323,7 @@ void storageAddCapabilities (uint32_t from, uint32_t to)
// Initialize NonmovingAllocators and UpdRemSets
if (RtsFlags.GcFlags.useNonmoving) {
nonmovingAddCapabilities(to);
- for (i = 0; i < to; ++i) {
+ for (i = from; i < to; ++i) {
init_upd_rem_set(&capabilities[i]->upd_rem_set);
}
}
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/c6f17f1218301dc7aed2a3d29f071353ee16fb89...7bf7a61cce31be34953c0a4c023caca74179bf18
--
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/c6f17f1218301dc7aed2a3d29f071353ee16fb89...7bf7a61cce31be34953c0a4c023caca74179bf18
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/20221206/ad50c76c/attachment-0001.html>
More information about the ghc-commits
mailing list