perf(object): let a RegExp answer the descriptor-summary probe - #9862
Closed
proggeramlug wants to merge 2 commits into
Closed
perf(object): let a RegExp answer the descriptor-summary probe#9862proggeramlug wants to merge 2 commits into
proggeramlug wants to merge 2 commits into
Conversation
added 2 commits
September 6, 2026 10:18
`set_last_index_throwing` asks `get_property_attrs(re, "lastIndex")` on every global or sticky `test()`/`exec()`, because a user may make `lastIndex` non-writable and the spec's `Set(R, "lastIndex", n, true)` must then throw. That question is meant to be answered by PerryTS#6759 phase C2's per-object meta summary without touching the tables — but `may_have_descriptor_entry` reached the summary through `meta_capable_object`, which answers only for `GC_TYPE_OBJECT`. A `RegExp` is its own cell type, so the filter returned the conservative "maybe" for every RegExp receiver and the probe ran: `key.to_string()` — a `String` allocation — plus a SipHash of `(usize, String)`, on roughly 96,500 global `test()` calls per 400-character claude-code reply. The capability was already there and simply unwired. PerryTS#6759 phase 1 unified the metadata edge behind `cell_meta_slot`, which answers for Object, Error, Map, Set, RegExp, Promise and Date; `RegExpHeader::meta` is traced by `GcLayoutSlotKind::RegExpFields` and moves with its header. So this adds no state and no new invariant: it asks the narrower question the summary actually needs (`descriptor_summary_meta`) instead of the `ObjectHeader`-shaped one the other callers of `meta_capable_object` need, and every cell type with a meta edge benefits, not only RegExp. The three-way answer is the contract. `None` means the cell type has no meta edge and the caller must stay conservative; `Some(null)` means the edge exists and no record was ever installed, which PROVES absence; `Some(meta)` means read the summary words. Collapsing the first two would turn a conservative "maybe" into a false "no" for the types that still lack an edge. Install and probe move together, which is the safety argument: all five descriptor-summary sites now share one predicate, so an owner whose install set the key bit is always found. Every insert into `property_descriptors` / `accessor_descriptors` routes through `set_property_attrs` / `set_accessor_descriptor` and therefore through `note_meta_descriptor_key`; the touches outside this module are all removals, which can only make a probe more conservative. `js_regexp_new` writes `meta = null` on every construction, so a fresh header at a recycled address cannot inherit a dead tenant's bits. Counters, diagnostic only and armed with `PERRY_REGEX_DIAG`: `desc_regexp_probes` (RegExp receivers this filter sees) and `desc_regexp_meta_negative` (those it now proves absent). The second was 0 by construction before this change. Claude-Session: https://claude.ai/code/session_014knX724SYDogwzsXybCGxp
proggeramlug
added
the
run-extended-tests
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 8 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe descriptor-summary path now reads metadata from RegExp cells. RegExp probes can prove absent keys without table lookups, preserve descriptor detection, and report diagnostic counters. Tests cover fresh RegExp objects and non-writable ChangesRegExp descriptor summary
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to RegExp descriptor probes now avoid unnecessary table lookups while preserving detection of installed non-writable lastIndex descriptors. The covered behavior and passing runtime tests indicate no remaining merge-blocking risk. Sequence Diagram(s)sequenceDiagram
participant RegExp
participant may_have_descriptor_entry
participant descriptor_summary_meta
participant DescriptorTables
participant RegexDiag
RegExp->>may_have_descriptor_entry: probe descriptor key
may_have_descriptor_entry->>descriptor_summary_meta: read RegExp metadata
descriptor_summary_meta-->>may_have_descriptor_entry: absent or possibly present
may_have_descriptor_entry->>RegexDiag: record RegExp probe
alt metadata proves key absent
may_have_descriptor_entry-->>RegExp: return absent
else metadata permits a descriptor
may_have_descriptor_entry->>DescriptorTables: check descriptor tables
DescriptorTables-->>may_have_descriptor_entry: return descriptor result
may_have_descriptor_entry-->>RegExp: return result
end
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
Contributor
Author
|
Landed on |
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.
What this is
set_last_index_throwingasksget_property_attrs(re, "lastIndex")on everyglobal or sticky
test()/exec(), because a user can makelastIndexnon-writable and the spec's
Set(R, "lastIndex", n, true)must then throw(test262
prototype/{exec,test}/y-fail-lastindex-no-write). That question ismeant to be answered by #6759 phase C2's per-object meta summary without
touching the descriptor tables — but
may_have_descriptor_entryreached thesummary through
prototype_chain::meta_capable_object, which answers only forGC_TYPE_OBJECT. ARegExpis its own cell type, so the filter returned theconservative "maybe" for every RegExp receiver and the slow probe ran every
time:
key.to_string()— aStringallocation — plus a SipHash of(usize, String)and a map lookup that was always going to miss.The capability was already there and simply unwired. #6759 phase 1 unified
the metadata edge behind
object::cell_meta_slot, which answers for Object,Error, Map, Set, RegExp, Promise and Date;
RegExpHeader::metais traced byGcLayoutSlotKind::RegExpFieldsand moves with its header. So this adds nostate and no new invariant. It asks the narrower question the descriptor
summary actually needs (
descriptor_summary_meta) instead of theObjectHeader-shaped onemeta_capable_object's other callers need.The general rule, which is the point
This is not a RegExp patch. All five descriptor-summary sites
(
get_property_attrs,get_accessor_descriptor,own_descriptor_may_cover_key,owner_may_have_descriptor_entries,note_meta_descriptor_key*) now share onepredicate over
cell_meta_slot, so every cell type that owns a meta edge cananswer a descriptor probe from its own header — Error, Map, Set, Promise and
Date as well as RegExp. RegExp is merely the type claude-code drives hardest.
The three-way answer is the contract
Collapsing the first two into one
Nonewould turn a conservative maybe intoa false no for the cell types that still lack an edge (Temporal, the
typed-array views). That is why
descriptor_summary_metareturnsOption<*mut ObjectMeta>rather than a bool.Counters — measured, pre-registered, and one prediction missed
One 400-char streamed reply through the offline rig (
stream_scale.py, chunk100, sandbox
rx,PERRY_REGEX_DIAGarmed), on a binary relinked from thisbranch's runtime source. One binary serves both arms: before this change
desc_regexp_meta_negativeis 0 by construction, because the filter answered"maybe" for every RegExp receiver — so the negative count measured here is
exactly the work this change removes.
desc_regexp_probestest_global(~96,500); ~0 would mean inertdesc_regexp_meta_negativeprobescompiles std / fancy / repeatrxA baseline (211 / 88 / 33)new,site_hitThe missed prediction, stated as a miss. The pre-registration derived the
probe count from one call site —
set_last_index_throwing, at ~1 probe perglobal
test(). The counter, deliberately, sits at the shared predicate, soit also counts the other descriptor-summary consumer that meets a RegExp
receiver:
js_native_call_method'sget_accessor_descriptoron the receiver ofevery native method call, which for
re.test(s)is the same RegExp — plus theSet(obj, "lastIndex", 0, true)thatRegExpInitializestep 12 runs on everyconstruction, and this loop constructs a fresh
RegExpper call (84,387newsagainst 84,398 global tests). Measured rate: 5.02 probes per global
test().The split between those sites is not separately counted and is not claimed
here; the total is. The direction of the registration holds and the magnitude is
five times larger than predicted, which strengthens rather than rescues the
change.
new/site_hit"unchanged" also failed, and it was the wrong statement toregister. The two captures are different runs with different diag windows
(46.2 s vs 10.1 s). All four per-call counters moved together by 12.2–12.5 %
while every ratio among them is preserved:
site_hit/new0.9895 → 0.9880,test_global/new0.9468 → 0.9435,test/test_global2.0154 → 2.0167. Thecompile counts, which are a property of the program rather than of how many
renders the turn did, moved by at most one pattern each (the distinct-pattern
table went 1,056 → 1,061). So the workload's shape is identical and its
absolute level is not — an invariant that should have been registered as a
ratio. Nothing here indicates a semantic change, and no regex is compiled
differently.
Absolute counts are a property of the sandbox project — the same interaction
reads
test_global96,511 inrxand 180,802 inint— so 5.02 probes perglobal
test()is the portable number, and the removed work is oneStringallocation plus one SipHash plus one always-missing map lookup, each.
Correctness
cargo test --profile gcaudit -p perry-runtime -- --test-threads=1:3,171 passed / 0 failed / 4 ignored, first attempt. This is the gate that
matters here: the meta record is GC-allocated and moves with its owner.
-D warningsgate is clean.Install and probe move together — that is the safety argument. Every insert
into
property_descriptors/accessor_descriptorsroutes throughset_property_attrs/set_accessor_descriptorand therefore throughnote_meta_descriptor_key, which now uses the installing twin of the samepredicate (
descriptor_summary_meta_ensure). A probe widened without itsinstall would answer "proven absent" for an owner that really has a descriptor.
js_regexp_newwritesmeta = nullon every construction, so a fresh header ata recycled address cannot inherit a dead tenant's bits. The touches outside this
module are all removals, which can only make a probe more conservative.
Sabotage
a_fresh_regexp_proves_lastindex_absent_without_probing_the_tablesasserts onthe filter's answer via a
#[cfg(test)]view ofmay_have_descriptor_entry, not on the value it filters to. Asserting onlyget_property_attrs(...).is_none()would pass against a change that didnothing — that is equally true when the fast negative never fires. The test
also asserts its own premise (
obj_type == GC_TYPE_REGEXP), so it cannotquietly become a test about a shaped object.
a_regexp_with_a_non_writable_lastindex_is_still_found_by_the_probeis thehalf that makes the fast negative safe: after
Object.defineProperty(re, "lastIndex", {writable:false})the probe must sendthe caller to the table and the descriptor must read back non-writable. A
probe widened without its install fails here, on that named assertion — and
set_last_index_throwingwould have silently stopped throwing.(
"source") still takes the fast negative: the summary is per key, not perowner, so widening it must not blunt it.
descriptor_summary_metato the Object-onlymeta_capable_objectfails both new tests on their own named assertions.What is NOT claimed
No CPU number. This is a counter result only. In the leaf profile of a
400-char reply the two symbols this change acts on rank at
get_accessor_descriptor1.1 % andget_property_attrs0.3–0.5 % of activesamples, and this change removes only the RegExp-receiver share of each — under
10 % of the turn by count, below what a shared 10-core box can resolve. It
therefore gets no dedicated clean-host arm; it rides the next combined
measurement, labelled. It is offered on the mechanism, the counter, and
gcaudit, not on a stopwatch.
Independent of #9845 (RegExp header in the nursery) — based on
main, noshared files with that stack.
https://claude.ai/code/session_014knX724SYDogwzsXybCGxp
Summary by CodeRabbit
Performance
Diagnostics
Compatibility
lastIndexbehavior, including errors when writing to non-writable descriptors.Tests
lastIndexdescriptors.