Skip to content

perf(runtime): prepare refreshable registry address index for #10110 - #10251

Closed
proggeramlug wants to merge 2 commits into
mainfrom
codex/issue-10110-registry-index
Closed

proggeramlug wants to merge 2 commits into
mainfrom
codex/issue-10110-registry-index

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Prepare the reusable address-index fix from #10110 on main. A 1,024-bit monotone filter loses selectivity at the reported 644-wrapper population and accumulates retired addresses indefinitely. RegistryAddrIndex supports a larger per-owner filter and rebuilds it from the live set while keeping probes lock-free.

This is a prerequisite for the canonical-wrapper integration. At base eb13fa188, native_handle/canonical.rs is absent and the three address predicates remain pure numeric checks. Issue #10110 remains open until that subsystem lands with a 256-word index and retirement wired after all three authoritative removal paths. The changelog fragment records those integration requirements. No campaign CPU improvement is claimed for main.

Changes

  • Adapt the reusable implementation and population/retirement/churn tests from perf(runtime): size the canonical-handle address index and release what it retires (-7.63% command CPU) proggeramlug/perry#1.
  • Parameterize filter width while preserving the existing 16-word symbol filter and its hash positions.
  • Scale all three hash slices with the selected width; the linked patch's fixed first slice used only the first 1,024 bits even in a wider filter.
  • Cover address reuse and concurrent admissions/refreshes, and document lifecycle ordering.
  • Add changelog.d/10251-registry-address-index.md. No workspace version, lockfile, CLAUDE.md, or CHANGELOG.md changes.

Related issue

Refs #10110; canonical-wrapper wiring remains outstanding because the subsystem is absent from main.

Test plan

  • cargo test --locked -p perry-runtime --lib registry_latch -- --test-threads=1: 36 passed, including the existing symbol/probe integration tests.
  • Full single-threaded runtime unit suite: 3,719 passed, 4 ignored, 1 failed. gc::tests::telemetry_verifier::emergency_full_trace_is_excluded_from_ordinary_pause_stats expects unsupported on Windows but receives executed; it also fails alone. The test and allocator-maintenance implementation are unchanged from the main base.
  • Mutation checks: regression tests fail with refresh disabled, the 16-word population index restored, and the original fixed hash slices restored.
  • Passed: cargo fmt -p perry-runtime -- --check, test-registration, address-classification and GC root-holder audits, and git diff --check.
  • File-size limits: checked the script's exact tracked Rust scope and allowlist in Python (4,087 non-exempt files, zero violations); stopped the slow Windows shell scan after this equivalent check.
  • Broader script lint (SKIP_COMPILE_GATES=1): 6 of 77 failed, with two CI-only checks skipped. Failures were missing jq, benchmark/release shell fixtures, workspace formatting exceeding the Windows command-length limit, benchmark freshness (inputs/artifact unchanged from the base), and the absolute unrooted-local inventory. Both raw-handle and unrooted-local comparisons against main passed: this PR adds no debt.
  • CI reports baseline failures in unchanged code: a raw thread_local! in regex/perex_owner.rs, and dead-code warnings for registered_extern_handle / wasm_memory_descriptor_maximum in object/global_this_webassembly.rs. These are outside this PR's three-file diff.

Refs #10110. Adapt the reusable index and population/churn witnesses from proggeramlug#1 for main. Scale all three hash slices with the per-owner width and cover concurrent refreshes and address reuse. The canonical-wrapper owner is still on the campaign branch; document its required integration without closing the issue.
@coderabbitai

coderabbitai Bot commented Sep 14, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 84436004-c8e3-4776-89f3-ef8e0ec30af7

📥 Commits

Reviewing files that changed from the base of the PR and between eb13fa1 and 4be2ada.

📒 Files selected for processing (3)
  • changelog.d/10251-registry-address-index.md
  • crates/perry-runtime/src/registry_latch.rs
  • crates/perry-runtime/src/symbol.rs

Included review availability: Your plan provides up to 8 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The runtime now supports const-generic registry address filters and adds RegistryAddrIndex. The index tracks live addresses, refreshes filter bits after retirements, and exposes occupancy diagnostics. Existing symbol filters retain their 16-word configuration.

Changes

Registry address indexing

Layer / File(s) Summary
Configurable registry filters
crates/perry-runtime/src/registry_latch.rs, crates/perry-runtime/src/symbol.rs
RegistryAddrFilter now accepts a const-generic word count. Hash positions and capacity use the configured width. Rebuild support publishes replacement words. Existing symbol-filter tests retain a 16-word filter.
Live address admission and retirement
crates/perry-runtime/src/registry_latch.rs, changelog.d/10251-registry-address-index.md
RegistryAddrIndex tracks live addresses, admits and retires entries, refreshes the filter after population changes, and reports occupancy and live counts. Tests cover reuse, retirement, concurrency, sizing, and churn.

Priority: ⬇️ Low

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant RegistryAddrIndex
  participant LiveAddressSet
  participant RegistryAddrFilter
  Caller->>RegistryAddrIndex: admit or retire address
  RegistryAddrIndex->>LiveAddressSet: update live set
  RegistryAddrIndex->>RegistryAddrFilter: update or rebuild bits
  RegistryAddrFilter-->>Caller: answer may_contain
Loading

Merge Risk: ⚪ Minimal · up to 4be2a

The configurable filter and address index are mergeable as a prerequisite; integration with the future canonical-wrapper subsystem remains separate work.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 36 functions across 2 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely identifies the main change: preparing a refreshable registry address index for runtime performance work related to issue #10110.
Description check ✅ Passed The description includes a clear summary, concrete changes, related issue, detailed test results, known baseline failures, and scope limitations. The optional screenshots section and checklist are not…
Full details: Docstring Coverage

Explanation

Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 36 functions across 2 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/issue-10110-registry-index

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Landed in merge train #10261: #10261. The merged main tree matches the validated train, and the fresh-head patch audit confirms the changes arrived.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant