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
  • covered — perf(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

  1. proggeramlug commented on Sep 6, 2026

    @proggeramlug
    ContributorAuthor

    Not reproduced on the landing base — the performance framing in the body is withdrawn

    The body cites this pass at self 1,455 (21.3 %) / inclusive 1,594 (23.4 %)
    of a 3300-character claude-code turn. That was measured on arm H
    (644b9d362 + #9838 + #9857). On I2 — main 35c36f425 + #9838 + #9860 +
    #9857, i.e. the base this actually lands on — the same symbol is self 84
    (1.6 %) / inclusive 247 (4.8 %), 241 of those under copying minors
    .

    So the ~20× swing was an interaction with H's older base, not a property of
    main + #9857. On the landing base the pass sits at roughly the proportion it
    always had. There is no hot-path emergency here and no fix is being
    designed.
    Which train commit removed it is archaeology — plausibly #9835's
    pre-sized covered set — and nobody should chase it on the strength of this
    issue.

    What this does not change

    The defect is in the diagnostic, not in the pass's cost, and it stands
    whatever the pass costs:

    • the line still prints snapshot.dirty_pages.len() while the walk iterates
      snapshot.dirty_old_pages;
    • it still counts objects where the cost is per slot;
    • it still reports nothing about how much of the walk is productive.

    A cheap pass with a misleading instrument is exactly how #9835 sized this from
    objects_skipped=1026 for a set holding ~119,000. The next person to profile
    it will reach for the same line. Fixing the fields is a small change and is
    worth doing on its own terms; the counter branch
    (proggeramlug/perry measure/restore-coverage-counter @ e2c4e969f)
    remains available as the reference for what the fields should be, and is not
    proposed for merge as-is.

    One number in the body I cannot reconcile, flagged rather than quietly swapped

    The body says the pre-#9857 arm A is 71–78 inclusive (1.3–1.8 %) in all
    three draws
    . The figure I have now for A is 121 self / 355 inclusive.
    Those are not the same measurement and I do not know which of the two is the
    one I should be quoting — they may be different capture sets, or a self-vs-
    inclusive mix-up upstream of me. Either way the conclusion above is unaffected,
    because it rests on the I2 numbers, not on A. But the A row in the body should
    be treated as unverified until someone re-derives it, and I would rather say
    so than silently replace one number with the other.

    https://claude.ai/code/session_014knX724SYDogwzsXybCGxp

  2. proggeramlug commented on Sep 14, 2026

    @proggeramlug
    ContributorAuthor

    Fixed by merge train 186r (#10247) at eb13fa1.

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