Skip to content

Commit

Permalink
Remove broken assertions in debug runtime build
Browse files Browse the repository at this point in the history
These assertions were defensive sanity checks from years ago when
I was first working on the new block-based chunk system. The idea
was to confirm the invariant that the frontier should always lie
within the first block of the current chunk.

However, these particular assertions are broken. Although unlikely,
it's possible for frontier == limitPlusSlop, in which case the
inSameBlock check will find two different blocks and therefore
spuriously fail.

We could fix these assertions... one idea would be to do something
along the lines of
  assert(inSameBlock(frontier, limitPlusSlop-1) || frontier == limitPlusSlop)
but then the assertion immediately following (sanity-checking the
chunk magic number) would still be broken...

So, for now, I'm just going to remove these assertions. I feel like
these checks are ad-hoc/messy and should have been removed years ago
in favor of a more careful treatment. The runtime system is in need of
a refactor/cleanup, and perhaps this can be rolled into future code
cleanup efforts.
  • Loading branch information
shwestrick committed Aug 3, 2024
1 parent 987d0c3 commit b8be87d
Showing 1 changed file with 0 additions and 15 deletions.
15 changes: 0 additions & 15 deletions runtime/gc/thread.c
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,6 @@ void GC_HH_setDepth(pointer threadp, Word32 depth) {
thread->currentDepth = depth;
// printf("%s %d\n", "setting thread depth to ", depth);
// printf("%s %d\n", "HH depth = ", thread->hierarchicalHeap->depth);

/* SAM_NOTE: not super relevant here, but if we do eventually decide to
* control the "use ancestor chunk" optimization, a good sanity check. */
assert(inSameBlock(s->frontier, s->limitPlusSlop-1));
assert(((HM_chunk)blockOf(s->frontier))->magic == CHUNK_MAGIC);
}

void GC_HH_setMinLocalCollectionDepth(pointer threadp, Word32 depth) {
Expand Down Expand Up @@ -273,11 +268,6 @@ void GC_HH_joinIntoParentBeforeFastClone(
HM_HH_promoteChunks(s, thread);
thread->currentDepth = newDepth;

/* SAM_NOTE: not super relevant here, but if we do eventually decide to
* control the "use ancestor chunk" optimization, a good sanity check. */
assert(inSameBlock(s->frontier, s->limitPlusSlop-1));
assert(((HM_chunk)blockOf(s->frontier))->magic == CHUNK_MAGIC);

GC_HH_decheckJoin(s, tidLeft, tidRight);

leave(s);
Expand Down Expand Up @@ -352,11 +342,6 @@ void GC_HH_joinIntoParent(
HM_HH_promoteChunks(s, thread);
thread->currentDepth = newDepth;

/* SAM_NOTE: not super relevant here, but if we do eventually decide to
* control the "use ancestor chunk" optimization, a good sanity check. */
assert(inSameBlock(s->frontier, s->limitPlusSlop-1));
assert(((HM_chunk)blockOf(s->frontier))->magic == CHUNK_MAGIC);

GC_HH_decheckJoin(s, tidLeft, tidRight);


Expand Down

0 comments on commit b8be87d

Please sign in to comment.