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.
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