Skip to content

Fix handling of dirty objects in non-moving GC

Ben Gamari requested to merge wip/gc/T18016 into master

This fixes the underlying cause of #18016 (closed).

Previously we (incorrectly) relied on failed_to_evac to be "precise". That is, we expected it to only be true if all of an object's fields lived outside of the non-moving heap. However, does not match the behavior of failed_to_evac, which is true if any of the object's fields weren't promoted (meaning that some others may live in the non-moving heap).

This is problematic as we skip the non-moving write barrier for dirty objects (which we can only safely do if all fields point outside of the non-moving heap).

Clearly this arises due to a fundamental difference in the behavior expected of failed_to_evac in the moving and non-moving collector. e.g., in the moving collector it is always safe to conservatively say failed_to_evac=true whereas in the non-moving collector the safe value is false.

This issue went unnoticed as I never wrote down the dirtiness invariant enforced by the non-moving collector. We now define this invariant as

An object being marked as dirty implies that all of its fields are on the mark queue (or, equivalently, update remembered set).

To maintain this invariant we teach nonmovingScavengeOne to push the fields of objects which we fail to evacuate to the update remembered set. This is a simple and reasonably cheap solution and avoids the complexity and fragility that other, more strict alternative invariants would require.

All of this is described in a new Note, Note [Dirty flags in the non-moving collector] in NonMoving.c.

This also includes two other somewhat related commits:

  • a patch ensuring that bdescr's flag field is cleared when sanity-checking is enabled. This makes it significantly easier to track the lifecycle of a block.

  • a patch to make failed_to_evac a bit more precise

Fixes #18115 (closed).

Edited by Ben Gamari

Merge request reports