[Git][ghc/ghc][master] 9 commits: ThreadPaused: Don't zero slop until free vars are pushed
Marge Bot
gitlab at gitlab.haskell.org
Sun Nov 29 20:34:02 UTC 2020
Marge Bot pushed to branch master at Glasgow Haskell Compiler / GHC
Commits:
e992ea84 by GHC GitLab CI at 2020-11-29T15:33:54-05:00
ThreadPaused: Don't zero slop until free vars are pushed
When threadPaused blackholes a thunk it calls `OVERWRITING_CLOSURE` to
zero the slop for the benefit of the sanity checker. Previously this was
done *before* pushing the thunk's free variables to the update
remembered set. Consequently we would pull zero'd pointers to the update
remembered set.
- - - - -
e82cd140 by GHC GitLab CI at 2020-11-29T15:33:54-05:00
nonmoving: Fix regression from TSAN work
The TSAN rework (specifically aad1f803) introduced a subtle regression
in GC.c, swapping `g0` in place of `gen`. Whoops!
Fixes #18997.
- - - - -
35a5207e by GHC GitLab CI at 2020-11-29T15:33:54-05:00
rts/Messages: Add missing write barrier in THROWTO message update
After a THROWTO message has been handle the message closure is
overwritten by a NULL message. We must ensure that the original
closure's pointers continue to be visible to the nonmoving GC.
- - - - -
0120829f by GHC GitLab CI at 2020-11-29T15:33:54-05:00
nonmoving: Add missing write barrier in shrinkSmallByteArray
- - - - -
8a4d8fb6 by GHC GitLab CI at 2020-11-29T15:33:54-05:00
Updates: Don't zero slop until closure has been pushed
Ensure that the the free variables have been pushed to the update
remembered set before we zero the slop.
- - - - -
2793cfdc by GHC GitLab CI at 2020-11-29T15:33:54-05:00
OSThreads: Fix error code checking
pthread_join returns its error code and apparently doesn't set errno.
- - - - -
e391a16f by GHC GitLab CI at 2020-11-29T15:33:54-05:00
nonmoving: Don't join to mark_thread on shutdown
The mark thread is not joinable as we detach from it on creation.
- - - - -
60d088ab by Ben Gamari at 2020-11-29T15:33:54-05:00
nonmoving: Add reference to Ueno 2016
- - - - -
3aa60362 by GHC GitLab CI at 2020-11-29T15:33:54-05:00
nonmoving: Ensure that evacuated large objects are marked
See Note [Non-moving GC: Marking evacuated objects].
- - - - -
11 changed files:
- includes/rts/storage/ClosureMacros.h
- rts/Messages.c
- rts/Messages.h
- rts/PrimOps.cmm
- rts/RaiseAsync.c
- rts/ThreadPaused.c
- rts/Updates.h
- rts/posix/OSThreads.c
- rts/sm/Evac.c
- rts/sm/GC.c
- rts/sm/NonMoving.c
Changes:
=====================================
includes/rts/storage/ClosureMacros.h
=====================================
@@ -520,11 +520,15 @@ INLINE_HEADER StgWord8 *mutArrPtrsCard (StgMutArrPtrs *a, W_ n)
#if defined(PROFILING) || defined(DEBUG)
#define OVERWRITING_CLOSURE(c) \
overwritingClosure(c)
+#define OVERWRITING_CLOSURE_SIZE(c, size) \
+ overwritingClosureSize(c, size)
#define OVERWRITING_CLOSURE_MUTABLE(c, off) \
overwritingMutableClosureOfs(c, off)
#else
#define OVERWRITING_CLOSURE(c) \
do { (void) sizeof(c); } while(0)
+#define OVERWRITING_CLOSURE_SIZE(c, size) \
+ do { (void) sizeof(c); (void) sizeof(size); } while(0)
#define OVERWRITING_CLOSURE_MUTABLE(c, off) \
do { (void) sizeof(c); (void) sizeof(off); } while(0)
#endif
=====================================
rts/Messages.c
=====================================
@@ -97,7 +97,7 @@ loop:
case THROWTO_SUCCESS: {
// this message is done
StgTSO *source = t->source;
- doneWithMsgThrowTo(t);
+ doneWithMsgThrowTo(cap, t);
tryWakeupThread(cap, source);
break;
}
=====================================
rts/Messages.h
=====================================
@@ -23,8 +23,16 @@ void sendMessage (Capability *from_cap, Capability *to_cap, Message *msg);
#include "SMPClosureOps.h"
INLINE_HEADER void
-doneWithMsgThrowTo (MessageThrowTo *m)
+doneWithMsgThrowTo (Capability *cap, MessageThrowTo *m)
{
+ // The message better be locked
+ ASSERT(m->header.info == &stg_WHITEHOLE_info);
+ IF_NONMOVING_WRITE_BARRIER_ENABLED {
+ updateRemembSetPushClosure(cap, (StgClosure *) m->link);
+ updateRemembSetPushClosure(cap, (StgClosure *) m->source);
+ updateRemembSetPushClosure(cap, (StgClosure *) m->target);
+ updateRemembSetPushClosure(cap, (StgClosure *) m->exception);
+ }
OVERWRITING_CLOSURE((StgClosure*)m);
unlockClosure((StgClosure*)m, &stg_MSG_NULL_info);
LDV_RECORD_CREATE(m);
=====================================
rts/PrimOps.cmm
=====================================
@@ -227,6 +227,21 @@ stg_shrinkSmallMutableArrayzh ( gcptr mba, W_ new_size )
{
ASSERT(new_size <= StgSmallMutArrPtrs_ptrs(mba));
+ IF_NONMOVING_WRITE_BARRIER_ENABLED {
+ // Ensure that the elements we are about to shrink out of existence
+ // remain visible to the non-moving collector.
+ W_ p, end;
+ p = mba + SIZEOF_StgSmallMutArrPtrs + WDS(new_size);
+ end = mba + SIZEOF_StgSmallMutArrPtrs + WDS(StgSmallMutArrPtrs_ptrs(mba));
+again:
+ ccall updateRemembSetPushClosure_(BaseReg "ptr",
+ W_[p] "ptr");
+ if (p < end) {
+ p = p + SIZEOF_W;
+ goto again;
+ }
+ }
+
OVERWRITING_CLOSURE_MUTABLE(mba, (BYTES_TO_WDS(SIZEOF_StgSmallMutArrPtrs) +
new_size));
StgSmallMutArrPtrs_ptrs(mba) = new_size;
=====================================
rts/RaiseAsync.c
=====================================
@@ -336,7 +336,7 @@ check_target:
}
// nobody else can wake up this TSO after we claim the message
- doneWithMsgThrowTo(m);
+ doneWithMsgThrowTo(cap, m);
raiseAsync(cap, target, msg->exception, false, NULL);
return THROWTO_SUCCESS;
@@ -580,7 +580,7 @@ maybePerformBlockedException (Capability *cap, StgTSO *tso)
throwToSingleThreaded(cap, msg->target, msg->exception);
source = msg->source;
- doneWithMsgThrowTo(msg);
+ doneWithMsgThrowTo(cap, msg);
tryWakeupThread(cap, source);
return 1;
}
@@ -602,7 +602,7 @@ awakenBlockedExceptionQueue (Capability *cap, StgTSO *tso)
i = lockClosure((StgClosure *)msg);
if (i != &stg_MSG_NULL_info) {
source = msg->source;
- doneWithMsgThrowTo(msg);
+ doneWithMsgThrowTo(cap, msg);
tryWakeupThread(cap, source);
} else {
unlockClosure((StgClosure *)msg,i);
@@ -700,7 +700,7 @@ removeFromQueues(Capability *cap, StgTSO *tso)
// ASSERT(m->header.info == &stg_WHITEHOLE_info);
// unlock and revoke it at the same time
- doneWithMsgThrowTo(m);
+ doneWithMsgThrowTo(cap, m);
break;
}
=====================================
rts/ThreadPaused.c
=====================================
@@ -314,10 +314,6 @@ threadPaused(Capability *cap, StgTSO *tso)
continue;
}
- // zero out the slop so that the sanity checker can tell
- // where the next closure is.
- OVERWRITING_CLOSURE(bh);
-
// an EAGER_BLACKHOLE or CAF_BLACKHOLE gets turned into a
// BLACKHOLE here.
#if defined(THREADED_RTS)
@@ -345,11 +341,16 @@ threadPaused(Capability *cap, StgTSO *tso)
// overwrite to the update remembered set.
// N.B. We caught the WHITEHOLE case above.
updateRemembSetPushThunkEager(cap,
- THUNK_INFO_PTR_TO_STRUCT(bh_info),
- (StgThunk *) bh);
+ THUNK_INFO_PTR_TO_STRUCT(bh_info),
+ (StgThunk *) bh);
}
}
+ // zero out the slop so that the sanity checker can tell
+ // where the next closure is. N.B. We mustn't do this until we have
+ // pushed the free variables to the update remembered set above.
+ OVERWRITING_CLOSURE_SIZE(bh, closure_sizeW_(bh, INFO_PTR_TO_STRUCT(bh_info)));
+
// The payload of the BLACKHOLE points to the TSO
RELAXED_STORE(&((StgInd *)bh)->indirectee, (StgClosure *)tso);
SET_INFO_RELEASE(bh,&stg_BLACKHOLE_info);
=====================================
rts/Updates.h
=====================================
@@ -49,7 +49,6 @@
W_ bd; \
\
prim_write_barrier; \
- OVERWRITING_CLOSURE(p1); \
bd = Bdescr(p1); \
if (bdescr_gen_no(bd) != 0 :: bits16) { \
IF_NONMOVING_WRITE_BARRIER_ENABLED { \
@@ -60,6 +59,7 @@
} else { \
TICK_UPD_NEW_IND(); \
} \
+ OVERWRITING_CLOSURE(p1); \
StgInd_indirectee(p1) = p2; \
prim_write_barrier; \
SET_INFO(p1, stg_BLACKHOLE_info); \
=====================================
rts/posix/OSThreads.c
=====================================
@@ -401,8 +401,9 @@ interruptOSThread (OSThreadId id)
void
joinOSThread (OSThreadId id)
{
- if (pthread_join(id, NULL) != 0) {
- sysErrorBelch("joinOSThread: error %d", errno);
+ int ret = pthread_join(id, NULL);
+ if (ret != 0) {
+ sysErrorBelch("joinOSThread: error %d", ret);
}
}
=====================================
rts/sm/Evac.c
=====================================
@@ -109,6 +109,8 @@ alloc_for_copy (uint32_t size, uint32_t gen_no)
//
// However, if we are in a deadlock detection GC then we disable aging
// so there is no need.
+ //
+ // See Note [Non-moving GC: Marking evacuated objects].
if (major_gc && !deadlock_detect_gc)
markQueuePushClosureGC(&gct->cap->upd_rem_set.queue, (StgClosure *) to);
return to;
@@ -134,6 +136,52 @@ alloc_for_copy (uint32_t size, uint32_t gen_no)
The evacuate() code
-------------------------------------------------------------------------- */
+/*
+ * Note [Non-moving GC: Marking evacuated objects]
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * When the non-moving collector is in use we must be careful to ensure that any
+ * references to objects in the non-moving generation from younger generations
+ * are pushed to the mark queue.
+ *
+ * In particular we need to ensure that we handle newly-promoted objects are
+ * correctly marked. For instance, consider this case:
+ *
+ * generation 0 generation 1
+ * ────────────── ──────────────
+ *
+ * ┌───────┐
+ * ┌───────┐ │ A │
+ * │ B │ ◁────────────────────────── │ │
+ * │ │ ──┬─────────────────┐ └───────┘
+ * └───────┘ ┆ after GC │
+ * ┆ │
+ * ┌───────┐ ┆ before GC │ ┌───────┐
+ * │ C │ ◁┄┘ └─────▷ │ C' │
+ * │ │ │ │
+ * └───────┘ └───────┘
+ *
+ *
+ * In this case object C started off in generation 0 and was evacuated into
+ * generation 1 during the preparatory GC. However, the only reference to C'
+ * is from B, which lives in the generation 0 (via aging); this reference will
+ * not be visible to the concurrent non-moving collector (which can only
+ * traverse the generation 1 heap). Consequently, upon evacuating C we need to
+ * ensure that C' is added to the update remembered set as we know that it will
+ * continue to be reachable via B (which is assumed to be reachable as it lives
+ * in a younger generation).
+ *
+ * Where this happens depends upon the type of the object (e.g. C'):
+ *
+ * - In the case of "normal" small heap-allocated objects this happens in
+ * alloc_for_copy.
+ * - In the case of compact region this happens in evacuate_compact.
+ * - In the case of large objects this happens in evacuate_large.
+ *
+ * See also Note [Aging under the non-moving collector] in NonMoving.c.
+ *
+ */
+
/* size is in words
We want to *always* inline this as often the size of the closure is static,
@@ -356,6 +404,9 @@ evacuate_large(StgPtr p)
__atomic_fetch_or(&bd->flags, BF_EVACUATED, __ATOMIC_ACQ_REL);
if (RTS_UNLIKELY(RtsFlags.GcFlags.useNonmoving && new_gen == oldest_gen)) {
__atomic_fetch_or(&bd->flags, BF_NONMOVING, __ATOMIC_ACQ_REL);
+
+ // See Note [Non-moving GC: Marking evacuated objects].
+ markQueuePushClosureGC(&gct->cap->upd_rem_set.queue, (StgClosure *) p);
}
initBdescr(bd, new_gen, new_gen->to);
@@ -510,6 +561,9 @@ evacuate_compact (StgPtr p)
bd->flags |= BF_EVACUATED;
if (RTS_UNLIKELY(RtsFlags.GcFlags.useNonmoving && new_gen == oldest_gen)) {
__atomic_fetch_or(&bd->flags, BF_NONMOVING, __ATOMIC_RELAXED);
+
+ // See Note [Non-moving GC: Marking evacuated objects].
+ markQueuePushClosureGC(&gct->cap->upd_rem_set.queue, (StgClosure *) str);
}
initBdescr(bd, new_gen, new_gen->to);
@@ -695,13 +749,6 @@ loop:
*/
if (flags & BF_LARGE) {
evacuate_large((P_)q);
-
- // We may have evacuated the block to the nonmoving generation. If so
- // we need to make sure it is added to the mark queue since the only
- // reference to it may be from the moving heap.
- if (major_gc && flags & BF_NONMOVING && !deadlock_detect_gc) {
- markQueuePushClosureGC(&gct->cap->upd_rem_set.queue, q);
- }
return;
}
=====================================
rts/sm/GC.c
=====================================
@@ -1701,13 +1701,8 @@ collect_gct_blocks (void)
static void
collect_pinned_object_blocks (void)
{
- generation *gen;
const bool use_nonmoving = RtsFlags.GcFlags.useNonmoving;
- if (use_nonmoving && major_gc) {
- gen = oldest_gen;
- } else {
- gen = g0;
- }
+ generation *const gen = (use_nonmoving && major_gc) ? oldest_gen : g0;
for (uint32_t n = 0; n < n_capabilities; n++) {
bdescr *last = NULL;
@@ -1732,7 +1727,7 @@ collect_pinned_object_blocks (void)
if (gen->large_objects != NULL) {
gen->large_objects->u.back = last;
}
- g0->large_objects = RELAXED_LOAD(&capabilities[n]->pinned_object_blocks);
+ gen->large_objects = RELAXED_LOAD(&capabilities[n]->pinned_object_blocks);
RELAXED_STORE(&capabilities[n]->pinned_object_blocks, NULL);
}
}
=====================================
rts/sm/NonMoving.c
=====================================
@@ -191,8 +191,8 @@ Mutex concurrent_coll_finished_lock;
* === Other references ===
*
* Apart from the design document in docs/storage/nonmoving-gc and the Ueno
- * 2016 paper (TODO citation) from which it drew inspiration, there are a
- * variety of other relevant Notes scattered throughout the tree:
+ * 2016 paper [ueno 2016] from which it drew inspiration, there are a variety
+ * of other relevant Notes scattered throughout the tree:
*
* - Note [Concurrent non-moving collection] (NonMoving.c) describes
* concurrency control of the nonmoving collector
@@ -204,6 +204,10 @@ Mutex concurrent_coll_finished_lock;
* - Note [Aging under the non-moving collector] (NonMoving.c) describes how
* we accommodate aging
*
+ * - Note [Non-moving GC: Marking evacuated objects] (Evac.c) describes how
+ * non-moving objects reached by evacuate() are marked, which is necessary
+ * due to aging.
+ *
* - Note [Large objects in the non-moving collector] (NonMovingMark.c)
* describes how we track large objects.
*
@@ -232,6 +236,11 @@ Mutex concurrent_coll_finished_lock;
* how we use the DIRTY flags associated with MUT_VARs and TVARs to improve
* barrier efficiency.
*
+ * [ueno 2016]:
+ * Katsuhiro Ueno and Atsushi Ohori. 2016. A fully concurrent garbage
+ * collector for functional programs on multicore processors. SIGPLAN Not. 51,
+ * 9 (September 2016), 421–433. DOI:https://doi.org/10.1145/3022670.2951944
+ *
*
* Note [Concurrent non-moving collection]
* ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -312,6 +321,8 @@ Mutex concurrent_coll_finished_lock;
*
* The non-moving collector will come to C in the mark queue and mark it.
*
+ * The implementation details of this are described in Note [Non-moving GC:
+ * Marking evacuated objects] in Evac.c.
*
* Note [Deadlock detection under the non-moving collector]
* ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -726,7 +737,6 @@ void nonmovingStop(void)
"waiting for nonmoving collector thread to terminate");
ACQUIRE_LOCK(&concurrent_coll_finished_lock);
waitCondition(&concurrent_coll_finished, &concurrent_coll_finished_lock);
- joinOSThread(mark_thread);
}
#endif
}
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/1bc104b029b4f043cac42253ee2387f4d574abca...3aa603620ef5a6aae1778278aa9914f344ab526e
--
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/1bc104b029b4f043cac42253ee2387f4d574abca...3aa603620ef5a6aae1778278aa9914f344ab526e
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/20201129/db458d05/attachment-0001.html>
More information about the ghc-commits
mailing list