docs(gc): establish current collector source of truth - #7883
Merged
Conversation
📝 WalkthroughWalkthroughThe PR adds a current garbage-collector reference, removes obsolete ChangesGC documentation and configuration alignment
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant LintWorkflow
participant CheckGcEnvKnobs
participant ProductionRust
participant CurrentClaims
LintWorkflow->>CheckGcEnvKnobs: run self-test and audit
CheckGcEnvKnobs->>ProductionRust: scan live GC parsers
CheckGcEnvKnobs->>CurrentClaims: scan current knob claims
CheckGcEnvKnobs-->>LintWorkflow: return drift status and counts
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
proggeramlug
marked this pull request as ready for review
August 11, 2026 18:59
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
CLAUDE.md (1)
132-161: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep
CLAUDE.mdconcise.Move the detailed knob behavior and issue history to
docs/src/internals/garbage-collector.md. Keep a short current-state summary and a link here.As per coding guidelines, "
CLAUDE.mdconcise" and "do not write per-change changelog entries into it."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CLAUDE.md` around lines 132 - 161, The CLAUDE.md section is too detailed and includes knob behavior and issue history that belong in the collector source of truth. Replace the long GC knob documentation with a concise current-state summary covering the essential defaults and escape hatches, plus a link to docs/src/internals/garbage-collector.md for detailed behavior and history; do not retain per-change changelog entries here.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/engine-plan.md`:
- Around line 401-404: Update the statepoint-checker work item at Lines 562–563
to mark it closed by `#7663`, referencing gc_root_dominance_check.py’s
--statepoints mode and its gc.statepoint relocation-bundle checks.
In `@docs/src/internals/garbage-collector.md`:
- Around line 138-149: Update the required-context statement and table in the GC
documentation to remove compile-smoke from branch-protection-required
validation. Keep it listed only if explicitly described as non-required or
informational, and ensure the documented required checks match mandatory failing
PR gates that run for normal pull requests.
In `@scripts/check_gc_env_knobs.py`:
- Around line 39-45: Update CLAIM_ROOTS to include the crates test configuration
tree and CLAIM_SUFFIXES to include .rs so Rust integration-test settings are
scanned as executable claims. Preserve parsed_knobs() behavior so Rust test
files remain excluded from parser ownership and production-only parsing.
---
Nitpick comments:
In `@CLAUDE.md`:
- Around line 132-161: The CLAUDE.md section is too detailed and includes knob
behavior and issue history that belong in the collector source of truth. Replace
the long GC knob documentation with a concise current-state summary covering the
essential defaults and escape hatches, plus a link to
docs/src/internals/garbage-collector.md for detailed behavior and history; do
not retain per-change changelog entries here.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 02621167-65d4-4be3-b4a5-6b6d39e7e53e
📒 Files selected for processing (29)
.github/workflows/test.ymlCLAUDE.mdbenchmarks/gc_ratchet/baseline/gc-ratchet-v1.jsonbenchmarks/gc_ratchet/gc_ratchet.pychangelog.d/7883-current-gc-docs.mdcrates/perry/tests/gc_array_prototype_hole_read_6981.rscrates/perry/tests/gc_closure_self_pointer_root_7055.rsdocs/ecs-perf-case-study.mddocs/engine-plan.mddocs/generational-gc-plan.mddocs/po/de.podocs/po/es.podocs/po/fr.podocs/po/id.podocs/po/it.podocs/po/ja.podocs/po/ko.podocs/po/messages.potdocs/po/th.podocs/po/vi.podocs/po/zh-CN.podocs/src/SUMMARY.mddocs/src/internals/garbage-collector.mddocs/src/internals/memory-model.mddocs/src/testing/ci-gate-scheduling.mddocs/statepoint-gc-experiment.mdscripts/check_gc_env_knobs.pyscripts/gate_freshness.jsonscripts/run_memory_stability_tests.sh
💤 Files with no reviewable changes (15)
- docs/po/messages.pot
- docs/po/ja.po
- docs/po/de.po
- crates/perry/tests/gc_closure_self_pointer_root_7055.rs
- benchmarks/gc_ratchet/baseline/gc-ratchet-v1.json
- docs/po/es.po
- docs/po/vi.po
- docs/po/it.po
- docs/po/fr.po
- benchmarks/gc_ratchet/gc_ratchet.py
- crates/perry/tests/gc_array_prototype_hole_read_6981.rs
- docs/po/ko.po
- docs/po/zh-CN.po
- docs/po/th.po
- docs/po/id.po
Comment on lines
+401
to
+404
| - ~~The statepoint lowering has no static root-dominance checker.~~ **Closed by | ||
| #7663.** `gc-root-dominance-statepoints` reads the production statepoint | ||
| rewrite and checks `gc.statepoint` `"gc-live"` bundles. The shadow and native | ||
| arms remain separate contexts because they inspect different IR contracts. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
rg -n -C 3 'Statepoint-side static checker|gc_root_dominance_check|gc-root-dominance-statepoints|`#7663`' docs/engine-plan.md docs/src/internals/garbage-collector.mdRepository: PerryTS/perry
Length of output: 2022
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- relevant plan sections ---'
sed -n '380,415p;545,570p' docs/engine-plan.md
printf '%s\n' '--- checker references and implementation ---'
fd -i 'gc_root_dominance_check.py|gc-root-dominance-statepoints' .
rg -n -C 5 'gc_root_dominance_check|gc-root-dominance-statepoints|relocation bundles|gc-live|statepoint' scripts docs .github 2>/dev/null | head -240
printf '%s\n' '--- recent file metadata ---'
git status --short
git diff --statRepository: PerryTS/perry
Length of output: 24520
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- all exact checker references ---'
rg -n -C 4 'gc-root-dominance-statepoints|gc_root_dominance_check|Statepoint-side static checker|relocation bundles' .
printf '%s\n' '--- checker outline ---'
ast-grep outline scripts/gc_root_dominance_check.py
printf '%s\n' '--- checker source ---'
wc -l scripts/gc_root_dominance_check.py
sed -n '1,260p' scripts/gc_root_dominance_check.py
printf '%s\n' '--- workflow and script references ---'
rg -n -C 6 'root-dominance|dominance_check|gc-live|reloc' .github scripts | head -300Repository: PerryTS/perry
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
import re
plan = Path("docs/engine-plan.md").read_text()
checker = Path("scripts/gc_root_dominance_check.py").read_text()
wiring = Path("scripts/gc_gate_wiring_check.py").read_text()
workflow = Path(".github/workflows/gc-root-dominance.yml").read_text()
checks = {
"plan_marks_statepoint_gate_closed": bool(re.search(
r"statepoint lowering has no static root-dominance checker.*?Closed by\s+#7663.*?gc-root-dominance-statepoints",
plan, re.S)),
"plan_lists_checker_work_as_open": bool(re.search(
r"\*\*Statepoint-side static checker\*\*.*?gc_root_dominance_check\.py.*?read relocation bundles",
plan, re.S)),
"wiring_maps_statepoint_gate_to_same_script": bool(re.search(
r'gc-root-dominance-statepoints".*?gc_root_dominance_check\.py --statepoints',
wiring, re.S)),
"wiring_describes_relocation_bundles": "gc.statepoint relocation" in wiring,
"checker_has_statepoints_mode": "--statepoints" in checker,
"checker_documents_relocation_bundles": "gc.statepoint" in checker and "relocation bundles" in checker,
"workflow_defines_statepoint_job": "gc-root-dominance-statepoints" in workflow,
}
for name, value in checks.items():
print(f"{name}={value}")
assert all(checks.values()), checks
print("CONCLUSION=the open worklist item is stale; `#7663` closed relocation-bundle support in the same checker")
PY
printf '%s\n' '--- exact supporting source excerpts ---'
sed -n '64,83p' scripts/gc_gate_wiring_check.py
sed -n '168,190p' scripts/gc_root_dominance_check.py
sed -n '3178,3190p' scripts/gc_root_dominance_check.py
rg -n -C 2 'gc-root-dominance-statepoints|--statepoints' .github/workflows/gc-root-dominance.ymlRepository: PerryTS/perry
Length of output: 5305
Close the stale statepoint-checker work item.
gc-root-dominance-statepoints is the --statepoints mode of gc_root_dominance_check.py and checks gc.statepoint relocation bundles. Mark Lines 562–563 as closed by #7663.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/engine-plan.md` around lines 401 - 404, Update the statepoint-checker
work item at Lines 562–563 to mark it closed by `#7663`, referencing
gc_root_dominance_check.py’s --statepoints mode and its gc.statepoint
relocation-bundle checks.
Comment on lines
+138
to
+149
| As of 2026-08-11, branch protection requires `lint`, `cargo-test`, `parity`, | ||
| `compile-smoke`, `api-docs-drift`, `security-audit`, and | ||
| `conformance-smoke-complete`. The GC-specific coverage is split deliberately: | ||
|
|
||
| | check | where it runs | required status | | ||
| |---|---|---| | ||
| | root-holder custody and GC-knob drift self-tests/live scans | `test.yml` → `lint` | yes (`lint`) | | ||
| | runtime unit suite and `run_memory_stability_tests.sh` four-mode matrix | `test.yml` → `cargo-test` | yes (`cargo-test`) | | ||
| | emitted root dominance, including native statepoint IR | `gc-root-dominance.yml` | not currently branch-required | | ||
| | pinned collector counters/RSS/wall matrix | `gc-ratchet.yml` | not currently branch-required | | ||
| | thread-local mechanism/policy budget | `tls-budget.yml` | not currently branch-required | | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Correct the required-context list.
compile-smoke is configured with continue-on-error: true in .github/workflows/test.yml Line 1947. It also does not run for normal pull requests without the run-extended-tests label. Do not list it as branch-protection-required GC validation unless the workflow becomes a mandatory failing PR gate.
As per coding guidelines, a CI gate must not use continue-on-error: true and must be included in required branch-protection contexts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/src/internals/garbage-collector.md` around lines 138 - 149, Update the
required-context statement and table in the GC documentation to remove
compile-smoke from branch-protection-required validation. Keep it listed only if
explicitly described as non-required or informational, and ensure the documented
required checks match mandatory failing PR gates that run for normal pull
requests.
Source: Coding guidelines
Comment on lines
+39
to
+45
| CLAIM_ROOTS = ( | ||
| "docs", | ||
| "scripts", | ||
| "benchmarks/gc_ratchet", | ||
| ".github/workflows", | ||
| ) | ||
| CLAIM_SUFFIXES = {".json", ".md", ".po", ".pot", ".py", ".sh", ".yaml", ".yml"} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Scan Rust test configurations as executable claims.
CLAIM_ROOTS excludes crates/, and CLAIM_SUFFIXES excludes .rs. A deleted knob in a Rust integration test environment can therefore bypass this audit. This PR already changes executable GC settings in crates/perry/tests/.
Add Rust test configuration files as claim sources. Continue to exclude test files only from parsed_knobs(), because parser ownership must remain production-only.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/check_gc_env_knobs.py` around lines 39 - 45, Update CLAIM_ROOTS to
include the crates test configuration tree and CLAIM_SUFFIXES to include .rs so
Rust integration-test settings are scanned as executable claims. Preserve
parsed_knobs() behavior so Rust test files remain excluded from parser ownership
and production-only parsing.
proggeramlug
pushed a commit
that referenced
this pull request
Aug 11, 2026
proggeramlug
added a commit
that referenced
this pull request
Aug 12, 2026
…r — interp −11.2% instructions (#7907) * perf(codegen): make the PIC miss block dominated by the token block The polymorphic-way block re-derived every value the way compares need -- four header loads, keys_array, parent_class_id, the token select and a second pair of epoch loads -- because #7883 routed the two receiver-validation failures into the same block, leaving those values live on only some edges. Route those two edges to a new pic.miss.cold instead: a receiver that fails them also fails way_hit, so the compares were dead work for it, and with them gone pic.miss is dominated by pic.token and can use its values directly. Also spell the cached-slot bound as slot < FLOOR || slot < field_count instead of materialising max(field_count, FLOOR); identical predicate, one fewer dependency on the field_count load. * perf(codegen): reduce the PIC way (token, slot) match as a balanced tree The left fold made way_slot a chain of PIC_WAYS dependent selects whose last node is the operand of the bounds compare that gates the branch out of pic.ways. At most one way can hold a given token, so the association is free to change. Adds the codegen contracts for both halves of #7902. * docs(codegen): reference PR #7907 and add the changelog fragment Also fixes the pic.ways slice in way_slot_reduction_is_a_balanced_tree: block labels carry a numeric suffix, so the search for the next block has to start past this block's own label or it matches itself. --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
This was referenced Aug 12, 2026
proggeramlug
pushed a commit
that referenced
this pull request
Sep 13, 2026
Per untyped `obj.prop`, `lower_generic_property_get` expanded 33 basic blocks, 191 pre-RS4GC IR instructions and SIX runtime call sites: an SSO arm, an INT32 class-ref arm, a nullish-throw arm, a non-object-receiver arm, an overflow-slot load, a deleted-slot miss, two Array-subclass named-prefix ladders, and the miss+prime. Every call is a statepoint, so `.text` and `.perry_gcmap` both scale with call sites x live GC values -- on @babel/parser that tower was 29% of all emitted IR across 6,487 sites. The hit is worth its bytes; the arms around it are not. What stays inline is what it was: the receiver-tag test, the small-handle test, the packed header kind/descriptor word, the packed-MRU compare, the overflow-bit test, the inline field load with its hole check, the polymorphic ways (#7753), and the `.length`/`.size` short-circuits. Everything else branches to one of two new runtime entries that reproduce the arms in the same ORDER and with the same cache-priming decisions: js_object_get_field_ic_nonptr(obj_bits, key, site_id) the receiver-tag ladder -- SSO, INT32 class ref, nullish TypeError, and the by-name fallback for every other tag. Takes no cache: none of these arms can prime one. js_object_get_field_ic_slow(obj_handle, key, cache_slot, packed) the heap-pointer arms -- the overflow slot (including its fallback to the THREE-argument miss entry, i.e. WITHOUT republishing the packed word, which is what the old helper did), the deleted-slot miss, the Array-subclass named-prefix proof, and the priming `get_field_ic_miss_impl`. Per site: 33 -> 14 tower blocks, 191 -> 106 IR instructions, 6 -> 2 calls. @babel/parser .text -14.4%, .perry_gcmap -11.7%, O0-fallback units unchanged. TWO exits and not one, measured. With the receiver-tag test and the small-handle test failing to the SAME block, SimplifyCFG folds them into one flat predicate -- `cmp; sete; cmp; setae; test; je` where the chain was `cmp; jne; cmp; ja` -- and every property-read HIT pays +4.00 instructions on a 10M-read monomorphic loop. That is #7883's flat-predicate cost arriving from the optimiser instead of from codegen. Distinct callees keep the chain branchy and let the unmasked receiver bits die in the entry block instead of living across the whole hit path. Three further shapes were needed to finish paying for the hit, each measured on the same 10M-read loop (instructions retired, mine vs base): * the field address is a typed `gep double` rather than `shl 3` + `add`. With an explicit shift the slot is the packed word's last use, so InstCombine folds `(packed >> 32) << 3` into `(packed >> 29) & mask` and isel pays a 10-byte `movabs` plus an `and`; as a GEP index there is no shift to fold and the scaled addressing mode survives. +3.00 -> +1.00 per hit. * the spill-buffer read moved into a `#[cold] #[inline(never)]` `overflow_arm`. Inlined, its `overflow_get` call forced the entry to save callee-saved registers, which gave it a frame and stopped the miss handler from being a sibling call. +49 -> +31 -> +20 per megamorphic miss (the first step was the exit split). * the named-prefix conjunction asks `ObjectMeta` first: that is one load from the header line the entry has already touched, against two dependent loads through the site's cache slot, and an object with no metadata record cannot carry the token. +20 -> +17 per miss. Final micro numbers, instructions retired per read vs base: monomorphic hit +1.00 (one `jmp`, because the hit's and the way's hole checks become congruent after their empty successors fold and SimplifyCFG tail-merges them -- fixing it needs `!prof` branch weights the IR builder cannot emit today), 4-shape polymorphic (the inline ways) -1.24, megamorphic miss +17.02. Registries updated with the new symbols: runtime declarations, the `cold` placement hint, the eh_mode throwing-callee assertion, gc_call_effects' allocating-helper list, and POLL_CAPABLE_RUNTIME in scripts/gc_root_dominance_check.py (both directions of its property-GET self-test fixture now run against both entries -- omitting them would silently re-open the #7154 GET hole, since the calls they replaced no longer appear at those sites). `test-files/test_gap_generic_get_one_exit_arms.ts` is the behavioural witness: every arm that moved out of line, checked against node's own output. PERRY_IC_DIAG counters are identical between the two toolchains on a deterministic fixture (41,216 misses over 5 sites, same per-reason split, same 20,216 primes, same fresh/armed/megamorphic distribution), and the gap-suite subset (10 filters, 290 tests) has byte-identical verdicts. (cherry picked from commit a46769d)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #7877.
What changed
Reproduction
Before the fix, the new live audit failed on
PERRY_GEN_GC_EVACUATEclaims in the required memory-stability script, ratchet metadata, current memory-model page,CLAUDE.md, and generated gettext catalogs even though production has no parser for it.Validation
python3 scripts/check_gc_env_knobs.py --self-testpython3 scripts/check_gc_env_knobs.pypython3 scripts/gc_runtime_root_holders.py --self-testpython3 scripts/gc_runtime_root_holders.pypython3 -m unittest discover -s tests -p 'test_gc_ratchet.py' -v(92 passed)python3 benchmarks/gc_ratchet/gc_ratchet.py validate --scope structuralcargo fmt --all -- --check./docs/i18n.sh build enbash -n scripts/run_memory_stability_tests.shbash scripts/check_file_size.shactionlintreports the same three pre-existing ShellCheck findings as cleanmain; this change adds none.Summary by CodeRabbit
Documentation
Maintenance
Quality