Merge train: #9891, #9892, #9893, #9904, #9905, #9906, #9909, #9910, #9911, #9913, #9914 - #9922
Merged
Conversation
…at cannot fail silently `PERRY_REGEX_DIAG` gains four per-construction work counters — `barrier_taken` / `barrier_gated` (whose sum must equal `new`), `header_bytes`, `site_verify_bytes` and `side_table_inserts` — so what `js_regexp_new` costs per call is a number rather than a reading of a profile. Writers for the first group arrive with the change they measure; `site_verify_bytes` is written here. `site_verify_bytes` is deliberately NOT `pattern_bytes`: the latter counts every construction's pattern length whether the site-cache probe hit or missed, while the full byte compare that verifies a fingerprint match is the part that is linear in the pattern — what makes a 12 KB emoji pattern expensive and a 60-byte one free. Counted at the construction probe only; `insert` and `install_programs` verify too and are not counted here. Two reliability fixes, both of the same shape as the campaign's missing exit-line trap — an absent output that greps identically to an instrument that was never built: * a file sink that cannot write now reports the path and the error on stderr once and keeps writing there, instead of swallowing the error; * the first snapshot is written at the first tick rather than one full DUMP_INTERVAL_MS later, so a run shorter than a second produces output. (cherry picked from commit df202bf)
Since #9845 the `RegExpHeader` is a nursery allocation, so its two string field stores cannot owe the remembered set anything — and they were still taking the full barrier twice to discover that: four page-map classifications, two dirty-page-cache probes and two child classifications per construction, every one of them ending at `ParentNotOldSkips`. The gate is the runtime twin of the one the compiler already emits in front of every one of its own stores (`emit_parent_may_need_remembering_check`, #7511): `GC_FLAG_TENURED` clear on the parent's LIVE header, and a globally idle incremental mark barrier. The first clause answers the generational question; the second is what makes it legal to skip the SATB/insertion shading as well, and dropping either one is a live child swept. Both are read live, so a header a collection promoted between `arena_alloc_gc` and the store, and `RegExp.prototype.compile` reassigning a tenured receiver, still take the full path. `gc::tests::inline_generation_gate_contract` already pins those two clauses for the emitted gate against a stranded-child witness; it now pins the runtime twin to the same codegen predicate clause by clause, and a third test asserts on the header `js_regexp_new` actually returns — so the skip arm is proven REACHED, not merely available. Measured motivation (segment-loop probe, region B, 60,000 reps, `sample`, main thread, leaf sum = thread header exactly): one `RegExp` per grapheme from a literal inside a function body, and the barrier subtree under `js_regexp_new` is 739 of 14,628 main-thread samples — 32 % of that function's own subtree. `PERRY_REGEX_NEWBORN_BARRIER_GATE=0` restores the unconditional pair. With the gate off nothing else changes, so the OFF arm is the pre-change code path exactly rather than a control still carrying the bookkeeping. (cherry picked from commit 3b5f5ef)
…n text `regex::site_cache` answers "have I seen this pattern TEXT before?" — the right question for a dynamic `new RegExp(s)`, and the wrong one for a literal, which is one source site whose pattern and flags are fixed at compile time. Because a content fingerprint can collide, every hit is verified by a full byte compare of the pattern, and a literal constructs a fresh object every time it is reached: on claude-code that verify is ~2.0 GB of `memcmp` per 400-character reply and 39.6 % of `js_regexp_new`'s own profile subtree. `Expr::RegExp` now emits an 8-byte private global per literal site and passes its ADDRESS to a new `js_regexp_new_site(pattern, flags, site_key)`. The address is unique by construction, immortal and never moves — the three properties a `StringHeader` address lacks, which is why the earlier analysis concluded no sound string identity was available and left the compare in place. A hit compares one word plus the site's <= 8-byte flags text and then reads nothing about the pattern: no fingerprint, no memcmp, no validation (validity is a pure function of the pair and the site's first construction established it), no flag canonicalization (the seven bits are a property of the site), and the programs the site already compiled are installed eagerly. `site_key = 0` is "no site": every dynamic construction keeps the two-argument entry and never touches the table. Kill switch `PERRY_REGEX_SITE_KEY=0`. Tests: two sites whose patterns have EQUAL LENGTH and different text, each constructed twice — the sabotage of keying the table by pattern length hands the second site the first's entry and fails on `.source` and on `test`; four dynamic constructions leave the table empty while one site-keyed construction fills it; a second construction at an executed site is born built; and the new symbol's declaration is asserted by name AND arity, because a missing declare fails only at the LLVM parse and a wrong arity miscompiles silently. Measurement is owed on the cc rig, where the 12,807-character pattern lives — the segment-loop probe's literal is ~60 characters and its memcmp is 0.16 % of the thread, so the probe cannot show this change. (cherry picked from commit 7cbadc1)
…d counters moving the dump Two corrections from the I6 cc arm, both found by reading the instrument back rather than by argument. **1. The site table must never be the reason a program stays alive.** Measured on one 3300-char reply: settled footprint 478/474 MB -> 500/527 MB and idle CPU 2.37 -> 2.68 s against main. 1,024 entries at ~19 KB per compiled program is that order, and the campaign's directive is both metrics together — a CPU win bought with resident memory does not land. The entry now holds `Weak<Regex>` / `Weak<fancy_regex::Regex>` / `Weak<RepeatMatcherRegex>`; strong references stay where they belong, in the `(pattern, flags)` program caches and in every live header that installed them with `Arc::into_raw`. An entry whose programs have expired reports "not built yet" and the next construction re-picks them up from the content cache — the same path the site's first construction takes, so the lane self-heals. The upgrade is ALL-OR-NOTHING. #9801 fixed an incoherent triple — a standard program memoized beside a missing fancy fallback — which does not error, it silently never matches; three independent `Arc` lifetimes reintroduce exactly that shape unless one dead reference invalidates the whole entry. Pinned by a test that drops ONLY the fancy program and asserts the entry reports unbuilt, which the natural per-field upgrade fails. **2. An added counter moved the instrument's own sampling.** `regex_with` counts every call as an event and dumps every `TICK_EVERY` events after a second has passed, so a second probe on an already-instrumented path doubles that path's event rate and moves the snapshot a SIGKILLed process leaves behind. On the I6 pair that showed up as `new / t` 206 k/s vs 173 k/s between two arms whose per-call ratios agree to 0.13 %, i.e. the two files describe different windows of the same workload. `regex_counters` accumulates without ticking the dump clock, and the three counters that ride along on already instrumented paths (barrier gate outcome, side-table inserts, site-verify bytes) now use it. (cherry picked from commit 78e8b9d)
…is fixed The comment at the `Expr::RegExp` lowering described the artifact-discarding bail-out in `codegen/method.rs` in the present tense. #9896 fixed it: every return there now goes through `publish_lowered_fn_artifacts`, which drains all three collections and restores `llmod.ic_counter`, closing the duplicate site-id half as well. Rewritten as the obligation rather than the bug — every lowering exit must PUBLISH `typed_parse_rodata`, and a future early return that drops it breaks this site loudly at the in-process LLVM parse. A comment describing a hazard that no longer exists is a false lead, which is the thing it was written to prevent. (cherry picked from commit 91a7791)
… walk
`js_segments_view_regexp_test` asks "is `RegExp.prototype.test` still the
builtin?" twice per grapheme, and the proof cost more than the match it guards.
Symbolised on the view arm of the string-width probe, the proof was ~13 % of the
thread — `get_field_by_name_object_tail` 3.6, `js_object_get_field_by_name` 3.5,
`get_accessor_descriptor` 2.1, `closure_get_dynamic_prop` 1.75,
`RandomState::hash_one<&str>` 1.4 (it hashed the key string on every call),
`js_object_get_prototype_of` 1.3 — against 0.8 % for `regexp_test_str_bounded`,
the actual matching.
The property belongs to `RegExp.prototype`, not to the call, so it is recorded
once when the prototype's methods are installed: the prototype pointer, the
FIELD INDEX of its own `test`, and the canonical closure value. A call reads
that slot by index and compares — three loads — plus the per-key accessor Bloom
bit off the meta record. Everything it can get wrong, it gets wrong in the
declining direction: a replaced or deleted `test` no longer matches the recorded
closure; a reshaped prototype makes the index hold something else, which also
does not match; `defineProperty(proto,"test",{get})` leaves the data slot alone
and is caught by the accessor bit; a reparented receiver is caught by
`object_static_prototype`, which answers from the object's own meta record or an
atomic "nothing was ever recorded" latch — no mutex, no chain walk.
Deliberately NOT a flag invalidated from the property-set path: that design
makes every property store in the program pay for this one question and adds an
invalidation surface that fails silently. This one hooks no shared write path.
`REGEXP_PROTOTYPE_TEST_WALKS` counts by-name walks. The fast path does none, so
it counts realms rather than calls, and the tests pin the property that matters:
50 accepted calls, plus a second cursor and a second regex, add ZERO walks; and
patching `RegExp.prototype.test` after the site is recorded makes the very next
call decline, so the caller materialises and runs the user's function.
Also removes `report_segview_counters`, which had no caller. The counters now
have one — the test suite — and a comment says to wire a runtime-side printer
when a rig run needs the numbers, not before.
`cargo test -p perry-runtime --release --lib -- --test-threads=1`: 3,239 passed,
0 failed.
(cherry picked from commit 638b832)
Every view entry point re-derives the input `&str` from the cursor's traced slot per call — that is the §9a rooting contract and it stays — but each derivation also re-ran `std::str::from_utf8` over the WHOLE input. On the symbolised view arm of the string-width probe that was the top self symbol at 6.2 %. `open` already validates: it refuses an input that is not already a string primitive and runs `from_utf8` on its bytes before allocating the cursor. So the per-call validation re-establishes something the slot's only writer guaranteed. The borrow now uses `from_utf8_unchecked`, with the invariant written out where the `unsafe` is, in four checkable parts: `F_INPUT` is written exactly once, by `open`, and never reassigned; `open` validated that value; a collection MOVES the string but never rewrites its bytes, and the traced slot is updated to the new address; the SSO path decodes the same value into the stack buffer. A `debug_assert` re-checks it in debug builds, which is where a future second writer to the slot would be caught. `cargo test -p perry-runtime --release --lib -- --test-threads=1`: 3,239 passed, 0 failed. (cherry picked from commit ea54f0a)
The canonicality fast path records the prototype address and the canonical `test` closure and reads them on every call — and neither was scanned. An address held across a collection without being visited is stale the first time the collector moves the object, which is the #9539 / #9445 shape and exactly what this campaign keeps finding. Nothing had failed yet because a realm prototype is long-lived and rarely moves; that is luck, not a design. The packed `(index << 48) | ptr` word is split so each part can be handled correctly: * `REGEXP_PROTOTYPE_PTR` — a raw address, visited by `scan_object_cache_roots_mut` with `visit_atomic_i64_slot` beside the iterator-prototype towers, so a move rewrites it; * `REGEXP_PROTOTYPE_TEST_CLOSURE` — a NaN-boxed word, visited with `visit_atomic_nanbox_u64_slot` and stored through `runtime_store_root_atomic_nanbox_u64` with the GC_STORE_AUDIT(ROOT) note the other mutable roots carry, so the pointer inside it is rewritten too; * the field index is not an address and stays an ordinary atomic. The per-call cost is unchanged — three loads and the accessor Bloom bit — and the identity compare stays an identity compare across a move, because both sides are now maintained by the collector. `cargo test -p perry-runtime --release --lib -- --test-threads=1`: 3,239 passed, 0 failed. (cherry picked from commit f076656)
(cherry picked from commit f2a0f3e)
(cherry picked from commit 4f49591)
(cherry picked from commit 23f0e50)
(cherry picked from commit 48cf508)
`PERRY_SEGVIEW` is not a diagnostic — it changes the emitted code — and it is in neither the build-cache fingerprint nor any object-cache key. So a cached build can return a binary compiled with the OTHER setting: compile a source with the tier on, compile it again with the tier off, and the second can be served from the first. That is precisely the shape every A/B in this campaign uses — one compiler binary, two compiles of one source differing only in this variable — so the failure mode is not a broken build, it is two arms that are secretly the same binary and a measured difference of zero, or two arms swapped. Silent, and it would look like a result. `PERRY_SEGVIEW_DIAG` was already excluded for the weaker reason that a cached build prints no report. The switch that changes codegen was not, which is the worse omission of the two and mine. Excluded rather than keyed because the tier is experimental and default OFF. A cache key is the right fix when it ships on by default; exclusion is correct now and cannot produce a stale entry that becomes the measurement. (cherry picked from commit a423a1b)
The cursor local is declared in the ENCLOSING statement list, not inside the
loop:
let __segview_recv = <receiver>
let __segview_input = <input>
let __segview_cursor = js_segments_view_open(recv, inp)
let __segview_iter = cur != 0 ? undefined : GetIterator(...)
For { ... } <- last read of the cursor
so without a clear its slot stays a live GC root until the function returns.
The cursor holds the input string in a traced slot, so a cursor promoted during
the loop drags that string into the old generation, and leaving the slot rooted
afterwards keeps a DEAD cursor doing it for the rest of the function.
`string-width` is entered thousands of times per reply.
That is a candidate mechanism for the idle behaviour measured on cc: I4 settles
45-65 MB ABOVE I3 at 3300 and 15-20 MB at 400 after 120 s, despite winning
20-50 MB of PEAK RSS in 12/12 paired runs. Lower peak with a higher floor is
not "less garbage"; it is something being retained.
One unconditional `LocalSet(cursor, undefined)` after the loop covers both
paths: on the declined path the local holds `0.0`, a number, so the clear is a
no-op. `break` reaches it; `return` inside the body pops the frame, which is
equally fine.
WHAT THIS DOES NOT DO, stated so the commit is not read as a cure: it does not
prevent promotion DURING the loop, and nothing in the compiler can, because the
cursor is genuinely live there. It removes only the post-loop rooting of a dead
cursor. If the idle delta comes from cursors promoted mid-loop, this will not
move it. perrymaster's old-gen census after idle, counting class id
0xFFFF_000E on I5-spec / I5-view / I7-view, decides that independently.
The test is structural rather than string-matched: it locates the rewritten
`For`, reads the cursor's LocalId out of the loop's own guard, and requires the
next statement to be `LocalSet(that id, Undefined)`. It fails if the clear is
removed, clears the wrong local, or is emitted before the loop.
17/17 segview tests.
(cherry picked from commit 15ea8eb)
(cherry picked from commit c0bb08a)
(cherry picked from commit 1b5ee0d)
(cherry picked from commit c8663cf)
…he new roots Two files went over 2000 lines: - regex.rs -> regex/compile_cache.rs takes the program-compilation and cache group (size limit, std/fancy builders, eviction, the checked compile-and-cache entry). `js_regexp_new_impl` was the obvious bigger extraction and is deliberately NOT the one taken: it carries the two #7341 raw-handle debt sites, and raw_handle_debt.py's --no-raise-vs arm refuses a ceiling on a file absent at the merge base, so moving it would have forced either surgery on the allocation path two perf PRs are tuning, or a with_const_ptr(|p| p) that games the ratchet without scoping anything. - regex/tests.rs -> regex/tests_part2.rs, split at a test boundary. All 70 #[test] items are accounted for across the two files, and part2 carries the same cfg(all(test, feature)) gate as its sibling. #9893's new roots are classified: REGEXP_PROTOTYPE_PTR_SLOT and REGEXP_PROTOTYPE_TEST_CLOSURE_SLOT are covered_elsewhere, naming scan_object_cache_roots_mut, which really does visit them (as an i64 slot and a nanbox word respectively) and is registered via reg_scanner!. The test index and the walk counter are not_a_gc_pointer. NEVER_MATCH moved with the compile-cache group, so its inventory entry is retargeted. #9906's new test thread_local is recorded cold — it is a test file, and the other 23 there are recorded the same way.
… store - shape_descriptor_census asserted the dedicated GC birth kind inside `js_regexp_new`. #9892 split construction into a thin js_regexp_new / js_regexp_new_site pair over a shared js_regexp_new_impl, which is where the allocation now lives, so the census read a wrapper with no birth site. It follows the birth site instead. Verified the retargeted gate still fails when the kind is blunted inside js_regexp_new_impl. - gc_store_site_inventory wanted a marker on `REGEXP_PROTOTYPE_PTR.store`. The store is already correct — the sibling closure store spells out its barrier only because it uses with_slot and bypasses the wrapper, while RealmAtomicI64::store routes through runtime_store_root_atomic_raw_i64 itself. Marker records that.
This was referenced Sep 6, 2026
This was referenced Sep 6, 2026
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.
Merge train: #9891, #9892, #9893, #9904, #9905, #9906, #9909, #9910, #9911, #9913, #9914.
All eleven cherry-picked without a single conflict. #9892 is stacked on #9891 and picked in that order.
Two audits worth recording
#9891 skips the write barrier for a nursery
RegExpHeader's field stores. The predicate was checked against the one codegen already emits in front of every store (emit_parent_may_need_remembering_check, #7511) and matches it, including the clause that is easy to miss:newborn_parent_needs_barriershort-circuits totruewhenever incremental marking is not globally idle, before it ever looks atGC_FLAG_TENURED. Its test asserts its own premise — "a fresh nursery allocation must not be TENURED — otherwise this test exercises nothing".#9893 records two heap pointers in thread-locals, which is the unrooted-cache shape the static checker structurally cannot see. The doc comments claim they are visited by
scan_object_cache_roots_mut; that is true in code, the function is registered viareg_scanner!, and each slot is visited by the method matching its representation —visit_atomic_i64_slotfor the bare address,visit_atomic_nanbox_u64_slotfor the NaN-boxed closure word.File cap: which half of regex.rs to split
regex.rsreached 2141 lines andregex/tests.rs2121.js_regexp_new_implis the obvious extraction at 538 lines, and is deliberately not the one taken. It carries the two #7341 raw-handle debt sites, andraw_handle_debt.py's--no-raise-vsarm refuses a ceiling on a file that did not exist at the merge base — its bare arm accepts relocating the ceiling, the--no-raise-vsarm does not. That left two bad options: surgery on the allocation path both #9891 and #9892 are tuning, or awith_const_ptr(|p| p)that satisfies the regex while scoping nothing, which is the ratchet-gaming shape. Neither was taken; the debt-free program-compilation and cache group moved toregex/compile_cache.rsinstead.regex/tests.rssplit at a test boundary intotests_part2.rs. All 70#[test]items are accounted for across the two files, and the sibling carries the samecfg(all(test, feature = "regex-engine"))gate.Gate fixes
js_regexp_new/js_regexp_new_sitepair over a sharedjs_regexp_new_impl, so the census was reading a wrapper with no birth site in it. Retargeted to the birth site, and verified the retargeted gate still fails when the kind is blunted insidejs_regexp_new_impl.REGEXP_PROTOTYPE_PTR.store. This looked like a missed barrier — the sibling closure store usesruntime_store_root_atomic_nanbox_u64while this one used a plain.store(), andruntime_store_root_atomic_raw_i64exists as its exact counterpart. It is not:RealmAtomicI64::storeroutes through that barriered helper itself, and the sibling spells it out only because it useswith_slotand bypasses the wrapper. Marker added recording that.NEVER_MATCHmoved with the compile-cache group, so its holder entry is retargeted; perf(intl): take the view mode's canonicality proof and UTF-8 re-validation off the per-grapheme path #9893's four new holders are classified (twocovered_elsewherenaming the scanner, twonot_a_gc_pointer); fix(stream): wait for both finished sides #9906's new testthread_local!is recorded cold, which is what its 23 siblings in that file already are.Validation
run_lint_gates: all 64 gates passed; 2 CI-only skipped