perf(regex): drop the traced-source side table, share one program set per header (72→56 B), tag the matcher kind - #9918
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.
Since PerryTS#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`, PerryTS#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.
…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.
…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. PerryTS#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.
…S#9890 is fixed The comment at the `Expr::RegExp` lowering described the artifact-discarding bail-out in `codegen/method.rs` in the present tense. PerryTS#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.
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
Replace whole-map overflow clears with one-entry eviction, and keep content-cache entries pinned while a recorded literal site refers to them. Only dynamic or displaced-site entries can leave the bounded table. Add a sabotage test that crosses both cache bounds, collects dead nursery headers, and proves the recorded literal does not rebuild.
2a7aaff to
107d40a
Compare
jdalton
left a comment
There was a problem hiding this comment.
Review of 107d40adb9881ff5f91f94124e24907fcfea5796 (2026-09-07).
The content cache is bounded in entry count, but its new miss path needs a distinct-content workload: make_room first sums every bucket in entry_count, then evict_one_dynamic can scan all entries and call site_key::references_content for each candidate. Literal hits do not exercise this cost. Please add a workload beyond MAX_ENTRIES with pinned literals mixed with changing dynamic patterns, reporting construction CPU and retained bytes with the cache on/off. Keep full byte-comparison coverage for fingerprint collisions. The header-size/literal-loop measurements alone cannot establish that the new cache has no dynamic-construction regression.
Validation scope: source/diff inspection; I have not run this PR's build or test suite locally.
Stacked on #9892 (
perf/regex-literal-site-key@ 91a7791). Three runtime-only commits, written by codex from issue #9908's design lead and the segmenter lane's matcher flag, not yet compiled (dev box out of disk at the time); gates and the cc-rig measurement run on perrymaster and will be appended here.What changes
0178825dd— removeREGEX_SOURCE_TABLE. It existed (RegExpRouter: trie.buildRegExp() returns non-RegExp object in slot 0; matchers[method] = arr corrupts keys_array #637) because the header'spattern_ptr/flags_ptrwere raw pointers into strings the GC could free while the header wasgc_malloc'd and untraced. Since perf(regex): allocate the RegExp header in the nursery, not the malloc arm #9845 those two slots are traced GC edges, so.source,.flags, lazy compilation and the RegExp-pattern constructor arm read them directly; every insert, lookup, move-rekey and death removal of the table is gone. Write barriers added whereRegExp.prototype.compilereplaces the two edges.EscapeRegExpPatternis WTF-8 byte-aware so.sourcepreserves lone surrogates.REGEX_POINTERSis untouched (the copied-minor finaliser enumerates it).1469d8089— one header-ownedArc<Programs>handle instead of three per-object matcher pointers; the content cache holds the same shared set, the literal-site cache one weak reference to the complete set. Header 72 → 56 B (−22 %);header_byteswas 72.3 MB per 3300-char reply.6f98d35d5—MatcherKindtag in the former padding byte:regexp_test_str_boundedcompiles once, branches once, borrows the selected matcher without cloning an Arc (previously twoensure_regex_compiled+ Arc clone/drop per call, ~172 k calls per reply from the segment loop).Expected counters (cc rig, one 3300 reply)
side_table_inserts / new: 2.00 → 1.00;regex_header_clear_dead_for_gchalf its removals (was 2.63 % of the thread in the probe profile);header_bytes−22 %; compile counts andsite_key_hitunchanged.Tests (named; not yet executed)
regexp_construct_reads_source_and_flags_from_the_pattern_header,regexp_compile_replaces_the_header_source_and_flags,regexp_source_round_trips_wtf8_lone_surrogates_from_the_header,regexp_header_is_one_56_byte_per_object_record,bounded_test_matcher_tag_routes_fancy_patterns_to_fancy_regex(sabotage: a(?<=left)rightpattern whose standard program is the never-match placeholder — a wrongStandardtag fails it across lazy build, born-built cache hit, andcompile).Gates
Not run (disk):
cargo build --release -p perrydefault features,nm, the runtime suite, the regex filter, test262's RegExp subset. Draft until perrymaster's seam runs them.Gate history
cc98590a7:cargo build --release -p perryrc=0, but that builds the compiler'sdefault-features = falseruntime copy. The archive build (--features perry-runtime/wasm-host) and the lib test target failed:error[E0425]: cannot find valueowned_flagsin this scopeatregex.rs:1351(a binding present only in the other cfg's arm).1158dff8c(head): "fix(regex): retain canonical flags through allocation" threadsowned_flagsthrough thematch site_entrytuple on both arms and touches its five call sites. Re-gating under the archive feature set on perrymaster; rows follow.1158dff8cre-gated on perrymaster (picked onto the site-key tree, stamp0f867bbb4):cargo build --release -p perryrc=0; full-feature archives rc=0; runtime suite 3231 passed / 0 failed / 4 ignored with the five named tests green. test262 RegExp subset not run (novendor/test262on that host) — the remaining gate before un-draft.Measured (perrymaster, cc 3300-char reply, 5-round alternating rotation, load 1.15–1.32; paired deltas only — see caveat)
Per round this PR beats main by 0.19–0.36 s (5/5, ≈ −6 %) and the site key alone by 0.01–0.09 s: the CPU win is the site key's; this PR's contribution is memory. Peak −13…−24 MB vs the site key and −5…−13 vs main; settled −28 vs the site key and −13 vs main, i.e. it removes the site key's residual +15 MB and ends below main. 400-char turn: peak 516 vs 545 main (−29).
Counters on one reply (whole
[regex-diag]line,new=1152724):side_table_inserts/new = 1.000(was 2.000) withsrc_ins=0;header_bytes/new = 56.0B (was 72.0, −22 %);site_key_hit99.79 % ofnew;exec/test/match/replacecounts unchanged; compilesstd=208vs 209 before (one fewer, to explain),fancy=88andrepeat=33unchanged.Caveat: every arm's absolute CPU on this host now reads ~35 % above the same rotation's afternoon values (a foreign 27 %-CPU service appeared on the box); the within-rotation pairs above hold, cross-rotation absolutes do not.
Follow-up
107d40adb— preserve live literal programs on eviction (uncompiled; perrymaster stage I6d)The per-pattern diag diff between the site-key tree and this branch showed three patterns built more often per reply (the 12.8 KB emoji
/gliteral 1 → 2,\s+2 → 3,\[1m\]/i 1 → 2) withcache_clears=2on both arms. Mechanism: the removed source table held only source/flags text, never programs. This branch's oneWeak<Programs>per literal site (the site-key tree held three weak matcher references that the engine maps kept alive) could no longer upgrade after the 1,024-slot content table overwrote a colliding entry and young-header finalization released the last strong reference; independently, the four 512-entry engine/validation maps cleared wholesale on overflow.Change: the content construction cache is a bounded fingerprint map with collision buckets and full byte verification; capacity eviction happens only on a distinct-content miss and never evicts an entry whose (pattern, flags) is still recorded in the literal-site table; a content-owned build publishes one weak bundle reference to every matching literal site (no strong site → program table); the engine maps evict one entry instead of clearing.
[regex-diag]keepscache_clearsas a zero control and addsevictions=. Named sabotage test:literal_site_program_is_not_rebuilt_after_cache_overflow_and_young_collection.Acceptance on the cc reply: every pattern
builds ≤ 1,lazy_builds≈ 126,compiles std ≤ 208,cache_clears=0, retention not above the census finding; paired CPU vs the previous head not slower. Open question the per-pattern table answers: cc's 1,062 live literal sites exceed the 1,024-entry table; if the overflow set still rebuilds, the table must be sized above the working set.