Skip to content

Commit

Permalink
fix long-standing GC bug??
Browse files Browse the repository at this point in the history
This patch appears to fix #156. The solution is different than what was
proposed in the discussion there, and also different from what was
proposed in #178. The idea here is to restrict what range of the
work-stealing deque is accessed by LGC, to ensure that the only slots
accessed are those that are in-scope of the LGC. To implement this,
I created a new `foreachObjptrInSequenceSlice` function which traces
the roots within an index range of a sequence object.
  • Loading branch information
shwestrick committed Feb 16, 2024
1 parent 8d32f9b commit d1646cf
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 12 deletions.
7 changes: 2 additions & 5 deletions basis-library/schedulers/par-pcall/MkScheduler.sml
Original file line number Diff line number Diff line change
Expand Up @@ -562,8 +562,7 @@ struct
; dbgmsg' (fn _ => "back from GC stuff")
)
else
( clear()
; setQueueDepth (myWorkerId ()) newDepth
( setQueueDepth (myWorkerId ()) newDepth
);

(* This can be reused here... the name isn't appropriate in this
Expand Down Expand Up @@ -817,9 +816,7 @@ struct
end
)
else
( clear () (* this should be safe after popDiscard fails? *)

; if decrementHitsZero incounter then
( if decrementHitsZero incounter then
()
else
( ()
Expand Down
62 changes: 62 additions & 0 deletions runtime/gc/foreach.c
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,68 @@ pointer foreachObjptrInObject (GC_state s, pointer p,
return p;
}


/* ========================================================================= */

void foreachObjptrInSequenceSlice(
GC_state s,
pointer p,
GC_foreachObjptrClosure f,
uint64_t startIdx,
uint64_t stopIdx)
{
GC_header header = getRacyHeader(p);
GC_objectTypeTag tag;
uint16_t numObjptrs;
uint16_t bytesNonObjptrs;
splitHeader(s, header, &tag, NULL, &bytesNonObjptrs, &numObjptrs);

if (SEQUENCE_TAG != tag) {
DIE("argument object is not a sequence");
}

GC_sequenceLength numElements = getSequenceLength(p);

if (startIdx > stopIdx || stopIdx > numElements) {
DIE("invalid index range");
}

size_t bytesPerElement = bytesNonObjptrs + (numObjptrs * OBJPTR_SIZE);
size_t dataBytes = numElements * bytesPerElement;
pointer last;
if (0 == numObjptrs) {
/* No objptrs to process. */
return;
}

pointer start = p + (bytesPerElement * startIdx);
pointer stop = p + (bytesPerElement * stopIdx);

if (0 == bytesNonObjptrs) {
/* Sequence with only pointers. */
for (pointer cursor = start; cursor < stop; cursor += OBJPTR_SIZE) {
callIfIsObjptr(s, f, (objptr*)cursor);
}
} else {
/* Sequence with a mix of pointers and non-pointers. */
size_t bytesObjptrs = numObjptrs * OBJPTR_SIZE;

for (pointer cursor = start; cursor < stop; ) {
/* Skip the non-pointers. */
cursor += bytesNonObjptrs;
pointer next = cursor + bytesObjptrs;
/* For each internal pointer. */
for ( ; cursor < next; cursor += OBJPTR_SIZE) {
callIfIsObjptr(s, f, (objptr*)cursor);
}
}
}

}

/* ========================================================================= */


/* foreachObjptrInRange (s, front, back, f, skipWeaks)
*
* Apply f to each pointer between front and *back, which should be a
Expand Down
14 changes: 14 additions & 0 deletions runtime/gc/foreach.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,20 @@ static inline pointer foreachObjptrInObject (GC_state s, pointer p,
GC_objptrPredicateClosure g,
GC_foreachObjptrClosure f,
bool skipWeaks);

/* Similar to foreachObjptrInObject, except it only traces a slice of
* a sequence object, in the index range {i: startIdx <= i < stopIdx}.
* It will crash if called on a non-sequence object or if the indices are
* out of bounds.
*/
static inline void foreachObjptrInSequenceSlice(
GC_state s,
pointer p,
GC_foreachObjptrClosure f,
uint64_t startIdx,
uint64_t stopIdx
);

/* foreachObjptrInRange (s, front, back, f, skipWeaks)
*
* Apply f to each pointer between front and *back, which should be a
Expand Down
16 changes: 9 additions & 7 deletions runtime/gc/hierarchical-heap-collection.c
Original file line number Diff line number Diff line change
Expand Up @@ -643,14 +643,16 @@ void HM_HHC_collectLocal(uint32_t desiredScope)

/* ======================================================================= */

/* forward contents of deque */
/* forward contents of deque, only for the range of the deque that is
* in-scope of this collection. */
oldObjectCopied = forwardHHObjptrArgs.objectsCopied;
foreachObjptrInObject(s,
objptrToPointer(s->wsQueue,
NULL),
&trueObjptrPredicateClosure,
&forwardHHObjptrClosure,
FALSE);
foreachObjptrInSequenceSlice(
s,
objptrToPointer(s->wsQueue, NULL),
&forwardHHObjptrClosure,
minDepth,
originalLocalScope
);
LOG(LM_HH_COLLECTION, LL_DEBUG,
"Copied %" PRIu64 " objects from deque",
forwardHHObjptrArgs.objectsCopied - oldObjectCopied);
Expand Down

0 comments on commit d1646cf

Please sign in to comment.