diff --git a/changelog.d/9885-regex-newborn-barrier-gate.md b/changelog.d/9885-regex-newborn-barrier-gate.md new file mode 100644 index 0000000000..751904067f --- /dev/null +++ b/changelog.d/9885-regex-newborn-barrier-gate.md @@ -0,0 +1,43 @@ +**A `RegExp` literal's construction no longer pays the write barrier's parent +classification.** Since #9845 the `RegExpHeader` is a nursery allocation, so +its two string field stores — `pattern_ptr` and `flags_ptr` — cannot owe the +remembered set anything; they were still taking the full barrier and +discovering that fact, twice, at a cost of four page-map classifications, two +dirty-page-cache probes and two child classifications per construction, all +ending at `ParentNotOldSkips`. + +The fix is the runtime twin of a gate 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 ⇒ neither the remembered set nor the SATB shading has +anything to record. Both clauses are read live, so a header a collection +promoted between `arena_alloc_gc` and the store, or a +`RegExp.prototype.compile` reassigning a tenured receiver, still takes the +full path. + +Why the two clauses and not one: the tenured bit answers the generational +question, and the incremental count is what makes it legal to skip the +insertion shading as well — dropping either is a live child swept, which is +what `gc::tests::inline_generation_gate_contract` already pins for the emitted +gate and now pins for the runtime twin, clause by clause, against the same +codegen predicate. A third test asserts on the header `js_regexp_new` actually +returns, so the skip arm is proven reached rather than merely available. + +Measured motivation (segment-loop probe, region B, 60,000 reps, `sample`, main +thread, leaf sum = thread header exactly): the probe constructs 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 handicapped control. + +`PERRY_REGEX_DIAG` gains the counters that make the claim checkable rather +than argued: `barrier_taken` / `barrier_gated` (whose sum must equal `new`), +`header_bytes`, `site_verify_bytes` (the site cache's byte-compare volume, +which `pattern_bytes` does not isolate) and `side_table_inserts`. Two +reliability fixes ride along: a diag file the process cannot write now says so +on stderr and falls back there instead of vanishing silently, and the first +snapshot is written at the first tick rather than after a full second, so a +short run can no longer look like a dead instrument. diff --git a/changelog.d/9886-regex-literal-site-key.md b/changelog.d/9886-regex-literal-site-key.md new file mode 100644 index 0000000000..91ee033086 --- /dev/null +++ b/changelog.d/9886-regex-literal-site-key.md @@ -0,0 +1,55 @@ +**A regex literal is now identified by its SOURCE SITE, not by its text**, so +constructing one costs a single word compare instead of a content fingerprint +plus a full byte compare of the pattern. + +A regex literal evaluates to a fresh object every time it is reached +(ECMA-262), and TUI code reaches them inside hot functions: `string-width`'s +`emojiRegex()` returns a fresh ~12,807-character `/…/g` on every call, once per +grapheme in claude-code's layout pass. The runtime therefore re-derived "which +pattern is this?" from the text on every construction — `regex::site_cache` +keys on a cheap fingerprint and, because a fingerprint can collide, verifies +every hit with `&*entry.pattern == pattern`. That verify is linear in the +pattern: `PERRY_REGEX_DIAG` measured **2.0 GB of `memcmp` per 400-character +reply**, and a `sample` of the segment loop put `_platform_memcmp` at **39.6 % +of `js_regexp_new`'s own subtree**. + +The compiler knew the answer all along; the lowering just had no way to say it. +`Expr::RegExp` now emits an 8-byte private global per literal site and passes +its **address** as a third argument to a new `js_regexp_new_site(pattern, +flags, site_key)`. That address is unique by construction, immortal, and never +moves — which is exactly what a `StringHeader` address is not, and why the +earlier analysis of this problem concluded no sound string identity existed and +left the byte compare in place: string headers are GC-managed, so an address is +freed and reused, and a moving collector relocates them. + +A hit verifies with one word plus the site's ≤ 8-byte flags text (two spellings +of one canonical form must not answer for each other) and then reads nothing +about the pattern at all: no fingerprint, no `memcmp`, no validation — validity +is a pure function of `(pattern, flags)` and the site's first construction +established it — and no flag canonicalization, since the seven flag bits are a +property of the site. Once the site's first header has executed, later +constructions are born built. + +`site_key = 0` means "no site" and behaves exactly as before, so every dynamic +construction (`new RegExp(s)`, `js_regexp_construct`, +`RegExp.prototype.compile`, the runtime's own callers) keeps the two-argument +entry point and never touches the site table — pinned by a test that asserts +the table is still empty after four dynamic constructions, and non-empty after +one site-keyed one, so the zero is a property of the entry point rather than of +a table that never works. + +The named sabotage is a table keyed by anything weaker than the site address: +two literals at two sites, same flags, **same pattern length**, different text. +Under a length- or prefix-keyed table the second site inherits the first's +entry, `.source` reports a pattern the literal never contained and `test` +matches the wrong language. Each site is constructed twice, because a first +construction always misses and would pass under every sabotage. + +Kill switch: `PERRY_REGEX_SITE_KEY=0` — the probe misses and nothing is +recorded, so the OFF arm is the content-keyed path exactly rather than a +control still paying the bookkeeping. + +The new runtime symbol is declared in `runtime_decls/strings.rs` with a test +asserting its **name and arity**: a missing `declare` is invisible to every +HIR-level test and fails only at the in-process LLVM parse (`use of undefined +value`), and a wrong arity parses and miscompiles. diff --git a/crates/perry-codegen/src/expr/logical_collections.rs b/crates/perry-codegen/src/expr/logical_collections.rs index 1073cdd045..68b3be1a6e 100644 --- a/crates/perry-codegen/src/expr/logical_collections.rs +++ b/crates/perry-codegen/src/expr/logical_collections.rs @@ -1283,15 +1283,77 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { let flags_idx = ctx.strings.intern(flags); let pattern_global = format!("@{}", ctx.strings.entry(pattern_idx).handle_global); let flags_global = format!("@{}", ctx.strings.entry(flags_idx).handle_global); + // ★ A literal's SITE IDENTITY, as an immortal address. + // + // A regex literal evaluates to a fresh object every time it is + // reached (ECMA-262), and TUI code reaches them inside hot + // functions — `string-width`'s `emojiRegex()` returns a fresh + // ~12,807-character `/…/g` per call. The runtime therefore + // re-derives "which pattern is this?" per construction from the + // TEXT: a content fingerprint plus, on every hit, a full byte + // compare to verify it (`regex::site_cache::entry_matches`). On + // claude-code that verify is ~2.0 GB of `memcmp` per 400-character + // reply, and it is 39.6 % of `js_regexp_new`'s own profile subtree. + // + // The compiler knows the answer statically: this literal is one + // source site whose pattern and flags can never change. What the + // runtime was missing is the key, because the lowering passed only + // the two string handles. This emits an 8-byte private global per + // literal site and passes its ADDRESS — unique by construction + // (distinct globals have distinct addresses), immortal (it is not + // GC memory, so it can never be freed and reused under a stale + // cache entry, which is why the string handles themselves cannot + // serve), and stable for the process. The runtime's site table + // then verifies a hit by comparing that one word, and never looks + // at the pattern at all. + // + // The slot is zero-initialised so it lands in `__bss` and costs + // nothing until the linker lays it out (#9610's lesson about + // zero-initialised globals applies: `private global i64 0`, not a + // non-zero initialiser). Naming carries the module prefix for the + // same reason `inline_cache_global_name` does — codegen-unit + // splitting can promote a private global for cross-unit use. + // + // The slot must reach the module, so every lowering exit has to + // PUBLISH `typed_parse_rodata` rather than drop it. That was not + // true when this landed: `codegen/method.rs`'s "parent class has + // no callable constructor symbol" bail-out lowered the body and + // then discarded the three artifact collections, so a regex + // literal inside such a constructor would have referenced a + // global that is never defined (#9890, fixed by #9896 — every + // return now goes through `publish_lowered_fn_artifacts`, which + // also restores `llmod.ic_counter` and so closes the duplicate + // site-id half). Kept as a note because the obligation is real + // and unenforced: a future early return that drops the artifacts + // breaks this site, loudly, at the in-process LLVM parse (`use of + // undefined value`) rather than at runtime. + let site_id = ctx.ic_site_counter; + ctx.ic_site_counter += 1; + let slot_name = { + let prefix = ctx.strings.module_prefix(); + if prefix.is_empty() { + format!("perry_regexp_site_{site_id}") + } else { + format!("perry_regexp_site_{prefix}__{site_id}") + } + }; + ctx.typed_parse_rodata + .push(format!("@{slot_name} = private global i64 0")); + let slot_ref = format!("@{slot_name}"); let blk = ctx.block(); let pattern_box = blk.load(DOUBLE, &pattern_global); let flags_box = blk.load(DOUBLE, &flags_global); let pattern_handle = unbox_to_i64(blk, &pattern_box); let flags_handle = unbox_to_i64(blk, &flags_box); + let site_key = blk.ptrtoint(&slot_ref, I64); let result = blk.call( I64, - "js_regexp_new", - &[(I64, &pattern_handle), (I64, &flags_handle)], + "js_regexp_new_site", + &[ + (I64, &pattern_handle), + (I64, &flags_handle), + (I64, &site_key), + ], ); Ok(nanbox_pointer_inline(blk, &result)) } diff --git a/crates/perry-codegen/src/runtime_decls/mod.rs b/crates/perry-codegen/src/runtime_decls/mod.rs index ec14cd207e..fab34b46eb 100644 --- a/crates/perry-codegen/src/runtime_decls/mod.rs +++ b/crates/perry-codegen/src/runtime_decls/mod.rs @@ -213,3 +213,56 @@ pub fn declare_phase_a_strings(module: &mut LlModule) { // function once they grow. declare_phase_b_strings(module); } + +#[cfg(test)] +mod tests { + use super::*; + + /// A lowering that introduces a new runtime call needs one test that + /// reaches the DECLARATION, not just the HIR. + /// + /// #9859 emitted five `js_segments_view_*` calls whose symbols were never + /// declared in the LLVM module: twelve HIR-level unit tests passed and the + /// first real compile died at the in-process LLVM parse with `use of + /// undefined value`. The arity half matters just as much and fails more + /// quietly — a wrong arity PARSES and miscompiles, handing the runtime a + /// garbage argument. + /// + /// `Expr::RegExp` lowers to `js_regexp_new_site(pattern, flags, site_key)` + /// (`expr/logical_collections.rs`), so the declaration must be exactly + /// three `i64` parameters returning `i64`. + #[test] + fn the_literal_site_regexp_entry_is_declared_with_its_exact_arity() { + let mut module = crate::module::LlModule::new("arm64-apple-macosx"); + declare_phase_b_strings(&mut module); + + let line = module + .declaration_lines() + .find(|(name, _)| *name == "js_regexp_new_site") + .map(|(_, line)| line.to_string()) + .expect( + "`Expr::RegExp` emits a call to `js_regexp_new_site`; without a `declare` the \ + module fails the in-process LLVM parse with `use of undefined value`, which no \ + HIR-level test can see", + ); + assert!( + line.starts_with("declare i64 @js_regexp_new_site(i64, i64, i64)"), + "the site-keyed entry takes (pattern handle, flags handle, site key) and returns a \ + RegExpHeader handle — a wrong arity parses and miscompiles instead of failing. Got: \ + {line}" + ); + + // The two-argument form stays, because every non-literal construction + // (`new RegExp(str)`, `js_regexp_construct`, the runtime's own + // callers) uses it and must never reach the site table. + let plain = module + .declaration_lines() + .find(|(name, _)| *name == "js_regexp_new") + .map(|(_, line)| line.to_string()) + .expect("the dynamic form must remain declared"); + assert!( + plain.starts_with("declare i64 @js_regexp_new(i64, i64)"), + "got: {plain}" + ); + } +} diff --git a/crates/perry-codegen/src/runtime_decls/strings.rs b/crates/perry-codegen/src/runtime_decls/strings.rs index 765fe9c8e7..911e318a47 100644 --- a/crates/perry-codegen/src/runtime_decls/strings.rs +++ b/crates/perry-codegen/src/runtime_decls/strings.rs @@ -1312,6 +1312,13 @@ pub fn declare_phase_b_strings(module: &mut LlModule) { &[DOUBLE, DOUBLE, DOUBLE, I32, DOUBLE], ); module.declare_function("js_regexp_new", I64, &[I64, I64]); + // The literal-site form (`Expr::RegExp` lowering). A missing `declare` + // here is invisible to every HIR-level test and fails only at the + // in-process LLVM parse with `use of undefined value` — which is exactly + // how #9859's five segment-view externs were caught, after twelve passing + // unit tests. `runtime_decls::tests` asserts the name AND the arity: a + // wrong arity parses and miscompiles. + module.declare_function("js_regexp_new_site", I64, &[I64, I64, I64]); // Full ECMAScript RegExp constructor: NaN-boxed pattern + flags in, handles // RegExp/undefined/object patterns and ToString-coerced flags. module.declare_function("js_regexp_construct", I64, &[DOUBLE, DOUBLE]); diff --git a/crates/perry-runtime/src/gc/barrier_store.rs b/crates/perry-runtime/src/gc/barrier_store.rs index c77a3eb4bd..2b43c80240 100644 --- a/crates/perry-runtime/src/gc/barrier_store.rs +++ b/crates/perry-runtime/src/gc/barrier_store.rs @@ -319,3 +319,60 @@ pub(super) fn barrier_remembering_active() -> bool { bump_write_barrier_trace_counter(BarrierTraceCounter::UnarmedSkips); false } + +/// The runtime twin of codegen's `emit_parent_may_need_remembering_check` +/// (#7511): may a store into a **freshly allocated** GC parent be skipped +/// outright? +/// +/// # Why this exists on the runtime side too +/// +/// Every store emitted by the compiler is already gated this way — the +/// generated code reads the parent's `gc_flags` and, when `GC_FLAG_TENURED` +/// is clear *and* no incremental cycle is live anywhere, jumps over the +/// barrier call entirely. Runtime-Rust construction paths call +/// [`runtime_write_barrier_gc_slot`] unconditionally instead, so a native +/// header born in the nursery pays, per pointer slot: a page-map +/// classification for the malloc-parent probe, `barrier_child_prologue`, the +/// armed check, the dereferenceability test, the dirty-page-cache probe and a +/// second page-map classification in `barrier_parent_needs_remembering` — +/// all of which end at `ParentNotOldSkips` because the parent is young. +/// +/// Measured on the segment-loop probe (region B, 60,000 reps, `sample`, main +/// thread): `js_regexp_new` constructs one `RegExp` per grapheme and its +/// barrier subtree — `runtime_write_barrier_gc_slot` / +/// `write_barrier_slot_decoded` / `write_barrier_decoded_parent` / +/// `mark_dirty_external_slot_page` / `remembered_child_needs_tracking` / +/// `classify_heap_generation_uncached` — is 739 of the 14,628 main-thread +/// samples, 32 % of that function's own subtree. +/// +/// # Why it is sound +/// +/// Exactly the two clauses the emitted gate uses, for exactly the two reasons +/// its doc comment gives: +/// +/// * **`GC_FLAG_TENURED` clear** ⇒ the parent is not in the old generation, +/// so no old→young remembered-set entry can be owed. The flag is read +/// LIVE at the store, not claimed statically, because promotion can move +/// an object under any static proof (#7501) — a header that a collection +/// promoted between its allocation and this store reads TENURED here and +/// takes the full barrier. +/// * **`PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT == 0`** ⇒ no thread has +/// an incremental mark barrier installed, which is what makes it legal to +/// skip the SATB/insertion shading as well +/// ([`incremental_mark_barrier_globally_idle`]). Shading is not a +/// generational question and must never be dropped while a cycle is live, +/// so a non-zero count sends the store down the ordinary path. +/// +/// # Safety +/// +/// Dereferences `parent_addr - GC_HEADER_SIZE`. The caller must pass a live, +/// non-forwarded GC user pointer it has just allocated (or otherwise +/// validated) — the same contract `emit_parent_may_need_remembering_check` +/// places on its caller. +#[inline] +pub(crate) unsafe fn newborn_parent_needs_barrier(parent_addr: usize) -> bool { + if !super::barrier::incremental_mark_barrier_globally_idle() { + return true; + } + (*super::layout::header_from_user_ptr(parent_addr as *const u8)).gc_flags & GC_FLAG_TENURED != 0 +} diff --git a/crates/perry-runtime/src/gc/tests/inline_generation_gate_contract.rs b/crates/perry-runtime/src/gc/tests/inline_generation_gate_contract.rs index e4df96234c..04eade291e 100644 --- a/crates/perry-runtime/src/gc/tests/inline_generation_gate_contract.rs +++ b/crates/perry-runtime/src/gc/tests/inline_generation_gate_contract.rs @@ -262,3 +262,141 @@ fn the_incremental_clause_forces_the_call_for_a_young_parent() { store skips its insertion barrier and a live object is swept" ); } + +// --------------------------------------------------------------------------- +// The RUNTIME twin of the same gate. +// +// Runtime-Rust construction paths (`js_regexp_new` and friends) call the +// barrier unconditionally, so a native header born in the nursery pays the +// full parent classification on every field it initialises while generated +// code, storing into the very same kind of object, skips it. These tests pin +// `gc::newborn_parent_needs_barrier` to the emitted predicate CLAUSE FOR +// CLAUSE, so the two can only drift by failing here. +// --------------------------------------------------------------------------- + +/// Clause 1 (`GC_FLAG_TENURED`): the runtime twin must answer exactly what the +/// emitted gate answers for the same live header. +/// +/// Sabotage that this catches: a twin that only consults the incremental +/// count answers "skip" for the tenured parent and fails the second assert — +/// which is the stranded-child bug of +/// `sabotaged_parent_gate_strands_a_young_child_the_shipped_gate_keeps`, +/// reached from the runtime side instead of the emitted side. +#[test] +fn the_runtime_twin_reads_the_tenured_clause_from_the_live_header() { + let _guard = GcTestIsolationGuard::new(); + assert!( + crate::gc::incremental_mark_barrier_globally_idle(), + "this test isolates clause 1, so no cycle may be live" + ); + + let young = crate::arena::arena_alloc_gc(64, 8, GC_TYPE_OBJECT) as usize; + assert_eq!( + header_flags(young) & GC_FLAG_TENURED, + 0, + "a fresh nursery allocation must not be TENURED — otherwise this test exercises nothing" + ); + assert!( + !unsafe { crate::gc::newborn_parent_needs_barrier(young) }, + "a nursery parent with no cycle live is exactly the case the gate exists to skip" + ); + assert_eq!( + unsafe { crate::gc::newborn_parent_needs_barrier(young) }, + codegen_parent_may_need_remembering(header_flags(young), 0), + "the runtime twin and the emitted gate must agree on a nursery parent" + ); + + // The SAME address, now carrying the bit a promotion would have stamped. + // Read live, so this models a header a collection promoted between its + // allocation and the store that follows it. + unsafe { (*header_from_user_ptr(young as *const u8)).gc_flags |= GC_FLAG_TENURED }; + assert!( + unsafe { crate::gc::newborn_parent_needs_barrier(young) }, + "a TENURED parent owes the remembered set an entry and must take the full barrier" + ); + assert_eq!( + unsafe { crate::gc::newborn_parent_needs_barrier(young) }, + codegen_parent_may_need_remembering(header_flags(young), 0), + "the runtime twin and the emitted gate must agree on a tenured parent" + ); + unsafe { (*header_from_user_ptr(young as *const u8)).gc_flags &= !GC_FLAG_TENURED }; +} + +/// Clause 2 (the incremental count): with a cycle live anywhere, a nursery +/// parent must still take the call, because the skipped work includes the +/// SATB/insertion shading and that is not a generational question. +/// +/// Sabotage that this catches: a twin that only reads the header's flags +/// answers "skip" while a cycle is marking, and a child linked in during that +/// window is never shaded. +#[test] +fn the_runtime_twin_forces_the_barrier_while_an_incremental_cycle_is_live() { + let _guard = GcTestIsolationGuard::new(); + + let young = crate::arena::arena_alloc_gc(64, 8, GC_TYPE_OBJECT) as usize; + assert!( + !unsafe { crate::gc::newborn_parent_needs_barrier(young) }, + "precondition: with no cycle live this parent is skipped" + ); + + crate::gc::PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT.fetch_add(1, Ordering::Relaxed); + let forced = unsafe { crate::gc::newborn_parent_needs_barrier(young) }; + let codegen_answer = codegen_parent_may_need_remembering( + header_flags(young), + crate::gc::PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT.load(Ordering::Relaxed), + ); + crate::gc::PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT.fetch_sub(1, Ordering::Relaxed); + + assert!( + forced, + "a live incremental cycle must force the call even for a nursery parent — dropping this \ + clause skips the insertion shading and sweeps a live child" + ); + assert_eq!( + forced, codegen_answer, + "the runtime twin and the emitted gate must agree while a cycle is live" + ); + assert!( + !unsafe { crate::gc::newborn_parent_needs_barrier(young) }, + "the count is back to zero, so the same parent is skippable again" + ); +} + +/// **Did this code run?** The gate is only worth anything if the real +/// `js_regexp_new` header reaches its skip arm — a fast path that is available +/// but never taken is the campaign's "measured flat" failure in advance. +/// +/// Asserts on the header `js_regexp_new` actually produced, not on a synthetic +/// fixture: since #9845 it is a nursery allocation, so with no cycle live the +/// two `pattern_ptr` / `flags_ptr` stores skip the barrier entirely, and the +/// SAME header answers "take the barrier" the moment it carries the bit a +/// promotion would stamp. +#[cfg(feature = "regex-engine")] +#[test] +fn a_freshly_constructed_regexp_header_reaches_the_skip_arm() { + let _guard = GcTestIsolationGuard::new(); + + let pattern = crate::string::js_string_from_bytes(b"a(b)c".as_ptr(), 5); + let flags = crate::string::js_string_from_bytes(b"g".as_ptr(), 1); + let re = crate::regex::js_regexp_new(pattern, flags) as usize; + + assert_eq!( + crate::arena::classify_heap_generation(re), + crate::arena::HeapGeneration::Nursery, + "#9845 allocates the RegExp header in the NURSERY; if that changes, the construction path \ + stops being the case this gate is written for" + ); + assert!( + !unsafe { crate::gc::newborn_parent_needs_barrier(re) }, + "the construction path must actually TAKE the skip arm — an available-but-unreached fast \ + path is indistinguishable from no fast path in a measurement" + ); + + unsafe { (*header_from_user_ptr(re as *const u8)).gc_flags |= GC_FLAG_TENURED }; + assert!( + unsafe { crate::gc::newborn_parent_needs_barrier(re) }, + "the same header, promoted, must take the full barrier — this is the \ + `RegExp.prototype.compile`-on-a-tenured-receiver case" + ); + unsafe { (*header_from_user_ptr(re as *const u8)).gc_flags &= !GC_FLAG_TENURED }; +} diff --git a/crates/perry-runtime/src/hot_diag.rs b/crates/perry-runtime/src/hot_diag.rs index ca92f9f93b..51615783ca 100644 --- a/crates/perry-runtime/src/hot_diag.rs +++ b/crates/perry-runtime/src/hot_diag.rs @@ -37,13 +37,25 @@ fn sink_from_env(name: &str) -> Option { } } +/// A failed file write used to be swallowed (`if ... .is_ok()`), so an +/// unwritable path — a directory that does not exist, a read-only mount, a +/// sandbox — produced *no file and no message*, which greps identically to +/// "this instrument was never built". That is the campaign's own +/// missing-exit-line trap in a second form, and it cost a lane a measurement +/// run. Report the first failure on stderr, naming the path and the error, +/// and keep writing there. fn write_sink(sink: &Sink, text: &str) { match sink { Sink::Stderr => eprint!("{text}"), Sink::File(path) => { let tmp = format!("{path}.tmp"); - if std::fs::write(&tmp, text).is_ok() { - let _ = std::fs::rename(&tmp, path); + let wrote = std::fs::write(&tmp, text).and_then(|()| std::fs::rename(&tmp, path)); + if let Err(err) = wrote { + static WARNED: AtomicBool = AtomicBool::new(false); + if !WARNED.swap(true, Ordering::Relaxed) { + eprintln!("[hot-diag] cannot write {path}: {err} — falling back to stderr"); + } + eprint!("{text}"); } } } @@ -140,6 +152,34 @@ pub struct RegexDiag { /// meta edge was wired for RegExp this was 0 by construction: the filter /// answered "maybe" for every one of them. pub desc_regexp_meta_negative: u64, + /// Constructions whose two header string stores took the full write + /// barrier pair (`GC_FLAG_TENURED` set on the freshly allocated header, + /// or an incremental cycle live anywhere). + pub new_barrier_taken: u64, + /// Constructions the newborn-parent gate proved owe the remembered set + /// nothing, so neither barrier call ran. `taken + gated == new_calls` + /// is the invariant: a run where `gated` is 0 did not exercise the gate. + pub new_barrier_gated: u64, + /// Bytes of `RegExpHeader` allocated by `js_regexp_new`. Load-independent + /// and directly comparable with a probe's allocation-per-grapheme reading. + pub new_header_bytes: u64, + /// Bytes the literal-site cache byte-compared to VERIFY a fingerprint + /// match (`site_cache::entry_matches`). Distinct from `pattern_bytes`, + /// which counts every construction's pattern length whether the probe hit + /// or missed: this is the `memcmp` volume alone, which is what a 12 KB + /// emoji pattern makes expensive and a 60-byte one does not. + pub new_site_verify_bytes: u64, + /// Address-keyed side-table inserts performed per construction + /// (`REGEX_POINTERS` and `REGEX_SOURCE_TABLE`) — two per header, each a + /// `PtrHasher` hash plus a hashbrown insert, mirrored by two removals at + /// death and two rekeys per evacuation. + pub new_side_table_inserts: u64, + /// Constructions answered from the LITERAL-SITE table — identity by the + /// compiler-emitted site global's address, so neither the pattern's + /// fingerprint nor its byte compare ran. `site_hit` counts the + /// CONTENT-keyed cache; a site hit never reaches it, so the two are + /// disjoint and `site_key_hit + site_hit <= new`. + pub new_site_key_hit: u64, per_pattern: HashMap, } @@ -147,6 +187,33 @@ crate::perry_thread_local! { static REGEX_DIAG: RefCell = RefCell::new(RegexDiag::default()); } +/// Accumulate into the thread's regex counters WITHOUT ticking the dump clock. +/// +/// `regex_with` counts every call as an "event" and dumps every `TICK_EVERY` +/// events once a second has passed, so the snapshot a `SIGKILL`ed process +/// leaves behind lands wherever the event stream happened to be. Adding a +/// second probe to a path that already had one therefore does not just add a +/// counter — it **doubles that path's event rate and moves the last snapshot**, +/// which makes two arms' absolute counts describe different windows of the +/// same workload. +/// +/// Measured, on the I6 cc arm: the extra per-construction probes took +/// `new / t` from 206 k/s to 173 k/s between two arms whose per-call ratios +/// agree to 0.13 %. Counters that ride along on an already-instrumented path +/// use this entry point so the cadence stays the pre-change one and the +/// windows stay comparable. +#[inline] +pub fn regex_counters(f: impl FnOnce(&mut RegexDiag)) { + REGEX_DIAG.with(|d| { + let mut d = d.borrow_mut(); + if d.started.is_none() { + d.started = Some(Instant::now()); + d.last_dump = None; + } + f(&mut d); + }); +} + /// Run `f` against the thread's regex counters, then maybe dump. #[inline] pub fn regex_with(f: impl FnOnce(&mut RegexDiag)) { @@ -154,14 +221,17 @@ pub fn regex_with(f: impl FnOnce(&mut RegexDiag)) { let mut d = d.borrow_mut(); if d.started.is_none() { d.started = Some(Instant::now()); - d.last_dump = d.started; + // `last_dump` stays None so the FIRST tick dumps immediately: a + // run shorter than `DUMP_INTERVAL_MS` used to write nothing at + // all, which is indistinguishable from a dead instrument. + d.last_dump = None; } f(&mut d); d.events = d.events.wrapping_add(1); if d.events % TICK_EVERY == 0 { let due = d .last_dump - .is_some_and(|t| t.elapsed().as_millis() >= DUMP_INTERVAL_MS); + .is_none_or(|t| t.elapsed().as_millis() >= DUMP_INTERVAL_MS); if due { d.last_dump = Some(Instant::now()); if let Some(sink) = regex_sink() { @@ -254,7 +324,9 @@ impl RegexDiag { 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={} flags_alloc={} \ - desc_regexp_probes={} desc_regexp_meta_negative={}", + desc_regexp_probes={} desc_regexp_meta_negative={} \ + barrier_taken={} barrier_gated={} header_bytes={} site_verify_bytes={} \ + side_table_inserts={} site_key_hit={}", self.new_calls, self.new_validated_hit, self.new_site_hit, @@ -278,6 +350,12 @@ impl RegexDiag { self.new_flags_allocated, self.desc_regexp_probes, self.desc_regexp_meta_negative, + self.new_barrier_taken, + self.new_barrier_gated, + self.new_header_bytes, + self.new_site_verify_bytes, + self.new_side_table_inserts, + self.new_site_key_hit, ); // 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 73dd933c05..d8a1ccc1ef 100644 --- a/crates/perry-runtime/src/regex.rs +++ b/crates/perry-runtime/src/regex.rs @@ -69,6 +69,11 @@ mod replace_expand; mod replace_fn; #[cfg(feature = "regex-engine")] mod site_cache; +/// Literal-site keyed construction cache — identity by an immortal address +/// emitted per regex literal, so a hit costs one word compare instead of a +/// fingerprint plus a full byte compare of the pattern. +#[cfg(feature = "regex-engine")] +mod site_key; #[cfg(feature = "regex-engine")] mod unicode17; #[cfg(feature = "regex-engine")] @@ -943,6 +948,24 @@ pub(super) fn throw_regexp_syntax_error(message: &str) -> ! { crate::exception::js_throw(crate::value::js_nanbox_pointer(err as i64)) } +/// Kill switch for the newborn-parent barrier gate below +/// (`PERRY_REGEX_NEWBORN_BARRIER_GATE=0` ⇒ the two header stores take the +/// unconditional barrier pair, i.e. the pre-gate code path exactly). One +/// relaxed load of a `OnceLock` per construction, resolved once per process, +/// mirroring `regex::site_cache::enabled`. +#[cfg(feature = "regex-engine")] +#[inline] +fn newborn_barrier_gate_enabled() -> bool { + static ON: std::sync::OnceLock = std::sync::OnceLock::new(); + *ON.get_or_init(|| { + crate::gc::env_default_on_from_value( + std::env::var("PERRY_REGEX_NEWBORN_BARRIER_GATE") + .ok() + .as_deref(), + ) + }) +} + /// Create a new RegExp from pattern and flags strings /// Returns a pointer to RegExpHeader /// @@ -957,6 +980,49 @@ pub(super) fn throw_regexp_syntax_error(message: &str) -> ! { pub extern "C" fn js_regexp_new( pattern: *const StringHeader, flags: *const StringHeader, +) -> *mut RegExpHeader { + js_regexp_new_impl(pattern, flags, 0) +} + +/// [`js_regexp_new`] for a **regex literal**, which the compiler can identify +/// by its source site instead of by its text. +/// +/// `site_key` is the address of an 8-byte private global the `Expr::RegExp` +/// lowering emits once per literal (`expr/logical_collections.rs`). It is +/// unique by construction, immortal, and never moves, which is what makes it a +/// sound identity where a `StringHeader` address is not: string headers are +/// GC-managed, so an address is freed and reused and a moving collector +/// relocates them, and a pointer-keyed cache over them would answer for a +/// different pattern. +/// +/// A hit therefore verifies with ONE word compare (plus the site's ≤ 8-byte +/// flags text) and never reads the pattern at all — no fingerprint, no +/// `memcmp`, no validation, no flag canonicalization. On claude-code the +/// segment loop constructs `string-width`'s ~12,807-character `/…/g` once per +/// grapheme, and the content cache's exactness verify alone is ~2.0 GB of +/// `memcmp` per 400-character reply. +/// +/// A `site_key` of 0 means "no site" and behaves exactly like +/// [`js_regexp_new`]; every dynamic construction (`new RegExp(s)`, +/// [`js_regexp_construct`], the runtime's own callers) keeps the two-argument +/// form and never touches the site table. +/// +/// Kill switch: `PERRY_REGEX_SITE_KEY=0`. +#[cfg(feature = "regex-engine")] +#[no_mangle] +pub extern "C" fn js_regexp_new_site( + pattern: *const StringHeader, + flags: *const StringHeader, + site_key: i64, +) -> *mut RegExpHeader { + js_regexp_new_impl(pattern, flags, site_key as usize) +} + +#[cfg(feature = "regex-engine")] +fn js_regexp_new_impl( + pattern: *const StringHeader, + flags: *const StringHeader, + site_key: usize, ) -> *mut RegExpHeader { // ★ `pattern` is a raw `StringHeader*` in a Rust local, and this function // allocates twice below (`js_string_from_str` for the canonical flags, then @@ -973,151 +1039,215 @@ pub extern "C" fn js_regexp_new( // in `js_regexp_new` itself, on BOTH sides of an unrelated codegen change. let scope = crate::gc::RuntimeHandleScope::new(); let pattern_root = scope.root_string_ptr(pattern); - let pattern_str = if is_valid_ptr(pattern) { - string_as_str(pattern) - } else { - "" - }; let raw_flags_str = if is_valid_ptr(flags) { string_as_str(flags) } else { "" }; - // #2829: reject duplicate/unknown flags (SyntaxError) and store the - // canonical sorted form so `.flags` reflects Node's ordering. - 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. + // ★ LITERAL-SITE FAST PATH — identity by an immortal address. // - // 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. + // `site_key` is the address of a private global the compiler emits once + // per regex literal, so a match on it proves this is the SAME SOURCE SITE + // that recorded the entry, whose pattern and flags are fixed at compile + // time. Nothing about the pattern text is read: no fingerprint, no + // `memcmp`, no validation, no flag canonicalization. The flags text IS + // compared, because it is at most eight bytes and because two spellings of + // one canonical form (`/x/ig`, `/x/gi`) must not answer for each other. // - // 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'); - let sticky = flags_str.contains('y'); - let dot_all = flags_str.contains('s'); - let unicode = flags_str.contains('u') || flags_str.contains('v'); - let has_indices = flags_str.contains('d'); - - // Content-keyed construction cache (`regex::site_cache`): a verified hit - // means this exact `(pattern, canonical flags)` already cleared the - // validation below — validity is a pure function of the pair — and hands - // back the shared owned copies plus, once some header built from this - // text has been executed, its compiled programs. The probe is one - // fingerprint and one byte compare; everything below it that copies or - // hashes the pattern is skipped. - let site_hit = site_cache::lookup(pattern_str, flags_str); - let validated_hit = - site_hit.is_some() || lazy::pattern_already_validated(pattern_str, flags_str); - if crate::hot_diag::regex_on() { - crate::hot_diag::regex_with(|d| { - d.note_new( - pattern as usize, - pattern_str.as_bytes(), - flags_str, - validated_hit && site_hit.is_none(), - site_hit.is_some(), + // A `site_key` of 0 (every dynamic construction, and every runtime caller) + // misses by construction and takes the content-keyed path below unchanged. + let site_entry = site_key::lookup(site_key, raw_flags_str); + let (owned_pattern, owned_flags, programs, bits, shared_flags_root) = match site_entry { + Some(hit) => { + // The site's own flags literal, so this is the same sharing + // decision the first construction at this site made (#9819). + let shared_flags_root = (hit.flags_are_canonical && is_valid_ptr(flags)) + .then(|| scope.root_string_ptr(flags)); + debug_assert!( + !is_valid_ptr(pattern) || string_as_str(pattern) == &*hit.pattern, + "a site key names ONE source literal, whose pattern text cannot change; a \ + caller that reuses a key for different text would silently take another \ + site's program" + ); + if crate::hot_diag::regex_on() { + let bytes: &[u8] = hit.pattern.as_bytes(); + let flags_text: &str = &hit.flags; + crate::hot_diag::regex_with(|d| { + d.new_site_key_hit += 1; + d.note_new(pattern as usize, bytes, flags_text, false, true); + }); + } + // Until the site's first execution installs the compiled programs, + // pick them up from the content cache — one probe per construction, + // and in a loop that matches immediately that is exactly one. + let programs = match hit.programs { + Some(programs) => Some(programs), + None => { + let picked = + site_cache::lookup(&hit.pattern, &hit.flags).and_then(|h| h.programs); + if let Some(programs) = picked.clone() { + site_key::install_programs(site_key, programs); + } + picked + } + }; + ( + hit.pattern, + hit.flags, + programs, + hit.bits, + shared_flags_root, ) - }); - } + } + None => { + let pattern_str = if is_valid_ptr(pattern) { + string_as_str(pattern) + } else { + "" + }; - // #2829: reject invalid pattern syntax with a SyntaxError. A pattern the - // `regex` crate rejects is only a real error if `fancy-regex` (which - // covers the full JS feature set: lookbehind/lookahead/backreferences) - // ALSO rejects it — otherwise it is a valid JS pattern we route through - // the fancy fallback. `get_or_compile_regex` populates FANCY_CACHE when - // the regex crate fails but fancy-regex succeeds; check both here. - // - // PERF (#5777 follow-up): the ENTIRE validation block runs at most once - // per (pattern, flags). Regex validity is a pure function of the pair, so - // a pattern that has already cleared it can never fail it later; the - // cheap JS-syntax checks are not actually cheap - // (`has_invalid_repeated_quantifier` does a - // `pattern.chars().collect::>()` — a ~51 KB allocation for a - // 12,807-char pattern — plus an O(n) scan on EVERY `new RegExp(...)`), - // and the common `string-width`/`emoji-regex` npm packages construct a - // fresh ~12,807-char `/…/g` literal on every measurement, which a layout - // pass calls thousands of times. #5777 keyed that skip off a REGEX_CACHE - // hit, which worked only because construction also COMPILED; with the - // build deferred, the fact is recorded directly in `VALIDATED_PATTERNS`. - { - if !validated_hit { - if has_invalid_repeated_quantifier(pattern_str) { - throw_regexp_syntax_error(&format!( - "Invalid regular expression: /{}/: invalid pattern", - pattern_str - )); - } - // `--` is the real ClassSetExpression subtraction operator under - // the `v` flag (UTS #51) — `[a--z]` there means "a minus z", not - // a malformed range — so only legacy/`u`-mode patterns are - // subject to the doubled-hyphen range-order check. - if !flags_str.contains('v') && has_out_of_order_double_dash_class_range(pattern_str) { - throw_regexp_syntax_error(&format!( - "Invalid regular expression: /{}/: invalid pattern", - pattern_str - )); - } - // Annex B.1.4 legacy escapes (`\1` non-backref octal, `\0DD`, `\8`/`\9`, - // `\c` without a control letter) are accepted in sloppy patterns but are - // a hard SyntaxError under the `/u` (and `/v`) flag — `js_regex_to_rust` - // would otherwise silently relax them. (test262 RegExp/ - // unicode_restricted_octal_escape + unicode_restricted_identity_escape_c) - if unicode && has_unicode_forbidden_legacy_escape(pattern_str) { - throw_regexp_syntax_error(&format!( - "Invalid regular expression: /{}/: invalid pattern", - pattern_str - )); - } - // The remaining Annex B.1.4 leniencies (lone `]`/`}`, incomplete `{` - // quantifiers, `\d`-style range endpoints, quantified lookarounds, and - // forbidden IdentityEscapes) are likewise hard errors under `/u`. Gated - // on `u` specifically — `/v`'s ClassSetExpression grammar differs. - if flags_str.contains('u') && has_unicode_forbidden_pattern(pattern_str) { - throw_regexp_syntax_error(&format!( - "Invalid regular expression: /{}/: invalid pattern", - pattern_str - )); + // #2829: reject duplicate/unknown flags (SyntaxError) and store the + // canonical sorted form so `.flags` reflects Node's ordering. + 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 flags_are_canonical = raw_flags_str == flags_str; + let shared_flags_root = + (is_valid_ptr(flags) && flags_are_canonical).then(|| scope.root_string_ptr(flags)); + // Materialized HERE, while `raw_flags_str`'s borrow of the caller's + // GC string is still guaranteed live: the validation block below + // can allocate, and the site record is written after it. + let raw_flags_owned: Arc = Arc::from(raw_flags_str); + + let case_insensitive = flags_str.contains('i'); + let global = flags_str.contains('g'); + let multiline = flags_str.contains('m'); + let sticky = flags_str.contains('y'); + let dot_all = flags_str.contains('s'); + let unicode = flags_str.contains('u') || flags_str.contains('v'); + let has_indices = flags_str.contains('d'); + + // Content-keyed construction cache (`regex::site_cache`): a verified hit + // means this exact `(pattern, canonical flags)` already cleared the + // validation below — validity is a pure function of the pair — and hands + // back the shared owned copies plus, once some header built from this + // text has been executed, its compiled programs. The probe is one + // fingerprint and one byte compare; everything below it that copies or + // hashes the pattern is skipped. + let site_hit = site_cache::lookup(pattern_str, flags_str); + let validated_hit = + site_hit.is_some() || lazy::pattern_already_validated(pattern_str, flags_str); + if crate::hot_diag::regex_on() { + crate::hot_diag::regex_with(|d| { + d.note_new( + pattern as usize, + pattern_str.as_bytes(), + flags_str, + validated_hit && site_hit.is_none(), + site_hit.is_some(), + ) + }); } - // The remaining question — "is this a SyntaxError?" — used to be - // answered by BUILDING the pattern, which is why constructing a - // regex cost an NFA. Ask the standard engine's PARSER instead - // (`lazy::std_engine_syntax_ok`, the same `regex_syntax` parse - // `build_std_regex` performs, on the same string): 17.8x cheaper, - // and it agrees with the full build on every one of the 2,378 - // regex literals in the claude-code bundle (asserted over a - // corpus by `tests::syntax_check_agrees_with_full_build`). + + // #2829: reject invalid pattern syntax with a SyntaxError. A pattern the + // `regex` crate rejects is only a real error if `fancy-regex` (which + // covers the full JS feature set: lookbehind/lookahead/backreferences) + // ALSO rejects it — otherwise it is a valid JS pattern we route through + // the fancy fallback. `get_or_compile_regex` populates FANCY_CACHE when + // the regex crate fails but fancy-regex succeeds; check both here. // - // A parser rejection is NOT a verdict: every lookbehind / - // backreference pattern is rejected by the linear engine too. Fall - // through to the unchanged both-engines path, which owns the - // SyntaxError decision and populates the caches for the fancy - // fallback. - if !lazy::std_engine_syntax_ok(pattern_str, flags_str) + // PERF (#5777 follow-up): the ENTIRE validation block runs at most once + // per (pattern, flags). Regex validity is a pure function of the pair, so + // a pattern that has already cleared it can never fail it later; the + // cheap JS-syntax checks are not actually cheap + // (`has_invalid_repeated_quantifier` does a + // `pattern.chars().collect::>()` — a ~51 KB allocation for a + // 12,807-char pattern — plus an O(n) scan on EVERY `new RegExp(...)`), + // and the common `string-width`/`emoji-regex` npm packages construct a + // fresh ~12,807-char `/…/g` literal on every measurement, which a layout + // pass calls thousands of times. #5777 keyed that skip off a REGEX_CACHE + // hit, which worked only because construction also COMPILED; with the + // build deferred, the fact is recorded directly in `VALIDATED_PATTERNS`. + { + if !validated_hit { + if has_invalid_repeated_quantifier(pattern_str) { + throw_regexp_syntax_error(&format!( + "Invalid regular expression: /{}/: invalid pattern", + pattern_str + )); + } + // `--` is the real ClassSetExpression subtraction operator under + // the `v` flag (UTS #51) — `[a--z]` there means "a minus z", not + // a malformed range — so only legacy/`u`-mode patterns are + // subject to the doubled-hyphen range-order check. + if !flags_str.contains('v') + && has_out_of_order_double_dash_class_range(pattern_str) + { + throw_regexp_syntax_error(&format!( + "Invalid regular expression: /{}/: invalid pattern", + pattern_str + )); + } + // Annex B.1.4 legacy escapes (`\1` non-backref octal, `\0DD`, `\8`/`\9`, + // `\c` without a control letter) are accepted in sloppy patterns but are + // a hard SyntaxError under the `/u` (and `/v`) flag — `js_regex_to_rust` + // would otherwise silently relax them. (test262 RegExp/ + // unicode_restricted_octal_escape + unicode_restricted_identity_escape_c) + if unicode && has_unicode_forbidden_legacy_escape(pattern_str) { + throw_regexp_syntax_error(&format!( + "Invalid regular expression: /{}/: invalid pattern", + pattern_str + )); + } + // The remaining Annex B.1.4 leniencies (lone `]`/`}`, incomplete `{` + // quantifiers, `\d`-style range endpoints, quantified lookarounds, and + // forbidden IdentityEscapes) are likewise hard errors under `/u`. Gated + // on `u` specifically — `/v`'s ClassSetExpression grammar differs. + if flags_str.contains('u') && has_unicode_forbidden_pattern(pattern_str) { + throw_regexp_syntax_error(&format!( + "Invalid regular expression: /{}/: invalid pattern", + pattern_str + )); + } + // The remaining question — "is this a SyntaxError?" — used to be + // answered by BUILDING the pattern, which is why constructing a + // regex cost an NFA. Ask the standard engine's PARSER instead + // (`lazy::std_engine_syntax_ok`, the same `regex_syntax` parse + // `build_std_regex` performs, on the same string): 17.8x cheaper, + // and it agrees with the full build on every one of the 2,378 + // regex literals in the claude-code bundle (asserted over a + // corpus by `tests::syntax_check_agrees_with_full_build`). + // + // A parser rejection is NOT a verdict: every lookbehind / + // backreference pattern is rejected by the linear engine too. Fall + // through to the unchanged both-engines path, which owns the + // SyntaxError decision and populates the caches for the fancy + // fallback. + if !lazy::std_engine_syntax_ok(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 @@ -1125,49 +1255,91 @@ pub extern "C" fn js_regexp_new( && !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 - // compiles bare but blows the size limit with the flag prefix - // must stay a silent never-match (matching prior behavior), - // not a SyntaxError. - let translated = js_regex_to_rust(pattern_str); - if build_std_regex(&translated).is_err() && build_fancy_regex(&translated).is_err() - { - throw_regexp_syntax_error(&format!( - "Invalid regular expression: /{}/: invalid pattern", - pattern_str - )); + ) { + // Preserve the historical edge: validation used to test the + // BARE translated pattern (no `(?ims)` prefix). A pattern that + // compiles bare but blows the size limit with the flag prefix + // must stay a silent never-match (matching prior behavior), + // not a SyntaxError. + let translated = js_regex_to_rust(pattern_str); + if build_std_regex(&translated).is_err() + && build_fancy_regex(&translated).is_err() + { + throw_regexp_syntax_error(&format!( + "Invalid regular expression: /{}/: invalid pattern", + pattern_str + )); + } + } + lazy::mark_pattern_validated(pattern_str, flags_str); } } - lazy::mark_pattern_validated(pattern_str, flags_str); - } - } - // The compiled program is NOT built here. Validation above has already - // established that the pattern is legal, and a bundle evaluates hundreds - // of module-level literals it never matches with — building each one's - // NFA at construction is what put ~14% of a claude-code `--help` run - // inside `regex_syntax`/`regex_automata`. `regex_ptr` stays null (the - // "not built yet" state) and `lazy::ensure_regex_compiled` installs the - // owned `Arc`s on the first operation that needs a matcher. - - // ★ Last use of the borrowed pattern text before this function allocates. - // `pattern_str` borrows the GC string; the two allocations below can move - // it, and everything after this point reads the pattern from `owned_pattern` - // (a shared `Arc`, which relocation cannot invalidate) or from - // `pattern_root` (a runtime handle the collector rewrites). Nothing below - // may use `pattern_str` or the incoming `pattern` argument again. - let (owned_pattern, owned_flags, programs) = match site_hit { - Some(hit) => (hit.pattern, hit.flags, hit.programs), - None => { - let (p, f) = site_cache::insert(pattern_str, flags_str); - (p, f, None) + // The compiled program is NOT built here. Validation above has already + // established that the pattern is legal, and a bundle evaluates hundreds + // of module-level literals it never matches with — building each one's + // NFA at construction is what put ~14% of a claude-code `--help` run + // inside `regex_syntax`/`regex_automata`. `regex_ptr` stays null (the + // "not built yet" state) and `lazy::ensure_regex_compiled` installs the + // owned `Arc`s on the first operation that needs a matcher. + + // ★ Last use of the borrowed pattern text before this function allocates. + // `pattern_str` borrows the GC string; the two allocations below can move + // it, and everything after this point reads the pattern from `owned_pattern` + // (a shared `Arc`, which relocation cannot invalidate) or from + // `pattern_root` (a runtime handle the collector rewrites). Nothing below + // may use `pattern_str` or the incoming `pattern` argument again. + let (owned_pattern, owned_flags, programs) = match site_hit { + Some(hit) => (hit.pattern, hit.flags, hit.programs), + None => { + let (p, f) = site_cache::insert(pattern_str, flags_str); + (p, f, None) + } + }; + #[allow(unused_variables)] + let pattern_str: () = (); + + // Record what this construction established, so every later + // evaluation of this literal answers from the site key. Only ever + // written on the path that has already validated the pair — a + // hit legitimately skips validation because validity is a pure + // function of `(pattern, flags)`. + let bits = site_key::FlagBits { + case_insensitive, + global, + multiline, + sticky, + dot_all, + unicode, + has_indices, + }; + site_key::record( + site_key, + raw_flags_owned, + owned_pattern.clone(), + owned_flags.clone(), + flags_are_canonical, + bits, + programs.clone(), + ); + ( + owned_pattern, + owned_flags, + programs, + bits, + shared_flags_root, + ) } }; - #[allow(unused_variables)] - let pattern_str: () = (); + let site_key::FlagBits { + case_insensitive, + global, + multiline, + sticky, + dot_all, + unicode, + has_indices, + } = bits; // ★ The header is NURSERY-allocated, like an ordinary object. // @@ -1211,7 +1383,11 @@ pub extern "C" fn js_regexp_new( 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)) + // `owned_flags` IS the canonical text (the shared `Arc` the + // site or content cache handed back), and unlike `flags_str` it + // does not borrow the caller's GC string, so it is still valid + // here after the analysis above. + scope.root_string_ptr(js_string_from_str(&owned_flags)) } }; // ★ #7341: root the canonical flags string too. The header allocation below @@ -1275,20 +1451,52 @@ pub extern "C" fn js_regexp_new( // `runtime_write_barrier_gc_slot` classifies the parent and only // remembers genuinely-young children, so an already-old/interned // `pattern` is a harmless no-op. + // + // ★ Gated by the same live header test the COMPILER emits in front of + // every one of its own stores (`emit_parent_may_need_remembering_check`, + // #7511): a parent whose `GC_FLAG_TENURED` is clear owes the + // remembered set nothing, and a globally idle incremental barrier + // makes the SATB shading skippable too. Both clauses are read live — + // a header a collection promoted between `arena_alloc_gc` above and + // this store reads TENURED here and takes the full path, as does + // `RegExp.prototype.compile` reassigning a tenured header. + // + // Since #9845 the header is a NURSERY allocation, so on the common + // path both clauses are false and the pair of barrier calls — four + // page-map classifications, two dirty-page-cache probes and two child + // classifications, all ending at `ParentNotOldSkips` — collapses to + // one relaxed load of a static and one byte read of the header this + // function just wrote. `PERRY_REGEX_NEWBORN_BARRIER_GATE=0` restores + // the unconditional pair; nothing else changes with the gate off, so + // the OFF arm is the pre-change code path exactly. let regexp_parent_addr = ptr as usize; - if !pattern.is_null() { - crate::gc::runtime_write_barrier_gc_slot( - regexp_parent_addr, - std::ptr::addr_of!((*ptr).pattern_ptr) as usize, - js_nanbox_string(pattern as i64).to_bits(), - ); + let needs_barrier = !newborn_barrier_gate_enabled() + || crate::gc::newborn_parent_needs_barrier(regexp_parent_addr); + if crate::hot_diag::regex_on() { + crate::hot_diag::regex_counters(|d| { + if needs_barrier { + d.new_barrier_taken += 1; + } else { + d.new_barrier_gated += 1; + } + d.new_header_bytes += header_size as u64; + }); } - if !canonical_flags_ptr.is_null() { - crate::gc::runtime_write_barrier_gc_slot( - regexp_parent_addr, - std::ptr::addr_of!((*ptr).flags_ptr) as usize, - js_nanbox_string(canonical_flags_ptr as i64).to_bits(), - ); + if needs_barrier { + if !pattern.is_null() { + crate::gc::runtime_write_barrier_gc_slot( + regexp_parent_addr, + std::ptr::addr_of!((*ptr).pattern_ptr) as usize, + js_nanbox_string(pattern as i64).to_bits(), + ); + } + if !canonical_flags_ptr.is_null() { + crate::gc::runtime_write_barrier_gc_slot( + regexp_parent_addr, + std::ptr::addr_of!((*ptr).flags_ptr) as usize, + js_nanbox_string(canonical_flags_ptr as i64).to_bits(), + ); + } } (*ptr).case_insensitive = case_insensitive; (*ptr).global = global; @@ -1330,6 +1538,14 @@ pub extern "C" fn js_regexp_new( REGEX_POINTERS.with(|s| { s.borrow_mut().insert(ptr as usize); }); + if crate::hot_diag::regex_on() { + // Two address-keyed inserts per construction (this one and + // `REGEX_SOURCE_TABLE` below), each a `PtrHasher` hash plus a + // hashbrown insert, mirrored by two removals at death and two + // rekeys per evacuation. Counted so the pair is a number rather + // than a reading of the profile. + crate::hot_diag::regex_counters(|d| d.new_side_table_inserts += 2); + } // Issue #637: side-table owned copies of pattern + flags so // `.source` / `.flags` survive GC of the input StringHeaders. diff --git a/crates/perry-runtime/src/regex/site_cache.rs b/crates/perry-runtime/src/regex/site_cache.rs index b8e68af0da..bf0d3c1897 100644 --- a/crates/perry-runtime/src/regex/site_cache.rs +++ b/crates/perry-runtime/src/regex/site_cache.rs @@ -140,6 +140,16 @@ pub(super) fn lookup(pattern: &str, flags: &str) -> Option { for s in [slot, slot ^ 1] { if let Some(entry) = &cache[s] { if entry_matches(entry, fp, pattern, flags) { + // The verify is a FULL byte compare, so its cost is + // linear in the pattern and this counter — not + // `pattern_bytes`, which counts every construction + // whether it probed or not — is the `memcmp` volume. + // Counted at the construction probe only; `insert` and + // `install_programs` verify too and are not counted here. + if crate::hot_diag::regex_on() { + let n = pattern.len() as u64; + crate::hot_diag::regex_counters(|d| d.new_site_verify_bytes += n); + } return Some(Hit { pattern: entry.pattern.clone(), flags: entry.flags.clone(), diff --git a/crates/perry-runtime/src/regex/site_key.rs b/crates/perry-runtime/src/regex/site_key.rs new file mode 100644 index 0000000000..aff7f4f028 --- /dev/null +++ b/crates/perry-runtime/src/regex/site_key.rs @@ -0,0 +1,417 @@ +//! Literal-site keyed construction cache for `RegExp` — O(1), no hashing, no +//! byte compare. +//! +//! # Why this exists next to `site_cache` +//! +//! [`super::site_cache`] answers "have I seen this pattern TEXT before?" and +//! is what a dynamic `new RegExp(s)` needs. It is keyed by a content +//! fingerprint and, because a fingerprint can collide, every hit is verified +//! by a **full byte compare of the pattern**. That verify is linear in the +//! pattern, and a regex literal evaluates to a fresh object every time it is +//! reached: on claude-code the segment loop constructs `string-width`'s +//! ~12,807-character `/…/g` once per grapheme, so the verify alone is ~2.0 GB +//! of `memcmp` per 400-character reply and 39.6 % of `js_regexp_new`'s own +//! profile subtree. +//! +//! A literal does not need to be identified by its text. It is one source +//! site, and its pattern and flags are fixed at compile time. The compiler now +//! says so: `Expr::RegExp` emits an 8-byte private global per literal site and +//! passes its ADDRESS as `site_key` (`expr/logical_collections.rs`), and +//! [`js_regexp_new_site`](super::js_regexp_new_site) probes this table with +//! it. +//! +//! # Why the key is sound, and why the string handles are not +//! +//! Identity by address is only sound while the address cannot be reused for +//! something else. A `StringHeader` address fails that twice over — headers +//! are GC-managed, so an address is freed and reused, and a moving collector +//! relocates them — which is why the earlier analysis of this problem +//! concluded no sound string identity was available and left the content +//! compare in place. +//! +//! A per-site global has neither problem: it is emitted by the compiler into +//! the image, never freed, never moved, and distinct sites are distinct +//! globals and therefore distinct addresses. So an entry is verified by +//! comparing ONE WORD, and the pattern is never read at all — not hashed, not +//! fingerprinted, not compared. +//! +//! What that leaves per construction on a hit: two `Arc` refcount bumps for +//! the shared `(pattern, flags)` text, the program handles if the site has +//! been executed once, and the header allocation itself. No validation (the +//! first construction at this site did it, and validity is a pure function of +//! the pair), no flag canonicalization, no fingerprint, no `memcmp`. +//! +//! Kill switch: `PERRY_REGEX_SITE_KEY=0` (every probe misses and nothing is +//! recorded, so the construction falls through to the content-keyed path +//! exactly as before this existed). + +use std::cell::RefCell; +use std::sync::{Arc, Weak}; + +use super::site_cache::Programs; + +/// The site entry's view of a pattern's compiled programs: **weak**, so the +/// table can hand them out but can never be the reason they stay alive. +/// +/// Measured cost of holding them strongly (cc, one 3300-char reply): settled +/// footprint 478/474 MB → 500/527 MB and idle CPU 2.37 → 2.68 s. The site +/// table is 1,024 entries and a compiled program is ~19 KB, so a table that +/// outlives the content cache's own eviction retains programs nothing else +/// wants. The campaign's directive is both metrics together, and a CPU win +/// bought with resident memory does not land. +/// +/// Strong references remain where they belong: the `(pattern, flags)` program +/// caches, and every live header that installed them via `Arc::into_raw`. A +/// site entry whose programs have been dropped simply reports "not built +/// yet", and the next construction re-picks them up from the content cache — +/// the same path the site's very first construction takes. +struct WeakPrograms { + std: Weak<::regex::Regex>, + fancy: Option>, + repeat: Option>, +} + +impl WeakPrograms { + fn downgrade(programs: &Programs) -> Self { + Self { + std: Arc::downgrade(&programs.std), + fancy: programs.fancy.as_ref().map(Arc::downgrade), + repeat: programs.repeat.as_ref().map(Arc::downgrade), + } + } + + /// ALL-OR-NOTHING. A header must carry **every** program its pattern needs + /// — that is #9801's coherence rule, and a partial upgrade is exactly the + /// incoherent triple it fixed: a standard program installed beside a + /// missing fancy fallback silently never-matches instead of falling back. + /// So a single dead reference makes the whole entry report unbuilt. + fn upgrade(&self) -> Option { + let std = self.std.upgrade()?; + let fancy = match &self.fancy { + None => None, + Some(weak) => Some(weak.upgrade()?), + }; + let repeat = match &self.repeat { + None => None, + Some(weak) => Some(weak.upgrade()?), + }; + Some(Programs { std, fancy, repeat }) + } +} + +/// The flag bits `js_regexp_new` derives from the canonical flags text. They +/// are a pure function of the site's flags literal, so a hit reads them +/// instead of re-scanning the string seven times. +#[derive(Clone, Copy)] +pub(super) struct FlagBits { + pub(super) case_insensitive: bool, + pub(super) global: bool, + pub(super) multiline: bool, + pub(super) sticky: bool, + pub(super) dot_all: bool, + pub(super) unicode: bool, + pub(super) has_indices: bool, +} + +struct Entry { + key: usize, + /// The caller's flags text VERBATIM, as the site spells it. Compared on + /// every probe: flags are at most eight bytes, so the check is free, and + /// it makes the entry exact for a caller that is not the emitted lowering + /// (`/x/ig` and `/x/gi` are two spellings of one canonical form and must + /// not answer for each other's `flags_are_canonical`). + raw_flags: Arc, + pattern: Arc, + flags: Arc, + /// The caller's flags string already IS the canonical text, so the header + /// can share it instead of materializing a GC string (#9819). A property + /// of the site: the author wrote `/x/gi` or `/x/ig` once. + flags_are_canonical: bool, + bits: FlagBits, + programs: Option, +} + +/// What a construction gets back on a site hit. +pub(super) struct SiteHit { + pub(super) pattern: Arc, + pub(super) flags: Arc, + pub(super) flags_are_canonical: bool, + pub(super) bits: FlagBits, + pub(super) programs: Option, +} + +/// Direct-mapped, 2-way (a key may live in `slot` or `slot ^ 1`). A bundle's +/// live literal working set is small — claude-code holds 2,935 distinct +/// patterns across ~2,378 literal sites and a render cycles through a few +/// dozen. +const SLOTS: usize = 1024; + +crate::perry_thread_local! { + static SITE_KEY_TABLE: RefCell>> = RefCell::new(Vec::new()); +} + +fn enabled() -> bool { + static ON: std::sync::OnceLock = std::sync::OnceLock::new(); + *ON.get_or_init(|| { + crate::gc::env_default_on_from_value(std::env::var("PERRY_REGEX_SITE_KEY").ok().as_deref()) + }) +} + +/// The site global is 8-byte aligned, so the low three bits carry no +/// information; shift them out before masking. No hash — the key is already a +/// unique identity, and hashing it would be the cost this table exists to +/// remove. +#[inline] +fn slot_of(key: usize) -> usize { + (key >> 3) & (SLOTS - 1) +} + +/// The entry recorded for `key`, or `None`. +pub(super) fn lookup(key: usize, raw_flags: &str) -> Option { + if !enabled() || key == 0 { + return None; + } + let slot = slot_of(key); + SITE_KEY_TABLE.with(|table| { + let table = table.borrow(); + if table.is_empty() { + return None; + } + for s in [slot, slot ^ 1] { + if let Some(entry) = &table[s] { + if entry.key == key && &*entry.raw_flags == raw_flags { + return Some(SiteHit { + pattern: entry.pattern.clone(), + flags: entry.flags.clone(), + flags_are_canonical: entry.flags_are_canonical, + bits: entry.bits, + programs: entry.programs.as_ref().and_then(WeakPrograms::upgrade), + }); + } + } + } + None + }) +} + +/// Record what the first construction at `key` established. Callers must pass +/// the validated, canonical values — an entry is only ever written on the path +/// that has already validated the pair. +pub(super) fn record( + key: usize, + raw_flags: Arc, + pattern: Arc, + flags: Arc, + flags_are_canonical: bool, + bits: FlagBits, + programs: Option, +) { + if !enabled() || key == 0 { + return; + } + let slot = slot_of(key); + SITE_KEY_TABLE.with(|table| { + let mut table = table.borrow_mut(); + if table.is_empty() { + table.resize_with(SLOTS, || None); + } + for s in [slot, slot ^ 1] { + if let Some(entry) = &mut table[s] { + if entry.key == key && entry.raw_flags == raw_flags { + // Refresh a reference whose programs have been dropped, + // rather than only filling an empty one: a dead weak and + // an absent entry mean the same thing here, and the + // former must be able to heal. + if let Some(programs) = &programs { + if entry + .programs + .as_ref() + .and_then(WeakPrograms::upgrade) + .is_none() + { + entry.programs = Some(WeakPrograms::downgrade(programs)); + } + } + return; + } + } + } + let victim = if table[slot].is_none() { + slot + } else if table[slot ^ 1].is_none() { + slot ^ 1 + } else { + // Both ways taken by other sites: evict the primary. A site whose + // entry is evicted simply falls back to the content-keyed path, + // which is correct and merely slower. + slot + }; + table[victim] = Some(Entry { + key, + raw_flags, + pattern, + flags, + flags_are_canonical, + bits, + programs: programs.as_ref().map(WeakPrograms::downgrade), + }); + }); +} + +/// Attach the programs the first execution built, so later constructions at +/// this site are born built. A no-op when the site was evicted meanwhile. +pub(super) fn install_programs(key: usize, programs: Programs) { + if !enabled() || key == 0 { + return; + } + let slot = slot_of(key); + SITE_KEY_TABLE.with(|table| { + let mut table = table.borrow_mut(); + if table.is_empty() { + return; + } + for s in [slot, slot ^ 1] { + if let Some(entry) = &mut table[s] { + if entry.key == key + && entry + .programs + .as_ref() + .and_then(WeakPrograms::upgrade) + .is_none() + { + entry.programs = Some(WeakPrograms::downgrade(&programs)); + return; + } + } + } + }); +} + +#[cfg(test)] +pub(super) fn test_reset() { + SITE_KEY_TABLE.with(|table| table.borrow_mut().clear()); +} + +/// The pattern text this site is recorded under, or `None`. Test-only: the +/// probe that lets a sabotage of `slot_of`/the key comparison be caught by a +/// test that constructs two different literals at two colliding sites. +/// +/// Takes the key in the **emitted lowering's type** (`i64`, what +/// `Expr::RegExp`'s `ptrtoint` produces and what the `js_regexp_new_site` +/// extern declares) and narrows it here, so a test holds exactly the value the +/// compiler passes and crosses the same `as usize` boundary the product entry +/// point does. The table itself is keyed by `usize` because the key IS an +/// address; the two spellings meet at the FFI edge and nowhere else. +#[cfg(test)] +pub(super) fn test_recorded_pattern(key: i64, raw_flags: &str) -> Option { + lookup(key as usize, raw_flags).map(|hit| hit.pattern.to_string()) +} + +/// How many slots hold an entry. Test-only: proves a dynamic +/// `new RegExp(str)` did NOT record anything. +#[cfg(test)] +pub(super) fn test_occupied_slots() -> usize { + SITE_KEY_TABLE.with(|table| table.borrow().iter().filter(|e| e.is_some()).count()) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// **The all-or-nothing rule, made able to fail.** + /// + /// #9801 fixed an incoherent triple — a standard program memoized beside a + /// missing fancy fallback — which does not error: it silently never + /// matches. Holding the site entry's programs weakly reintroduces exactly + /// that shape unless a dead reference invalidates the WHOLE entry, because + /// the three `Arc`s have independent lifetimes and the fancy fallback is + /// the one a pattern the linear engine refused depends on. + /// + /// A sabotage that upgrades each field independently — the natural way to + /// write it — returns `Some(Programs { std, fancy: None, .. })` here and + /// fails on the second assertion. + #[test] + fn one_dead_reference_invalidates_the_whole_entry() { + let std_program = Arc::new(::regex::Regex::new("a(b)c").expect("linear program")); + let fancy_program = Arc::new(::fancy_regex::Regex::new("a(?=b)c").expect("fancy program")); + let programs = Programs { + std: std_program.clone(), + fancy: Some(fancy_program.clone()), + repeat: None, + }; + let weak = WeakPrograms::downgrade(&programs); + drop(programs); + + let upgraded = weak + .upgrade() + .expect("both strong references are still held here"); + assert!( + upgraded.fancy.is_some(), + "the fancy fallback must survive the round trip while its Arc is alive" + ); + drop(upgraded); + + // Only the FANCY program dies. The standard one is still strongly held. + drop(fancy_program); + assert!( + weak.upgrade().is_none(), + "one dead reference must invalidate the whole entry — handing back a header with a \ + standard program and no fancy fallback is #9801's incoherent triple, which never \ + matches instead of failing" + ); + drop(std_program); + assert!(weak.upgrade().is_none()); + } + + /// The table must not be the reason a program stays alive: once nothing + /// else holds it, a recorded entry reports "not built yet" and the next + /// construction re-picks it up from the content cache. + #[test] + fn the_site_table_does_not_keep_a_program_alive() { + test_reset(); + let key = 0x5171_E000_usize; + let std_program = Arc::new(::regex::Regex::new("keepalive").expect("linear program")); + let programs = Programs { + std: std_program.clone(), + fancy: None, + repeat: None, + }; + record( + key, + Arc::from("g"), + Arc::from("keepalive"), + Arc::from("g"), + true, + FlagBits { + case_insensitive: false, + global: true, + multiline: false, + sticky: false, + dot_all: false, + unicode: false, + has_indices: false, + }, + Some(programs), + ); + assert!( + lookup(key, "g") + .expect("the entry was just recorded") + .programs + .is_some(), + "precondition: the entry answers with its programs while they are alive" + ); + + drop(std_program); + let hit = lookup(key, "g").expect("the entry itself survives"); + assert!( + hit.programs.is_none(), + "the site table holds programs WEAKLY: with every other reference gone the entry must \ + report unbuilt rather than keeping ~19 KB per slot alive on its own" + ); + assert_eq!( + &*hit.pattern, "keepalive", + "the entry's identity is unaffected — only its programs expire" + ); + test_reset(); + } +} diff --git a/crates/perry-runtime/src/regex/tests.rs b/crates/perry-runtime/src/regex/tests.rs index cc5908b41e..76aad4a12d 100644 --- a/crates/perry-runtime/src/regex/tests.rs +++ b/crates/perry-runtime/src/regex/tests.rs @@ -1948,3 +1948,174 @@ fn a_regexp_with_a_non_writable_lastindex_is_still_found_by_the_probe() { "an unrelated key on the same RegExp must still take the fast negative" ); } + +// --------------------------------------------------------------------------- +// Literal-site keyed construction (`js_regexp_new_site` + `regex::site_key`). +// +// The site key is the address of a private global the compiler emits once per +// regex literal. These statics stand in for two such globals: distinct +// addresses, 8-byte aligned, immortal — the same three properties the emitted +// ones have, and the reason the key is a sound identity where a `StringHeader` +// address is not. +// --------------------------------------------------------------------------- + +// Distinct initialisers so nothing may merge them: the test uses only their +// ADDRESSES, and identical zero-valued statics are exactly the shape a +// constant-merging pass is allowed to collapse. `assert_ne!` on the two keys +// keeps that from being a silent assumption. +static SITE_SLOT_A: u64 = 0xA; +static SITE_SLOT_B: u64 = 0xB; +static SITE_SLOT_C: u64 = 0xC; + +fn site_key_of(slot: &'static u64) -> i64 { + slot as *const u64 as i64 +} + +/// **The sabotage the coordinator named: key the table by pattern length.** +/// +/// Two literals at two sites, same flags, same pattern LENGTH, different text. +/// A table that verifies a probe by anything weaker than the site key — a +/// length, a prefix, a fingerprint without the exactness check — hands the +/// second site the first site's entry, and `.source` then reports a pattern +/// this literal never contained while `test` matches the wrong language. +/// +/// Each site is constructed twice: the first construction records, the second +/// is the one that must come back from the table, which is the case a weak key +/// breaks. Asserting only on a first construction would pass under every +/// sabotage, because a miss always takes the content-keyed path. +#[test] +fn two_literal_sites_with_equal_length_patterns_never_answer_for_each_other() { + let _lock = crate::gc::global_side_table_test_lock(); + site_key::test_reset(); + + let key_a = site_key_of(&SITE_SLOT_A); + let key_b = site_key_of(&SITE_SLOT_B); + assert_ne!(key_a, key_b, "two sites must have two addresses"); + + for round in 0..2 { + let a = js_regexp_new_site(make_string("a.c"), make_string("g"), key_a); + let b = js_regexp_new_site(make_string("x.z"), make_string("g"), key_b); + assert_eq!( + string_payload(js_regexp_get_source(a)), + b"a.c".to_vec(), + "round {round}: site A must report its own pattern" + ); + assert_eq!( + string_payload(js_regexp_get_source(b)), + b"x.z".to_vec(), + "round {round}: site B must report its own pattern — a table keyed by anything \ + weaker than the site address hands B the entry A recorded, and both patterns are \ + three bytes long" + ); + assert!( + js_regexp_test(a, make_string("abc")) != 0 + && js_regexp_test(a, make_string("xyz")) == 0, + "round {round}: site A must match its own language" + ); + assert!( + js_regexp_test(b, make_string("xyz")) != 0 + && js_regexp_test(b, make_string("abc")) == 0, + "round {round}: site B must match its own language" + ); + } + + assert_eq!( + site_key::test_recorded_pattern(key_a, "g").as_deref(), + Some("a.c") + ); + assert_eq!( + site_key::test_recorded_pattern(key_b, "g").as_deref(), + Some("x.z") + ); +} + +/// A dynamic `new RegExp(str)` must never reach the site table. +/// +/// The two-argument entry point is what every non-literal construction uses — +/// `js_regexp_construct`, `RegExp.prototype.compile`, the runtime's own +/// callers — and it has no site to be keyed by. If it recorded under some +/// stand-in key, a later literal whose key collided would inherit a pattern +/// that a *variable* produced, which is the one thing the site key's +/// compile-time-constant argument is supposed to guarantee against. +#[test] +fn a_dynamic_construction_records_nothing_in_the_site_table() { + let _lock = crate::gc::global_side_table_test_lock(); + site_key::test_reset(); + + for _ in 0..4 { + let re = js_regexp_new(make_string("dyn(amic)"), make_string("g")); + assert!(js_regexp_test(re, make_string("dynamic")) != 0); + } + assert_eq!( + site_key::test_occupied_slots(), + 0, + "the two-argument entry point has no site key and must record nothing" + ); + + // The same TEXT through the site entry does record — so the zero above is + // a property of the entry point, not of a table that never works. + let key = site_key_of(&SITE_SLOT_C); + let re = js_regexp_new_site(make_string("dyn(amic)"), make_string("g"), key); + assert!(js_regexp_test(re, make_string("dynamic")) != 0); + assert_eq!( + site_key::test_occupied_slots(), + 1, + "the site entry point must record — otherwise the assertion above proves nothing" + ); + assert_eq!( + site_key::test_recorded_pattern(key, "g").as_deref(), + Some("dyn(amic)") + ); +} + +/// A site hit must be born built: the second construction at a site whose +/// first header has already executed installs the compiled programs eagerly, +/// so `regex_ptr` is non-null before any match runs. +/// +/// This is what makes the fast path complete — a hit that skipped the content +/// cache but arrived unbuilt would push the pattern's hash back onto the first +/// `test()` and give the site key nothing. +#[test] +fn a_site_hit_after_the_first_execution_is_born_built() { + let _lock = crate::gc::global_side_table_test_lock(); + site_key::test_reset(); + let key = site_key_of(&SITE_SLOT_A); + + let first = js_regexp_new_site(make_string("bo+rn"), make_string(""), key); + assert!( + unsafe { (*first).regex_ptr }.is_null(), + "construction must not build the program (that is #5777's deferred build)" + ); + assert!(js_regexp_test(first, make_string("boorn")) != 0); + assert!( + !unsafe { (*first).regex_ptr }.is_null(), + "the first execution installs the programs" + ); + + // Second construction at the SAME site. + let second = js_regexp_new_site(make_string("bo+rn"), make_string(""), key); + assert!( + !unsafe { (*second).regex_ptr }.is_null(), + "a site hit must install the programs the site already compiled, so the header is born \ + built and the first match pays no lookup" + ); + assert_ne!(first, second, "each evaluation is still a distinct object"); + assert!(js_regexp_test(second, make_string("born")) != 0); +} + +/// The kill switch has to remove the lane, not just its answers: with +/// `PERRY_REGEX_SITE_KEY=0` the table records nothing, so the OFF arm is the +/// content-keyed path exactly rather than a control still paying the +/// bookkeeping. +/// +/// Read once per process through a `OnceLock`, so this asserts the DEFAULT is +/// on rather than flipping the variable mid-run (which would only test the +/// cache of the first read). +#[test] +fn the_site_key_lane_is_on_by_default() { + assert!( + crate::gc::env_default_on_from_value(None), + "the site-key lane defaults ON; `PERRY_REGEX_SITE_KEY=0` is the kill switch" + ); + assert!(!crate::gc::env_default_on_from_value(Some("0"))); +}