Skip to content

[gc-restore-coverage] reports dirty_pages while the walk iterates dirty_old_pages, and counts objects where the cost is per slot #9877

Description

@proggeramlug

The line reports a different set from the one the walk iterates

restore_surviving_dirty_coverage (crates/perry-runtime/src/gc/verify.rs,
the #5029 post-cycle remembered-set repair) ends with:

eprintln!(
    "[gc-restore-coverage] {cycle_label} dirty_pages={} objects_walked={walked} \
     objects_skipped={skipped} pages_added={added}",
    snapshot.dirty_pages.len()
);

but the walk it is describing iterates a different field of the same
snapshot
:

if !snapshot.dirty_old_pages.is_empty() {
    crate::arena::old_arena_walk_objects_on_pages(&snapshot.dirty_old_pages, |hp| {});
}

RememberedDirtySnapshot (gc/barrier/mod.rs) carries dirty_old_pages and
dirty_pages as separate sets. The pass walks the first and prints the
second. Nothing else on the line names the set that was actually walked.

And the unit is wrong for the cost

Every field on that line counts objects (objects_walked,
objects_skipped) or pages (pages_added). The pass's cost is per
slot: for each walked parent it runs visit_gc_rewrite_slots, which
enumerates the object's layout, calls slot.record_layout_read() on every
slot and then remember_evacuated_old_to_young_slot. Two objects with the
same object count and wildly different layouts cost wildly different amounts,
and the line cannot distinguish them. It also never reports how much of the
walk was productive — how many enumerated slots held a real old→young (or
old→malloc) edge at all.

This has already produced a wrong number, in this function

While sizing the per-minor dirty-scan set for #9835, the figure "~1,000
entries" was taken from this very line's objects_skipped=1026. The set
actually holds ~119,000. A fix sized from that would have reserved 1,024,
read as a correct pre-size in review and in the changelog, and left hashbrown
climbing exactly the ladder it climbs today. That is the failure mode this
report is about: the misleading number is adjacent, produced by related
code, and of a believable order of magnitude, so it does not look wrong.

Why it matters now

On a post-#9857 base this pass is self 1,455 samples (21.3 %) and inclusive
1,594 (23.4 %)
of a 3300-character claude-code turn, with 1,579 of 1,594
under copying minors and none under fulls
. On a pre-#9857 base the same
symbol is 71–78 inclusive (1.3–1.8 %) across three draws — roughly 20× more
absolute samples with 8 % fewer minors
, i.e. an interaction rather than the
turn simply getting shorter around it. It is the largest single item on that
profile, and the only telemetry it has is the line above.

Replacement

measure/restore-coverage-counter on proggeramlug/perry (commit
e2c4e969f, runtime-only, measurement-only — not proposed for merge as-is)
adds, beside the existing line:

[gc-restore-counter] <cycle> dirty_old_pages=P external_entries=E covered=C
  objects_walked=W objects_skipped=S parents_visited=PV slots_visited=V
  slots_tracking=T pages_added=A sticky_old=SO sticky_external=SE
  • dirty_old_pages — the set old_arena_walk_objects_on_pages actually iterates
  • external_entries — the second, separately walked input
  • coveredperf(gc): pre-size the per-minor dirty-scan covered set instead of rebuilding it from empty #9835's dirty_scan_covered, the skip set
  • parents_visited — objects that passed the plausibility and old-generation
    guards and were descended into
  • slots_visited — slots visit_gc_rewrite_slots enumerated: the cost driver
  • slots_tracking — of those, how many held a real old→young/old→malloc edge

slots_visited vs slots_tracking answers "is this walk productive?";
slots_visited vs dirty_old_pages / parents_visited answers "is the cost
proportional to the whole dirty set or to what survived?". Neither question
can be asked of the current line.

remember_evacuated_old_to_young_slot gains a bool return so
slots_tracking can be counted without repeating its predicate; its three
other call sites discard it.

Suggested resolution

Either correct the existing line to print dirty_old_pages and add
slots_visited / slots_tracking, or land the counter's fields into it.
Printing a set the code does not walk is worse than printing nothing, because
it invites exactly the sizing mistake #9835 made.

https://claude.ai/code/session_014knX724SYDogwzsXybCGxp

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions