From 89be3b1feda125b29f22a012b39c563b52797eee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 10:35:08 +0200 Subject: [PATCH 1/5] perf(regex): close the backtracking cliff, allocation-free cache probes, engine prototype switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebased onto main after #9764 landed as ddbe0b126; the site cache, header-authoritative lookups and find-only global `test` are main's now and are gone from here. The program-cache coherence fix moved to #9801. Three changes remain. * The capture-group cliff. `repeat_matcher::capture_layout` takes a pattern off the linear engine when ECMA-262's RepeatMatcher capture semantics are observable — a capture directly under a quantifier, or a capture inside a negative lookaround. That routing is a correctness requirement, but the engine it routes to is a classical backtracker with no step budget, so adding parentheses fell from linear time to exponential (`/^(a+)+$/.test("a"*28 + "!")`: 16,522 ms; node 4,798 ms). 6.3 % of 4,463 real literals take that route. Both engines accept the same LANGUAGE and differ only in capture ASSIGNMENT, so `linear_rules_out_match` asks the linear program first and a subject it rules out — which is what every ReDoS input is — never reaches the backtracker. This removes the reachable exponential case; it does not bound the worst case (that needs the step budget open upstream as ridiculousfish/regress#177). * Allocation-free cache probes. The three compiled-program caches were `HashMap<(String, String), _>`, so every probe allocated two Strings and copied the pattern text, once per RegExp OBJECT. `ProgramKey = (Arc, Arc)` makes a probe two refcount increments; the remaining materialisations are cold (`RegExp.prototype.compile`, the syntax-error fallback). * `PERRY_REGEX_ENGINE=regress`, off by default: routes every pattern through the ECMAScript backtracker and installs a shared never-match placeholder as the standard program, so the tier-0 engine architecture can be measured in a real binary. Not a supported configuration — the backtracker has no budget. Tests: quantified_capture_pattern_does_not_backtrack_on_a_non_matching_subject, plus the `capture_layout` predicate assertions rewritten around the `(layout, needed)` pair. Claude-Session: https://claude.ai/code/session_01YPfnmWZmSpSWpmnoXvH8z2 --- changelog.d/regex-backtracking-cliff.md | 43 ++++ changelog.d/regex-borrowed-cache-keys.md | 26 +++ changelog.d/regex-engine-prototype-switch.md | 31 +++ crates/perry-runtime/src/regex.rs | 205 +++++++++++++++--- crates/perry-runtime/src/regex/compile.rs | 15 +- crates/perry-runtime/src/regex/exec.rs | 8 +- crates/perry-runtime/src/regex/lazy.rs | 11 +- crates/perry-runtime/src/regex/match_all.rs | 2 +- .../perry-runtime/src/regex/match_string.rs | 2 +- .../perry-runtime/src/regex/repeat_matcher.rs | 55 ++++- .../perry-runtime/src/regex/replace_expand.rs | 2 +- crates/perry-runtime/src/regex/tests.rs | 60 ++++- 12 files changed, 391 insertions(+), 69 deletions(-) create mode 100644 changelog.d/regex-backtracking-cliff.md create mode 100644 changelog.d/regex-borrowed-cache-keys.md create mode 100644 changelog.d/regex-engine-prototype-switch.md diff --git a/changelog.d/regex-backtracking-cliff.md b/changelog.d/regex-backtracking-cliff.md new file mode 100644 index 0000000000..f1b388b8e8 --- /dev/null +++ b/changelog.d/regex-backtracking-cliff.md @@ -0,0 +1,43 @@ +### Performance + +- **A capture group no longer turns a pattern into a ReDoS.** + `repeat_matcher::capture_layout` takes a pattern off the linear `regex` + engine when ECMA-262's RepeatMatcher capture semantics are observable — a + capture group directly under a quantifier, or a capture inside a negative + lookaround. That routing is a correctness requirement (the linear engine + keeps the last value of a capture nested in a quantified group; the spec + clears it on every iteration), but the engine it routes to, `regress`, is a + classical backtracker with no step budget. So adding parentheses was enough + to fall off a linear-time path onto an exponential one: + + | pattern | node | perry (before) | perry (after) | + |---|---|---|---| + | `/^(a+)+$/.test("a"×28 + "!")` | 4,798 ms | **16,522 ms** | **0 ms** | + | `/^(?:a+)+$/.test(…)` (same language, no capture) | 4,288 ms | 0 ms | 0 ms | + + **6.3 %** of the 4,463 distinct regex literals across seven real bundles + take that route — claude-code 7.1 %, dayjs 25 %, luxon 29 % — including + shapes like `^[a-z][a-z0-9]*(-[a-z0-9]+)*$`. + + The two engines accept exactly the same LANGUAGE for a pattern they both + compile; they disagree only about which capture assignment to report. So the + linear program is asked first (`linear_rules_out_match`), and when it proves + there is no match at or after the search offset — which is what every ReDoS + input is, a subject that ALMOST matches and then fails — the backtracker is + never entered. Every `&str`-subject entry point goes through + `lookup_repeat_matcher_for`: `test`, `exec`, `match`, `matchAll`, `search`, + `split` and `replace` with a string replacement. The gate disables itself + where the linear engine has no opinion (a pattern it could not compile holds + the never-match placeholder), which is exactly the lookaround shapes. + + **This removes the reachable exponential case; it does not BOUND the worst + case.** A real step budget has to be counted by the backtracker, and + `regress` has none today (`fancy-regex`, by contrast, ships + `backtrack_limit: 1_000_000`). A 101-line patch adding one has been measured + — worst hostile search 51 s → 124 ms at a budget of 1,000,000, zero answers + changed across 13,389 real searches, upstream's own 544 tests unchanged — and + is open upstream as + [ridiculousfish/regress#177](https://github.com/ridiculousfish/regress/pull/177). + Until it lands and perry picks it up, do not read "cliff fixed" as "worst + case bounded". + (`quantified_capture_pattern_does_not_backtrack_on_a_non_matching_subject`) diff --git a/changelog.d/regex-borrowed-cache-keys.md b/changelog.d/regex-borrowed-cache-keys.md new file mode 100644 index 0000000000..76d06c09fa --- /dev/null +++ b/changelog.d/regex-borrowed-cache-keys.md @@ -0,0 +1,26 @@ +### Performance + +- **Probing the compiled-program caches no longer materialises the key.** The + three thread-local caches were `HashMap<(String, String), _>`, and + `HashMap::get` needs a `&(String, String)` — so **every probe allocated two + Strings and copied the pattern text into them**, on a path that runs once per + RegExp OBJECT, and a JS regex literal evaluates to a fresh object every time + it is reached. A native-churn census of the claude-code binary (2026-09-05) + put `js_regexp_test` → `lookup_repeat_matcher` → `build_and_install_programs` + at **6,044 MB of 8,334 MB of estimated allocation with zero live bytes** — + 73 % of all remaining native churn — split across the three probe sites: the + `get_or_compile_regex` probe (2,071 MB) and two `core::fmt::Formatter::pad` + frames (1,989 MB and 1,984 MB), which is what `.to_string()` on an `Arc` + lowers to. + + The caches are now keyed by `ProgramKey = (Arc, Arc)`. Every caller + that matters already holds those `Arc`s — `REGEX_SOURCE_TABLE` and + `regex::site_cache` share one allocation of a literal's text with every + header built from it — so a probe is two refcount increments and no + allocation at all. The two remaining `Arc::from` materialisations are on cold + paths: the syntax-error fallback in `js_regexp_new` (a pattern the linear + engine's parser refused, 7.7 % of real literals, once each) and + `RegExp.prototype.compile` (once per call from user code). + + Hashing still walks the pattern bytes; the allocation is what the census + measured and what this removes. diff --git a/changelog.d/regex-engine-prototype-switch.md b/changelog.d/regex-engine-prototype-switch.md new file mode 100644 index 0000000000..2d3e4f8a68 --- /dev/null +++ b/changelog.d/regex-engine-prototype-switch.md @@ -0,0 +1,31 @@ +### Internal + +- **`PERRY_REGEX_ENGINE=regress` — a measurable tier-0 engine prototype.** + Routes every pattern through `regress` (the ECMAScript backtracker perry + already links for RepeatMatcher capture semantics) instead of only the ones + whose capture semantics require it, and installs a shared never-match + placeholder as the standard program so no NFA is built. Every exec-family + entry point already consults the repeat matcher first, so this exercises the + whole engine surface — `exec`, `test`, `match`, `matchAll`, `search`, + `split`, `replace` — without a second implementation. + + It exists so the engine question is settled on measurements from a real + binary rather than on a corpus harness. Measured over 4,463 distinct regex + literals extracted from seven real bundles (two claude-code builds, ethers, + moment, dayjs, luxon, mongodb) with a tracking allocator and the programs + held live: + + | engine | accepted | compile µs (med) | bytes/program (med) | corpus total | + |---|---|---|---|---| + | `regex` crate (tier 1 today) | 92.3 % | 48.5 | 12,492 | 136.7 MB | + | `regress` | **100 %** | **2.2** | **512** | **4.9 MB** | + | `fancy-regex` (tier 2 today) | 97.8 % | 59.2 | 12,623 | 146.6 MB | + + node/V8, measured the same session, is ~2,600 bytes per program. A + differential over 4,119 patterns × 13 subjects (53,547 comparisons of match + presence, span and every capture span) found **0 disagreements** between the + linear engine and `regress`. + + **Not a supported configuration**: the backtracker has no step budget, so a + pathological pattern can run unbounded. Off by default, one relaxed atomic + load when unset. diff --git a/crates/perry-runtime/src/regex.rs b/crates/perry-runtime/src/regex.rs index b6787e3c51..fd25200d16 100644 --- a/crates/perry-runtime/src/regex.rs +++ b/crates/perry-runtime/src/regex.rs @@ -365,14 +365,14 @@ pub(crate) unsafe fn regex_gc_slot_ptrs(re: *mut RegExpHeader) -> (*mut u64, usi #[cfg(feature = "regex-engine")] crate::perry_thread_local! { /// Cache of compiled regex objects, keyed by (pattern, flags). - static REGEX_CACHE: RefCell>> = RefCell::new(HashMap::new()); + static REGEX_CACHE: RefCell>> = RefCell::new(HashMap::new()); /// Fancy-regex fallback cache for patterns with lookbehind/lookahead. - static FANCY_CACHE: RefCell>> = RefCell::new(HashMap::new()); + static FANCY_CACHE: RefCell>> = RefCell::new(HashMap::new()); /// ECMAScript backtracking matchers for quantified capture groups. These /// are the patterns where `regex`/`fancy-regex` cannot reproduce /// `RepeatMatcher` capture reset and nullable-iteration semantics (#5897). - static REPEAT_MATCHER_CACHE: RefCell>> = RefCell::new(HashMap::new()); + static REPEAT_MATCHER_CACHE: RefCell>> = RefCell::new(HashMap::new()); /// `(pattern, flags)` pairs that have already cleared construction-time /// validation. Validity is a pure function of the pair, so the answer is @@ -475,6 +475,28 @@ pub(crate) fn build_fancy_regex(pattern: &str) -> Result` lowers to. +/// +/// Keying by `Arc` makes a probe two refcount increments and no +/// allocation: every caller that matters already holds those `Arc`s, because +/// `REGEX_SOURCE_TABLE` and `regex::site_cache` share one allocation of a +/// literal's text with every header built from it. Hashing still walks the +/// pattern bytes — the allocation is what the census measured, and what this +/// removes. +#[cfg(feature = "regex-engine")] +pub(crate) type ProgramKey = (Arc, Arc); /// Entry cap for the compiled-regex caches (2026-07-09 GC audit: one entry /// per distinct `(pattern, flags)` ever compiled, no cap of any kind, entries @@ -491,7 +513,7 @@ const REGEX_CACHE_MAX_ENTRIES: usize = 512; /// validated-pattern set: make room for one more entry, wiping the map when it /// is at capacity. #[cfg(feature = "regex-engine")] -fn evict_regex_cache_if_full(cache: &mut HashMap<(String, String), V>) { +fn evict_regex_cache_if_full(cache: &mut HashMap) { if cache.len() >= REGEX_CACHE_MAX_ENTRIES { cache.clear(); if crate::hot_diag::regex_on() { @@ -518,28 +540,65 @@ fn evict_regex_cache_if_full(cache: &mut HashMap<(String, String), V>) { /// Returns `false` when BOTH engines reject it — nothing is cached and the /// caller decides whether that is a SyntaxError (see `js_regexp_new`'s /// bare-pattern fallback for the flag-prefix size edge). +/// One shared never-match program per thread. +/// +/// Only used by the `PERRY_REGEX_ENGINE=regress` measurement path, where every +/// pattern needs a value in `regex_ptr` (the built/not-built flag) but no NFA: +/// building a fresh one per pattern would be exactly the compile cost the +/// experiment exists to remove from the measurement. +#[cfg(feature = "regex-engine")] +fn shared_never_match_program() -> Arc { + crate::perry_thread_local! { + static NEVER_MATCH: RefCell>> = const { RefCell::new(None) }; + } + NEVER_MATCH.with(|slot| { + slot.borrow_mut() + .get_or_insert_with(|| Arc::new(Regex::new(NEVER_MATCH_PATTERN).unwrap())) + .clone() + }) +} + #[cfg(feature = "regex-engine")] -fn compile_and_cache_regex_checked(pattern: &str, flags: &str) -> bool { +fn compile_and_cache_regex_checked(pattern: &Arc, flags: &Arc) -> bool { let already = REGEX_CACHE.with(|cache| { cache .borrow() - .contains_key(&(pattern.to_string(), flags.to_string())) + .contains_key(&(pattern.clone(), flags.clone())) }); if already { return true; } - if let Some(repeat_matcher) = repeat_matcher::compile(pattern, flags) { + let regress_covers = if let Some(repeat_matcher) = repeat_matcher::compile(pattern, flags) { if crate::hot_diag::regex_on() { crate::hot_diag::regex_with(|d| d.compiles_repeat += 1); } REPEAT_MATCHER_CACHE.with(|cache| { + let mut cache = cache.borrow_mut(); + evict_regex_cache_if_full(&mut cache); + cache.insert((pattern.clone(), flags.clone()), Arc::new(repeat_matcher)); + }); + true + } else { + false + }; + // `PERRY_REGEX_ENGINE=regress` (measurement only — see + // `repeat_matcher::regress_first`): the ECMAScript backtracker is the + // primary engine, so stop here. Every exec-family entry point consults the + // repeat matcher first, and the shared never-match placeholder gives the + // header's `regex_ptr` built-flag a value WITHOUT building an NFA — which + // is the whole point of the experiment (the linear engine's program is + // ~12.5 KB median against regress's 512 B, measured over 4,463 literals + // from seven real bundles). + if regress_covers && repeat_matcher::regress_first() { + REGEX_CACHE.with(|cache| { let mut cache = cache.borrow_mut(); evict_regex_cache_if_full(&mut cache); cache.insert( - (pattern.to_string(), flags.to_string()), - Arc::new(repeat_matcher), + (pattern.clone(), flags.clone()), + shared_never_match_program(), ); }); + return true; } // Translate JS regex to Rust-compatible pattern, with the inline mode // prefix the flags imply. Shared with `lazy::std_engine_syntax_ok` so the @@ -561,10 +620,7 @@ fn compile_and_cache_regex_checked(pattern: &str, flags: &str) -> bool { } let mut fc = fc.borrow_mut(); evict_regex_cache_if_full(&mut fc); - fc.insert( - (pattern.to_string(), flags.to_string()), - std::sync::Arc::new(fre), - ); + fc.insert((pattern.clone(), flags.clone()), std::sync::Arc::new(fre)); true } else { false @@ -582,17 +638,17 @@ fn compile_and_cache_regex_checked(pattern: &str, flags: &str) -> bool { REGEX_CACHE.with(|cache| { let mut cache = cache.borrow_mut(); evict_regex_cache_if_full(&mut cache); - cache.insert((pattern.to_string(), flags.to_string()), Arc::new(regex)); + cache.insert((pattern.clone(), flags.clone()), Arc::new(regex)); }); true } #[cfg(feature = "regex-engine")] -fn get_or_compile_regex(pattern: &str, flags: &str) -> Arc { +fn get_or_compile_regex(pattern: &Arc, flags: &Arc) -> Arc { let hit = REGEX_CACHE.with(|cache| { cache .borrow() - .get(&(pattern.to_string(), flags.to_string())) + .get(&(pattern.clone(), flags.clone())) .cloned() }); if let Some(re) = hit { @@ -601,14 +657,14 @@ fn get_or_compile_regex(pattern: &str, flags: &str) -> Arc { let _ = compile_and_cache_regex_checked(pattern, flags); REGEX_CACHE.with(|cache| { let mut cache = cache.borrow_mut(); - if let Some(re) = cache.get(&(pattern.to_string(), flags.to_string())) { + if let Some(re) = cache.get(&(pattern.clone(), flags.clone())) { return re.clone(); } // Both engines rejected it (validation normally throws before this // point) — keep the historical behavior: cache + return never-match. let arc = Arc::new(Regex::new(NEVER_MATCH_PATTERN).unwrap()); evict_regex_cache_if_full(&mut cache); - cache.insert((pattern.to_string(), flags.to_string()), arc.clone()); + cache.insert((pattern.clone(), flags.clone()), arc.clone()); arc }) } @@ -943,7 +999,14 @@ pub extern "C" fn js_regexp_new( // SyntaxError decision and populates the caches for the fancy // fallback. if !lazy::std_engine_syntax_ok(pattern_str, flags_str) - && !compile_and_cache_regex_checked(pattern_str, flags_str) + // Cold: the linear engine's parser refused, so only a BUILD + // can tell a fancy-regex pattern from a SyntaxError. + // Materialising the `Arc` key happens once per distinct + // pattern that needs the fallback, not per object. + && !compile_and_cache_regex_checked( + &Arc::from(pattern_str), + &Arc::from(flags_str), + ) { // Preserve the historical edge: validation used to test the // BARE translated pattern (no `(?ims)` prefix). A pattern that @@ -1278,7 +1341,7 @@ pub extern "C" fn js_regexp_test(re: *const RegExpHeader, s: *const StringHeader }; } - if let Some(repeat_matcher) = lookup_repeat_matcher(re) { + if let Some(repeat_matcher) = lookup_repeat_matcher_for(re, str_data, 0) { return if repeat_matcher.regex.find(str_data).is_some() { 1 } else { @@ -1359,12 +1422,81 @@ pub(crate) fn lookup_fancy_regex(re: *const RegExpHeader) -> Option bool { + unsafe { + let program = (*re).regex_ptr; + if program.is_null() { + return false; + } + let program: &Regex = &*program; + if program.as_str() == NEVER_MATCH_PATTERN { + // The `regex` crate refused this pattern (lookaround / + // backreference); it has no opinion about the subject. + return false; + } + start <= subject.len() && !program.is_match_at(subject, start) + } +} + + +/// [`lookup_repeat_matcher`] with the linear pre-check applied: `None` also +/// when the linear program proves no match at or after `start`, so the +/// backtracker is never entered on a subject that cannot match. Every +/// `&str`-subject call site uses this; the WTF-8/UTF-16 replace path, which has +/// no `&str` to hand, uses the bare lookup. +#[cfg(feature = "regex-engine")] +fn lookup_repeat_matcher_for( + re: *const RegExpHeader, + subject: &str, + start: usize, +) -> Option> { + let matcher = lookup_repeat_matcher(re)?; + if linear_rules_out_match(re, subject, start) { + return None; + } + Some(matcher) +} + /// Look up the ECMAScript-native matcher used when quantified capture groups /// make `RepeatMatcher`'s capture-reset semantics observable. #[cfg(feature = "regex-engine")] @@ -1393,7 +1525,7 @@ fn lookup_repeat_matcher( REPEAT_MATCHER_CACHE.with(|cache| { cache .borrow() - .get(&(pat.to_string(), flags_str.to_string())) + .get(&(Arc::from(pat), Arc::from(flags_str))) .cloned() }) } @@ -1597,7 +1729,7 @@ pub extern "C" fn js_string_replace_regex( "undefined" }; - if let Some(repeat_matcher) = lookup_repeat_matcher(re) { + if let Some(repeat_matcher) = lookup_repeat_matcher_for(re, str_data, 0) { let result = repeat_matcher.replace(str_data, repl_str, (*re).global); return finish_replace_bytes(result.as_bytes()); } @@ -1720,18 +1852,19 @@ pub extern "C" fn js_string_split_regex_n( unsafe { // Each element is either a substring (`Some`) or `undefined` (`None`, // for an unmatched capture group spliced into the result). - let parts: Vec> = if let Some(repeat_matcher) = lookup_repeat_matcher(re) { - repeat_matcher.split(&str_data, limit) - } else if let Some(fre) = lookup_fancy_regex(re) { - crate::string::spec_fancy_regex_split(&fre, &str_data, limit) - } else { - // Standard engine: the JS `RegExp.prototype[Symbol.split]` algorithm - // (21.2.5.11). The `regex` crate's own `split` diverges from JS for - // zero-width matches (it emits leading/trailing/consecutive empty - // strings the spec's `e == p` skip suppresses) and never splices - // captured groups, so walk the string the spec's way instead. - crate::string::spec_regex_split(lazy::header_std_regex(re), &str_data, limit) - }; + let parts: Vec> = + if let Some(repeat_matcher) = lookup_repeat_matcher_for(re, &str_data, 0) { + repeat_matcher.split(&str_data, limit) + } else if let Some(fre) = lookup_fancy_regex(re) { + crate::string::spec_fancy_regex_split(&fre, &str_data, limit) + } else { + // Standard engine: the JS `RegExp.prototype[Symbol.split]` algorithm + // (21.2.5.11). The `regex` crate's own `split` diverges from JS for + // zero-width matches (it emits leading/trailing/consecutive empty + // strings the spec's `e == p` skip suppresses) and never splices + // captured groups, so walk the string the spec's way instead. + crate::string::spec_regex_split(lazy::header_std_regex(re), &str_data, limit) + }; let arr = crate::array::js_array_alloc(parts.len() as u32); let scope = crate::gc::RuntimeHandleScope::new(); @@ -1765,7 +1898,7 @@ pub extern "C" fn js_string_search_regex(s: *const StringHeader, re: *const RegE let str_data = string_as_str(s); unsafe { - if let Some(repeat_matcher) = lookup_repeat_matcher(re) { + if let Some(repeat_matcher) = lookup_repeat_matcher_for(re, str_data, 0) { return repeat_matcher .regex .find(str_data) diff --git a/crates/perry-runtime/src/regex/compile.rs b/crates/perry-runtime/src/regex/compile.rs index 6105ebc8da..f8472f50d6 100644 --- a/crates/perry-runtime/src/regex/compile.rs +++ b/crates/perry-runtime/src/regex/compile.rs @@ -150,13 +150,16 @@ pub extern "C" fn js_regexp_compile_value( // (see `REGEX_CACHE_MAX_ENTRIES`) can evict without invalidating this // receiver. Refresh `fancy_ptr` too — it must track the NEW pattern, not // the one the receiver was constructed with. - let arc = get_or_compile_regex(pattern_str, flags_str); + // `RegExp.prototype.compile` re-initialises an existing receiver — once per + // call from user code, not per object — so materialising the shared key + // here costs nothing measurable, and the same `Arc`s go into the source + // table below. + let pattern_key: std::sync::Arc = std::sync::Arc::from(pattern_str); + let flags_key: std::sync::Arc = std::sync::Arc::from(flags_str); + let arc = get_or_compile_regex(&pattern_key, &flags_key); let regex_ptr = Arc::into_raw(arc) as *mut Regex; let fancy_ptr: *const () = super::FANCY_CACHE.with(|fc| { - match fc - .borrow() - .get(&(pattern_str.to_string(), flags_str.to_string())) - { + match fc.borrow().get(&(pattern_key.clone(), flags_key.clone())) { Some(arc) => Arc::into_raw(arc.clone()) as *const (), None => std::ptr::null(), } @@ -164,7 +167,7 @@ pub extern "C" fn js_regexp_compile_value( let repeat_matcher_ptr: *const () = super::REPEAT_MATCHER_CACHE.with(|cache| { match cache .borrow() - .get(&(pattern_str.to_string(), flags_str.to_string())) + .get(&(pattern_key.clone(), flags_key.clone())) { Some(arc) => Arc::into_raw(arc.clone()) as *const (), None => std::ptr::null(), diff --git a/crates/perry-runtime/src/regex/exec.rs b/crates/perry-runtime/src/regex/exec.rs index dd09e6e44f..f0d760a39b 100644 --- a/crates/perry-runtime/src/regex/exec.rs +++ b/crates/perry-runtime/src/regex/exec.rs @@ -78,7 +78,9 @@ pub extern "C" fn js_regexp_exec( // `fancy_regex::Regex::captures_from_pos` and // `regress::Regex::find_from`. Their reported offsets are absolute, so // nothing downstream re-bases them. - let owned = if let Some(repeat_matcher) = lookup_repeat_matcher(re) { + let owned = if let Some(repeat_matcher) = + lookup_repeat_matcher_for(re, str_data, search_start_byte) + { repeat_matcher .regex .find_from(str_data, search_start_byte) @@ -212,7 +214,9 @@ pub(super) fn regexp_find_advancing( } else { 0 }; - let found = if let Some(repeat_matcher) = lookup_repeat_matcher(re) { + let found = if let Some(repeat_matcher) = + lookup_repeat_matcher_for(re, str_data, search_start_byte) + { repeat_matcher .regex .find_from(str_data, search_start_byte) diff --git a/crates/perry-runtime/src/regex/lazy.rs b/crates/perry-runtime/src/regex/lazy.rs index 236c7a80f4..d5d2ca4f11 100644 --- a/crates/perry-runtime/src/regex/lazy.rs +++ b/crates/perry-runtime/src/regex/lazy.rs @@ -233,7 +233,7 @@ fn build_and_install_programs(re: *const RegExpHeader) { let cache_hit = super::REGEX_CACHE.with(|cache| { cache .borrow() - .contains_key(&(pattern.to_string(), flags.to_string())) + .contains_key(&(pattern.clone(), flags.clone())) }); unsafe { let pattern_ptr = (*re).pattern_ptr; @@ -243,16 +243,13 @@ fn build_and_install_programs(re: *const RegExpHeader) { } } let std_arc = get_or_compile_regex(&pattern, &flags); - let fancy_arc: Option> = FANCY_CACHE.with(|fc| { - fc.borrow() - .get(&(pattern.to_string(), flags.to_string())) - .cloned() - }); + let fancy_arc: Option> = + FANCY_CACHE.with(|fc| fc.borrow().get(&(pattern.clone(), flags.clone())).cloned()); let repeat_arc: Option> = REPEAT_MATCHER_CACHE .with(|cache| { cache .borrow() - .get(&(pattern.to_string(), flags.to_string())) + .get(&(pattern.clone(), flags.clone())) .cloned() }); // ── Repair before publishing ────────────────────────────────────────── diff --git a/crates/perry-runtime/src/regex/match_all.rs b/crates/perry-runtime/src/regex/match_all.rs index 202e744609..998e2ad83f 100644 --- a/crates/perry-runtime/src/regex/match_all.rs +++ b/crates/perry-runtime/src/regex/match_all.rs @@ -97,7 +97,7 @@ unsafe fn materialize_match_all_results( let search_start = utf16_index_to_byte(str_data, start_char_index); let mut owned: Vec = Vec::new(); - if let Some(repeat_matcher) = super::lookup_repeat_matcher(re) { + if let Some(repeat_matcher) = super::lookup_repeat_matcher_for(re, str_data, search_start) { // `regress`'s own iterator is positional and already advances one // position past a zero-width match, which is the ECMAScript rule. for matched in repeat_matcher.regex.find_from(str_data, search_start) { diff --git a/crates/perry-runtime/src/regex/match_string.rs b/crates/perry-runtime/src/regex/match_string.rs index 6611923493..df4e49ad05 100644 --- a/crates/perry-runtime/src/regex/match_string.rs +++ b/crates/perry-runtime/src/regex/match_string.rs @@ -85,7 +85,7 @@ pub extern "C" fn js_string_match( let global = (*re).global; let has_indices = (*re).has_indices; - if let Some(repeat_matcher) = lookup_repeat_matcher(re) { + if let Some(repeat_matcher) = lookup_repeat_matcher_for(re, str_data, 0) { if global { let matches: Vec = repeat_matcher .regex diff --git a/crates/perry-runtime/src/regex/repeat_matcher.rs b/crates/perry-runtime/src/regex/repeat_matcher.rs index e2e19129c9..da008816de 100644 --- a/crates/perry-runtime/src/regex/repeat_matcher.rs +++ b/crates/perry-runtime/src/regex/repeat_matcher.rs @@ -241,7 +241,35 @@ fn quantifier_follows(bytes: &[u8], index: usize) -> bool { /// capture semantics. Besides quantified captures, this includes captures in a /// negative lookaround: after a successful negative assertion those captures /// are unmatched, so a later backreference must match the empty string. -fn quantified_capture_layout(pattern: &str) -> Option>> { +/// Is `regress` the PRIMARY engine for this process? +/// +/// `PERRY_REGEX_ENGINE=regress` routes every pattern through the ECMAScript +/// backtracker instead of only the ones whose RepeatMatcher capture semantics +/// are observable. It exists to MEASURE the tier-0 engine architecture end to +/// end — compile cost, bytes of program, and the match-time cost of giving up +/// the linear engine — in a real binary on the real rig rather than only in a +/// corpus harness. It is NOT a supported configuration: the backtracker has no +/// step budget, so a pathological pattern can run unbounded. +pub(super) fn regress_first() -> bool { + use std::sync::atomic::{AtomicU8, Ordering}; + static STATE: AtomicU8 = AtomicU8::new(0); + match STATE.load(Ordering::Relaxed) { + 1 => return false, + 2 => return true, + _ => {} + } + let on = std::env::var("PERRY_REGEX_ENGINE") + .map(|v| v.eq_ignore_ascii_case("regress")) + .unwrap_or(false); + STATE.store(if on { 2 } else { 1 }, Ordering::Relaxed); + on +} + +/// The capture-name layout of `pattern`, and whether ECMA-262's RepeatMatcher +/// capture-reset semantics are OBSERVABLE for it (a capture group directly +/// under a quantifier, or a capture inside a negative lookaround — the two +/// shapes where the linear engine's answer differs from the spec's). +fn capture_layout(pattern: &str) -> (Vec>, bool) { let bytes = pattern.as_bytes(); let mut captures = Vec::new(); let mut groups = Vec::new(); @@ -288,11 +316,14 @@ fn quantified_capture_layout(pattern: &str) -> Option>> { _ => index += 1, } } - needs_repeat_matcher.then_some(captures) + (captures, needs_repeat_matcher) } pub(super) fn compile(pattern: &str, flags: &str) -> Option { - let capture_names = quantified_capture_layout(pattern)?; + let (capture_names, needs_repeat_matcher) = capture_layout(pattern); + if !needs_repeat_matcher && !regress_first() { + return None; + } let regex = regress::Regex::with_flags(pattern, flags).ok()?; Some(RepeatMatcherRegex { regex, @@ -463,20 +494,20 @@ mod tests { #[test] fn detects_only_quantified_groups_with_captures() { - assert!(quantified_capture_layout(r"(a?b??)*").is_some()); - assert!(quantified_capture_layout(r"(?:(?=(abc))){0,1}a").is_some()); - assert!(quantified_capture_layout(r"(?!(a)b)\1").is_some()); - assert!(quantified_capture_layout(r"(?a)(b))*"), - Some(vec![Some("first".to_string()), None]) + capture_layout(r"(?:(?a)(b))*"), + (vec![Some("first".to_string()), None], true) ); } } diff --git a/crates/perry-runtime/src/regex/replace_expand.rs b/crates/perry-runtime/src/regex/replace_expand.rs index 4b87e5a0fd..856eed6e51 100644 --- a/crates/perry-runtime/src/regex/replace_expand.rs +++ b/crates/perry-runtime/src/regex/replace_expand.rs @@ -477,7 +477,7 @@ pub extern "C" fn js_string_replace_regex_named( } unsafe { - if let Some(repeat_matcher) = lookup_repeat_matcher(re) { + if let Some(repeat_matcher) = lookup_repeat_matcher_for(re, str_data, 0) { let result = repeat_matcher.replace(str_data, repl_str, (*re).global); return finish_replace_bytes(result.as_bytes()); } diff --git a/crates/perry-runtime/src/regex/tests.rs b/crates/perry-runtime/src/regex/tests.rs index d82dea78e9..e5a8c95349 100644 --- a/crates/perry-runtime/src/regex/tests.rs +++ b/crates/perry-runtime/src/regex/tests.rs @@ -905,7 +905,10 @@ fn regex_cache_capped_and_prior_headers_survive_eviction() { // Flood the cache with distinct patterns — far past the cap. for i in 0..(REGEX_CACHE_MAX_ENTRIES * 2 + 10) { - let _ = get_or_compile_regex(&format!("cachefill{i}[a-z]+"), ""); + let _ = get_or_compile_regex( + &Arc::from(format!("cachefill{i}[a-z]+").as_str()), + &Arc::from(""), + ); } let std_len = REGEX_CACHE.with(|c| c.borrow().len()); assert!( @@ -915,7 +918,10 @@ fn regex_cache_capped_and_prior_headers_survive_eviction() { // Flood the fancy cache as well (each pattern rejected by the std engine). for i in 0..(REGEX_CACHE_MAX_ENTRIES + 10) { - let _ = get_or_compile_regex(&format!("(?<=fill{i})x"), ""); + let _ = get_or_compile_regex( + &Arc::from(format!("(?<=fill{i})x").as_str()), + &Arc::from(""), + ); } let fancy_len = FANCY_CACHE.with(|c| c.borrow().len()); assert!( @@ -925,7 +931,7 @@ fn regex_cache_capped_and_prior_headers_survive_eviction() { // Quantified captures populate the ECMAScript RepeatMatcher cache. for i in 0..(REGEX_CACHE_MAX_ENTRIES + 10) { - let _ = get_or_compile_regex(&format!("(repeat{i})*"), ""); + let _ = get_or_compile_regex(&Arc::from(format!("(repeat{i})*").as_str()), &Arc::from("")); } let repeat_len = REPEAT_MATCHER_CACHE.with(|c| c.borrow().len()); assert!( @@ -1830,3 +1836,51 @@ fn a_single_program_cache_clear_cannot_disarm_a_lookbehind_literal() { "the literal must still match after an unrelated cache reached capacity" ); } + +/// The backtracking cliff: a capture group under a quantifier takes a pattern +/// off the linear engine, and the ECMAScript backtracker has no step budget. +/// `/^(a+)+$/.test("a"*28 + "!")` measured 16.5 s against 4.8 s for node and +/// 0 ms for the identical-language `/^(?:a+)+$/`. +/// +/// The linear program proves the answer in O(n) — the two engines accept the +/// same language and disagree only about capture ASSIGNMENT — so the +/// backtracker must not be entered for a subject the linear engine has already +/// ruled out. This test would take minutes without that gate. +#[test] +fn quantified_capture_pattern_does_not_backtrack_on_a_non_matching_subject() { + let _lock = crate::gc::global_side_table_test_lock(); + let scope = crate::gc::RuntimeHandleScope::new(); + let pattern = scope.root_string_ptr(make_string("^(a+)+$")); + let flags = scope.root_string_ptr(make_string("")); + let re = pattern.with_mut_ptr::(|pattern| { + flags.with_mut_ptr::(|flags| js_regexp_new(pattern, flags)) + }); + // The pattern really is on the backtracker — that is the premise. + assert!( + lookup_repeat_matcher(re).is_some(), + "a capture under a quantifier must route to the ECMAScript matcher" + ); + + let hay = format!("{}!", "a".repeat(40)); + let subject = scope.root_string_ptr(make_string(&hay)); + let started = std::time::Instant::now(); + assert_eq!( + subject.with_const_ptr::(|s| js_regexp_test(re, s)), + 0, + "no match: the subject ends in '!'" + ); + assert!( + started.elapsed() < std::time::Duration::from_secs(2), + "a non-matching subject must not be handed to the backtracker \ + (took {:?} for 40 characters)", + started.elapsed() + ); + + // A subject that DOES match still goes through the backtracker and still + // reports the spec's captures. + let good = scope.root_string_ptr(make_string("aaaa")); + assert_eq!( + good.with_const_ptr::(|s| js_regexp_test(re, s)), + 1 + ); +} From bddf7f00e8d03642727277a4dc379cffcffa6a6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 16:29:38 +0200 Subject: [PATCH 2/5] docs(changelog): key the three regex fragments to PR 9796 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit changelog.d/README.md asks for `-.md`; the three fragments landed unnumbered. Renames only — no entry text and no code changes, so the measured candidate binary is unaffected. Claude-Session: https://claude.ai/code/session_014UZWia6L37DpA93VLtNK9m --- ...gex-backtracking-cliff.md => 9796-regex-backtracking-cliff.md} | 0 ...x-borrowed-cache-keys.md => 9796-regex-borrowed-cache-keys.md} | 0 ...-prototype-switch.md => 9796-regex-engine-prototype-switch.md} | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{regex-backtracking-cliff.md => 9796-regex-backtracking-cliff.md} (100%) rename changelog.d/{regex-borrowed-cache-keys.md => 9796-regex-borrowed-cache-keys.md} (100%) rename changelog.d/{regex-engine-prototype-switch.md => 9796-regex-engine-prototype-switch.md} (100%) diff --git a/changelog.d/regex-backtracking-cliff.md b/changelog.d/9796-regex-backtracking-cliff.md similarity index 100% rename from changelog.d/regex-backtracking-cliff.md rename to changelog.d/9796-regex-backtracking-cliff.md diff --git a/changelog.d/regex-borrowed-cache-keys.md b/changelog.d/9796-regex-borrowed-cache-keys.md similarity index 100% rename from changelog.d/regex-borrowed-cache-keys.md rename to changelog.d/9796-regex-borrowed-cache-keys.md diff --git a/changelog.d/regex-engine-prototype-switch.md b/changelog.d/9796-regex-engine-prototype-switch.md similarity index 100% rename from changelog.d/regex-engine-prototype-switch.md rename to changelog.d/9796-regex-engine-prototype-switch.md From 57f5c0b4140271611536821f989d840c3815ec33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 19:27:08 +0200 Subject: [PATCH 3/5] fix(regex): reconcile #9801's repair block with the borrowed cache keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rebase onto `fbce42de6` (#9801) auto-merged `lazy.rs` without a conflict, and the result did not compile: #9801's "repair before publishing" block inserts into `FANCY_CACHE` and `REPEAT_MATCHER_CACHE`, whose key this branch changed from `(String, String)` to `ProgramKey = (Arc, Arc)`. One side added a writer, the other side changed the type those writers use, and git had nothing to complain about — the same shape as the `family_append_fresh` hazard, caught here only because the change is visible to the type checker. Both values are already `Arc` in that scope, so the repair path now clones two refcounts instead of copying the pattern text twice. Also drops this branch's `NEVER_MATCH_SOURCE`: #9801 landed the identical constant as `NEVER_MATCH_PATTERN`, documented for the same reason, and `linear_rules_out_match` now uses main's. Claude-Session: https://claude.ai/code/session_014UZWia6L37DpA93VLtNK9m --- crates/perry-runtime/src/regex/lazy.rs | 4 ++-- crates/perry-runtime/src/regex/tests.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/perry-runtime/src/regex/lazy.rs b/crates/perry-runtime/src/regex/lazy.rs index d5d2ca4f11..a982a34875 100644 --- a/crates/perry-runtime/src/regex/lazy.rs +++ b/crates/perry-runtime/src/regex/lazy.rs @@ -283,7 +283,7 @@ fn build_and_install_programs(re: *const RegExpHeader) { FANCY_CACHE.with(|fc| { let mut fc = fc.borrow_mut(); evict_regex_cache_if_full(&mut fc); - fc.insert((pattern.to_string(), flags.to_string()), arc.clone()); + fc.insert((pattern.clone(), flags.clone()), arc.clone()); }); fancy_arc = Some(arc); } @@ -298,7 +298,7 @@ fn build_and_install_programs(re: *const RegExpHeader) { REPEAT_MATCHER_CACHE.with(|cache| { let mut cache = cache.borrow_mut(); evict_regex_cache_if_full(&mut cache); - cache.insert((pattern.to_string(), flags.to_string()), arc.clone()); + cache.insert((pattern.clone(), flags.clone()), arc.clone()); }); repeat_arc = Some(arc); } diff --git a/crates/perry-runtime/src/regex/tests.rs b/crates/perry-runtime/src/regex/tests.rs index e5a8c95349..2c571153c4 100644 --- a/crates/perry-runtime/src/regex/tests.rs +++ b/crates/perry-runtime/src/regex/tests.rs @@ -1813,7 +1813,7 @@ fn a_single_program_cache_clear_cannot_disarm_a_lookbehind_literal() { assert!( REGEX_CACHE.with(|c| c .borrow() - .contains_key(&(source.to_string(), String::new()))), + .contains_key(&(std::sync::Arc::from(source), std::sync::Arc::from("")))), "the placeholder must survive, or this test exercises nothing" ); // A fresh literal site, so the construction cache cannot answer from the From 5f172649bffb218aaf34dd6626e54407b2acc3db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 18:32:38 +0200 Subject: [PATCH 4/5] perf(regex): construct a RegExp without allocating its flags string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `js_regexp_new` materialized the canonical flags twice on every construction — a Rust `String` from `validate_and_canonicalize_flags`, and a fresh GC `StringHeader` for `flags_ptr` — and a JS regex literal constructs a fresh object every time it is evaluated. `PERRY_REGEX_DIAG` counts 161,897 constructions per 400-character claude-code reply: ~5.2 MB of identical one- and two-byte GC strings, ~44 MB on a 3300-character reply, ~1.4 M allocations. There are eight legal flags and each may appear once, so the canonical form is at most eight ASCII bytes and now lives inline in `CanonicalFlags`. JS strings are immutable and have no identity semantics, so when the caller's flags text already IS the canonical text — a literal, whose flags the author wrote in spec order — the header shares the caller's string instead of duplicating it. Nothing downstream depends on the pointer being fresh: `flags_ptr`-keyed lookups read it through `string_as_str` and compare content. GC safety: the comparison and the root are taken BEFORE the validation block, because `raw_flags_str` borrows the caller's GC string and that block can allocate — the same hazard the ★ note on `pattern_root` describes, and the same one #7341 fixed for the freshly-allocated flags string. The existing re-read from `flags_root` after `gc_malloc` covers both arms unchanged. Below the campaign's ~10 % line at ~2-3 % of arena traffic per turn, so the cc rig is expected to read flat; the counter is the proof, not the benchmark. Claude-Session: https://claude.ai/code/session_014UZWia6L37DpA93VLtNK9m --- changelog.d/9812-regex-flags-no-alloc.md | 25 ++++++++++++ crates/perry-runtime/src/hot_diag.rs | 9 ++++- crates/perry-runtime/src/regex.rs | 48 +++++++++++++++++++++--- crates/perry-runtime/src/regex/flags.rs | 40 ++++++++++++++++---- 4 files changed, 108 insertions(+), 14 deletions(-) create mode 100644 changelog.d/9812-regex-flags-no-alloc.md diff --git a/changelog.d/9812-regex-flags-no-alloc.md b/changelog.d/9812-regex-flags-no-alloc.md new file mode 100644 index 0000000000..431d87e484 --- /dev/null +++ b/changelog.d/9812-regex-flags-no-alloc.md @@ -0,0 +1,25 @@ +### Performance + +- **Constructing a `RegExp` no longer allocates its flags string.** A JS regex + literal evaluates to a fresh `RegExp` object every time it is reached, and + `js_regexp_new` materialized the canonical flags twice per construction: once + as a Rust `String` from `validate_and_canonicalize_flags`, and once as a fresh + GC `StringHeader` for `flags_ptr`. On the claude-code TUI that is **161,897 + constructions per 400-character reply** (`PERRY_REGEX_DIAG`) — ~5.2 MB of + identical one- and two-byte GC strings per reply, ~44 MB on a 3300-character + one, and ~1.4 million allocations. + + Neither copy is needed. There are eight legal flags, each may appear once, so + the canonical form is at most eight ASCII bytes and now lives inline in a + `CanonicalFlags` value instead of on the heap. And JS strings are immutable + with no identity semantics, so when the caller's flags text already IS the + canonical text — which it is for a literal, whose flags the author wrote in + spec order — the header shares the caller's string rather than duplicating + it. Only a non-canonical spelling (`/x/ig` → `"gi"`) or a computed + `new RegExp(p, f)` still materializes one; the new `flags_alloc` counter in + `PERRY_REGEX_DIAG` reports how often that happens. + + This is a **below-the-line** allocation fix by the campaign's own ~10 % rule: + at ~2-3 % of arena traffic per turn it cannot change the collection schedule, + and the cc rig is expected to read flat. It is worth doing because the + allocation is pure waste, not because it moves a benchmark. diff --git a/crates/perry-runtime/src/hot_diag.rs b/crates/perry-runtime/src/hot_diag.rs index faa49fba65..f5486197a5 100644 --- a/crates/perry-runtime/src/hot_diag.rs +++ b/crates/perry-runtime/src/hot_diag.rs @@ -105,6 +105,12 @@ pub struct RegexDiag { /// Sum of pattern bytes seen by `js_regexp_new` (what a content hash or /// copy of the pattern costs per construction). pub new_pattern_bytes: u64, + /// `js_regexp_new` had to allocate a GC string for the canonical flags + /// because the caller's flags string was not already in canonical form. + /// The common case — a regex literal, whose flags text the author wrote in + /// spec order — shares the caller's immutable string instead, so this + /// counter is the per-construction flags allocation that remains. + pub new_flags_allocated: u64, pub compiles_std: u64, pub compiles_fancy: u64, pub compiles_repeat: u64, @@ -238,7 +244,7 @@ impl RegexDiag { "[regex-diag] t={secs:.1}s new={} validated_hit={} site_hit={} pattern_bytes={} \ compiles std={} fancy={} repeat={} cache_clears={} lazy_builds={} lazy_cache_hits={} \ exec={} exec_matched={} capture_slots={} capture_bytes={} test={} test_global={} \ - match={} replace={} replace_matches={} split={}", + match={} replace={} replace_matches={} split={} flags_alloc={}", self.new_calls, self.new_validated_hit, self.new_site_hit, @@ -259,6 +265,7 @@ impl RegexDiag { self.replace_calls, self.replace_matches, self.split_calls, + self.new_flags_allocated, ); // Merge by content (prefix, len, flags): distinct literal sites with // the same pattern are one row. diff --git a/crates/perry-runtime/src/regex.rs b/crates/perry-runtime/src/regex.rs index fd25200d16..ae9d23102e 100644 --- a/crates/perry-runtime/src/regex.rs +++ b/crates/perry-runtime/src/regex.rs @@ -896,6 +896,32 @@ pub extern "C" fn js_regexp_new( let canonical_flags = validate_and_canonicalize_flags(raw_flags_str); let flags_str = canonical_flags.as_str(); + // ★ Share the caller's flags string when it is ALREADY the canonical text. + // + // `flags_ptr` used to be a fresh `js_string_from_str` on every + // construction. A JS regex literal evaluates to a fresh RegExp object + // every time it is reached, so that is one 32-byte GC string per + // evaluation: `PERRY_REGEX_DIAG` counts 161,897 constructions per + // 400-character claude-code reply, ~5.2 MB of identical one- and two-byte + // strings, and ~44 MB on a 3300-character reply. + // + // JS strings are immutable and have no identity semantics, and a literal's + // flags text is written by the author in spec order (`/x/gi`, not + // `/x/ig`), so the caller's string usually IS the canonical text and can + // simply be shared. Nothing downstream depends on the pointer being fresh: + // `flags_ptr`-keyed lookups (`FANCY_CACHE`, `lookup_fancy_regex`) read it + // through `string_as_str` and compare CONTENT, and the header keeping a + // pointer to it is what keeps it alive. + // + // This comparison must happen HERE, before the validation block below, + // because `raw_flags_str` borrows the caller's GC string and that block + // can allocate. The root is taken here for the same reason: the raw + // `flags` argument may name from-space after any allocation, exactly as + // the ★ note on `pattern_root` says, and this one is stored into the + // header too. + let shared_flags_root = (is_valid_ptr(flags) && raw_flags_str == flags_str) + .then(|| scope.root_string_ptr(flags)); + let case_insensitive = flags_str.contains('i'); let global = flags_str.contains('g'); let multiline = flags_str.contains('m'); @@ -1055,11 +1081,21 @@ pub extern "C" fn js_regexp_new( // leaked every header, which was a 64-byte-per-call leak on top of the // (now-fixed) regex object leak. let header_size = std::mem::size_of::(); - // Materialize the canonical flags into a fresh StringHeader so that - // `flags_ptr`-keyed lookups (FANCY_CACHE, lookup_fancy_regex) and the - // GC-survivable source table all agree on the canonical form, and the - // header never holds the caller's possibly-temporary input flags. - let canonical_flags_ptr = js_string_from_str(flags_str); + // `flags_ptr` must hold the CANONICAL form, so that `flags_ptr`-keyed + // lookups (FANCY_CACHE, lookup_fancy_regex) and the GC-survivable source + // table all agree. When the caller's string already is that text it is + // shared (rooted above); only a non-canonical spelling (`/x/ig` → `"gi"`, + // or a computed `new RegExp(p, f)`) still has to materialize one. The + // counter makes the removal provable rather than asserted. + let flags_root = match shared_flags_root { + Some(root) => root, + None => { + if crate::hot_diag::regex_on() { + crate::hot_diag::regex_with(|d| d.new_flags_allocated += 1); + } + scope.root_string_ptr(js_string_from_str(flags_str)) + } + }; // ★ #7341: root the canonical flags string too. The `gc_malloc` below is an // allocation and therefore a collection point, exactly as the comment above // `pattern_root` says — but only the PATTERN was rooted and re-read. The @@ -1072,7 +1108,7 @@ pub extern "C" fn js_regexp_new( // // The write barrier below already treated this as a real GC edge; what was // missing is that the value written had to survive the allocation first. - let flags_root = scope.root_string_ptr(canonical_flags_ptr); + unsafe { let raw = crate::gc::gc_malloc(header_size, crate::gc::GC_TYPE_REGEXP); if raw.is_null() { diff --git a/crates/perry-runtime/src/regex/flags.rs b/crates/perry-runtime/src/regex/flags.rs index 606e414d6a..67f5e9caff 100644 --- a/crates/perry-runtime/src/regex/flags.rs +++ b/crates/perry-runtime/src/regex/flags.rs @@ -14,7 +14,28 @@ use super::throw_regexp_syntax_error; /// its set-notation matching semantics are not implemented (the regex crate /// has no equivalent); it behaves like an ordinary unicode pattern. #[cfg(feature = "regex-engine")] -pub(super) fn validate_and_canonicalize_flags(flags: &str) -> String { +/// The canonical flag text, held inline. +/// +/// There are eight legal flags and each may appear once, so the canonical form +/// is at most eight ASCII bytes and never needs the heap. It used to be a +/// `String`, i.e. one heap allocation on **every** `RegExp` construction — and +/// a JS regex literal constructs a fresh object every time it is evaluated, so +/// on the claude-code TUI that was ~162,000 allocations per 400-character +/// reply for text that is almost always one or two bytes. +#[derive(Clone, Copy)] +pub(super) struct CanonicalFlags { + buf: [u8; 8], + len: u8, +} + +impl CanonicalFlags { + pub(super) fn as_str(&self) -> &str { + // Every byte written below comes from `FLAG_ORDER`, which is ASCII. + std::str::from_utf8(&self.buf[..self.len as usize]).unwrap_or("") + } +} + +pub(super) fn validate_and_canonicalize_flags(flags: &str) -> CanonicalFlags { // Spec order of the flag bits: d g i m s u v y. const FLAG_ORDER: &[char] = &['d', 'g', 'i', 'm', 's', 'u', 'v', 'y']; let mut seen = [false; 8]; @@ -37,10 +58,15 @@ pub(super) fn validate_and_canonicalize_flags(flags: &str) -> String { } } } - FLAG_ORDER - .iter() - .enumerate() - .filter(|(i, _)| seen[*i]) - .map(|(_, c)| *c) - .collect() + let mut out = CanonicalFlags { + buf: [0; 8], + len: 0, + }; + for (i, c) in FLAG_ORDER.iter().enumerate() { + if seen[i] { + out.buf[out.len as usize] = *c as u8; + out.len += 1; + } + } + out } From f4bcb1e4dc6e82a9aede67ca972fd4b0a913db98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 18:50:43 +0200 Subject: [PATCH 5/5] docs(changelog): key the flags-allocation fragment to PR 9819 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename only — the fragment was written before the PR number was known. Claude-Session: https://claude.ai/code/session_014UZWia6L37DpA93VLtNK9m --- ...{9812-regex-flags-no-alloc.md => 9819-regex-flags-no-alloc.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{9812-regex-flags-no-alloc.md => 9819-regex-flags-no-alloc.md} (100%) diff --git a/changelog.d/9812-regex-flags-no-alloc.md b/changelog.d/9819-regex-flags-no-alloc.md similarity index 100% rename from changelog.d/9812-regex-flags-no-alloc.md rename to changelog.d/9819-regex-flags-no-alloc.md