Ben Gamari pushed to branch wip/T26053 at Glasgow Haskell Compiler / GHC Commits: 173e0312 by Ben Gamari at 2025-07-15T12:29:55-04:00 rts/nonmoving: Fix comment spelling - - - - - 34535bd7 by Ben Gamari at 2025-07-15T12:29:55-04:00 rts/nonmoving: Use atomic operations to update bd->flags - - - - - 1c70dab6 by Ben Gamari at 2025-07-15T12:29:55-04:00 Debug output - - - - - 3 changed files: - rts/include/stg/SMP.h - rts/sm/NonMoving.c - rts/sm/NonMovingMark.c Changes: ===================================== rts/include/stg/SMP.h ===================================== @@ -516,6 +516,14 @@ atomic_dec(StgVolatilePtr p) #endif } +EXTERN_INLINE StgWord +atomic_or(StgVolatilePtr p, StgWord v) +{ + // TODO: This ordering is stronger than necessary for many users (e.g. + // setting flags). + return __atomic_or_fetch(p, v, __ATOMIC_SEQ_CST); +} + /* * Some architectures have a way to tell the CPU that we're in a * busy-wait loop, and the processor should look for something else to ===================================== rts/sm/NonMoving.c ===================================== @@ -560,7 +560,7 @@ Mutex concurrent_coll_finished_lock; * arbitrary allocator sizes, we need to do some precomputation and make * use of the integer division by constants optimisation. * - * We currenlty try to balance these considerations by adopting the following scheme. + * We currently try to balance these considerations by adopting the following scheme. * We have nonmoving_alloca_dense_cnt "dense" allocators starting with size * NONMOVING_ALLOCA0, and incrementing by NONMOVING_ALLOCA_DENSE_INCREMENT. * These service the vast majority of allocations. ===================================== rts/sm/NonMovingMark.c ===================================== @@ -722,13 +722,14 @@ STATIC_INLINE bool needs_upd_rem_set_mark(StgClosure *p) { // TODO: Deduplicate with mark_closure bdescr *bd = Bdescr((StgPtr) p); + uint16_t flags = RELAXED_LOAD(&bd->flags); if (bd->gen != oldest_gen) { return false; - } else if (bd->flags & BF_LARGE) { - if (! (bd->flags & BF_NONMOVING_SWEEPING)) { + } else if (flags & BF_LARGE) { + if (! (flags & BF_NONMOVING_SWEEPING)) { return false; } else { - return ! (bd->flags & BF_MARKED); + return ! (flags & BF_MARKED); } } else { struct NonmovingSegment *seg = nonmovingGetSegment((StgPtr) p); @@ -740,8 +741,8 @@ STATIC_INLINE bool needs_upd_rem_set_mark(StgClosure *p) static void finish_upd_rem_set_mark_large(bdescr* bd) { // Someone else may have already marked it. ACQUIRE_LOCK(&nonmoving_large_objects_mutex); - if (! (bd->flags & BF_MARKED)) { - bd->flags |= BF_MARKED; + if (! (RELAXED_LOAD(&bd->flags) & BF_MARKED)) { + atomic_or(&bd->flags, BF_MARKED); dbl_link_remove(bd, &nonmoving_large_objects); dbl_link_onto(bd, &nonmoving_marked_large_objects); n_nonmoving_large_blocks -= bd->blocks; @@ -785,18 +786,28 @@ void updateRemembSetPushStack(Capability *cap, StgStack *stack) debugTrace(DEBUG_nonmoving_gc, "upd_rem_set: STACK %p", stack->sp); trace_stack(&cap->upd_rem_set.queue, stack); finish_upd_rem_set_mark((StgClosure *) stack); - return; } else { // The concurrent GC has claimed the right to mark the stack. // Wait until it finishes marking before proceeding with // mutation. - while (needs_upd_rem_set_mark((StgClosure *) stack)) + uint64_t iters = 0; + while (needs_upd_rem_set_mark((StgClosure *) stack)) { + iters++; + if (iters > 100000000) { + bdescr *bd = Bdescr(stack); + debugBelch("updateRemSetPushStack: stuck: %p\n", stack); + debugBelch(" bd->flags: %x\n", bd->flags); + debugBelch(" epoch : %x\n", nonmovingMarkEpoch); + debugBelch(" marking: %x\n", stack->marking); + abort(); + } + #if defined(PARALLEL_GC) busy_wait_nop(); // TODO: Spinning here is unfortunate #else ; #endif - return; + } } } } @@ -1371,35 +1382,36 @@ mark_closure (MarkQueue *queue, const StgClosure *p0, StgClosure **origin) // N.B. only the first block of a compact region is guaranteed to carry // BF_NONMOVING; consequently we must separately check for BF_COMPACT. - if (bd->flags & (BF_COMPACT | BF_NONMOVING)) { + const uint16_t flags = RELAXED_LOAD(&bd->flags); + if (flags & (BF_COMPACT | BF_NONMOVING)) { - if (bd->flags & BF_COMPACT) { + if (flags & BF_COMPACT) { StgCompactNFData *str = objectGetCompact((StgClosure*)p); bd = Bdescr((P_)str); - if (! (bd->flags & BF_NONMOVING_SWEEPING)) { + if (! (flags & BF_NONMOVING_SWEEPING)) { // Not in the snapshot return; } - if (! (bd->flags & BF_MARKED)) { + if (! (flags & BF_MARKED)) { dbl_link_remove(bd, &nonmoving_compact_objects); dbl_link_onto(bd, &nonmoving_marked_compact_objects); StgWord blocks = str->totalW / BLOCK_SIZE_W; n_nonmoving_compact_blocks -= blocks; n_nonmoving_marked_compact_blocks += blocks; - bd->flags |= BF_MARKED; + atomic_or(&bd->flags, BF_MARKED); } // N.B. the object being marked is in a compact region so by // definition there is no need to do any tracing here. goto done; - } else if (bd->flags & BF_LARGE) { - if (! (bd->flags & BF_NONMOVING_SWEEPING)) { + } else if (flags & BF_LARGE) { + if (! (flags & BF_NONMOVING_SWEEPING)) { // Not in the snapshot goto done; } - if (bd->flags & BF_MARKED) { + if (flags & BF_MARKED) { goto done; } } else { @@ -1731,24 +1743,25 @@ mark_closure (MarkQueue *queue, const StgClosure *p0, StgClosure **origin) * the object's pointers since in the case of marking stacks there may be a * mutator waiting for us to finish so it can start execution. */ - if (bd->flags & BF_LARGE) { + uint16_t bd_flags = RELAXED_LOAD(&bd->flags); + if (bd_flags & BF_LARGE) { /* Marking a large object isn't idempotent since we move it to * nonmoving_marked_large_objects; to ensure that we don't repeatedly * mark a large object, we only set BF_MARKED on large objects in the * nonmoving heap while holding nonmoving_large_objects_mutex */ ACQUIRE_LOCK(&nonmoving_large_objects_mutex); - if (! (bd->flags & BF_MARKED)) { + if (! (bd_flags & BF_MARKED)) { // Remove the object from nonmoving_large_objects and link it to // nonmoving_marked_large_objects dbl_link_remove(bd, &nonmoving_large_objects); dbl_link_onto(bd, &nonmoving_marked_large_objects); n_nonmoving_large_blocks -= bd->blocks; n_nonmoving_marked_large_blocks += bd->blocks; - bd->flags |= BF_MARKED; + RELAXED_STORE(&bd->flags, flags | BF_MARKED); } RELEASE_LOCK(&nonmoving_large_objects_mutex); - } else if (bd->flags & BF_NONMOVING) { + } else if (bd_flags & BF_NONMOVING) { // TODO: Kill repetition struct NonmovingSegment *seg = nonmovingGetSegment((StgPtr) p); nonmoving_block_idx block_idx = nonmovingGetBlockIdx((StgPtr) p); @@ -1927,7 +1940,7 @@ static bool nonmovingIsNowAlive (StgClosure *p) bdescr *bd = Bdescr((P_)p); - const uint16_t flags = bd->flags; + const uint16_t flags = RELAXED_LOAD(&bd->flags); if (flags & BF_LARGE) { if (flags & BF_PINNED && !(flags & BF_NONMOVING)) { // In this case we have a pinned object living in a non-full @@ -1937,15 +1950,15 @@ static bool nonmovingIsNowAlive (StgClosure *p) return true; } - ASSERT(bd->flags & BF_NONMOVING); - return (bd->flags & BF_NONMOVING_SWEEPING) == 0 + ASSERT(flags & BF_NONMOVING); + return (flags & BF_NONMOVING_SWEEPING) == 0 // the large object wasn't in the snapshot and therefore wasn't marked - || (bd->flags & BF_MARKED) != 0; + || (flags & BF_MARKED) != 0; // The object was marked } else { // All non-static objects in the non-moving heap should be marked as // BF_NONMOVING. - ASSERT(bd->flags & BF_NONMOVING); + ASSERT(flags & BF_NONMOVING); struct NonmovingSegment *seg = nonmovingGetSegment((StgPtr) p); StgClosure *snapshot_loc = View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/a8a49b17cc76aad661237a2afdaa4d4... -- View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/a8a49b17cc76aad661237a2afdaa4d4... You're receiving this email because of your account on gitlab.haskell.org.