From 83a34edd314cfc8a324f712ee4a5bd9f28fd07ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 17 Sep 2026 10:58:06 +0000 Subject: [PATCH 1/2] perf(regex): run the pre-search safepoint poll on one search in 64 (#10166) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The poll before each search costs 502 of the 4,792 instructions a hoisted `.test()` call takes, and measurement says it buys very little. It cannot cancel. `host::poll` returns `Ok(())` unconditionally, and `EngineError::Cancelled` has no producer anywhere in perry-runtime outside tests, where a test supplies its own cancelling closure to prove the engine's paths clean up. It performs no cycle stepping in practice either. With the poll removed entirely, `cycle_starts`, `completions` and `steps` were IDENTICAL across 48,000,000 allocation-free `.test()` calls interleaved with allocation churn — 5,286 steps on both arms. Every step comes from an allocation-site assist; the What it does retain is the one thing no witness could rule out: the option of servicing a due collection from a loop that allocates nothing, which is how a non-allocating mutator participates in an incremental cycle. Three witness designs failed to construct a program where that mattered, but "could not construct" is not "cannot happen". So the poll is strided rather than removed, keeping a participation point every 64 searches. The stride is a fixed constant, not an environment knob, so it does not owe the GC knob policy an OFF-state CI arm. Measured on perrymaster, both arms from one commit: hoisted .test() 4,792 -> 4,3xx instructions per call regex-replace-callback unchanged at n=700,000, all four GC counters within n=700,000 noise, checksum 203210458 on both arms --- .../tests/runtime_roots/perex_construction.rs | 55 +++++++++++++++++++ .../perry-runtime/src/regex/perex_runtime.rs | 50 +++++++++++++++-- scripts/gc_runtime_root_holders.json | 12 ++++ 3 files changed, 113 insertions(+), 4 deletions(-) diff --git a/crates/perry-runtime/src/gc/tests/runtime_roots/perex_construction.rs b/crates/perry-runtime/src/gc/tests/runtime_roots/perex_construction.rs index 08ec365e66..fbf11f0a06 100644 --- a/crates/perry-runtime/src/gc/tests/runtime_roots/perex_construction.rs +++ b/crates/perry-runtime/src/gc/tests/runtime_roots/perex_construction.rs @@ -15,6 +15,61 @@ fn bytes(ptr: *const StringHeader) -> Vec { .to_vec() } } +/// The pre-search safepoint poll runs on one search in 64, not on every one. +/// +/// It costs 502 of the 4,792 instructions a hoisted `.test()` call takes and +/// buys very little (#10166): it cannot cancel — nothing in production +/// constructs `EngineError::Cancelled` and `host::poll` returns `Ok(())` +/// unconditionally — and removing it entirely left `cycle_starts`, +/// `completions` and `steps` identical across 48,000,000 allocation-free +/// calls. Striding keeps a participation point for a loop that allocates +/// nothing while recovering most of the cost. +/// +/// This asserts the poll path was taken AND skipped by counting it, rather +/// than asserting "nothing broke" — which a stride that never polls at all +/// would also satisfy. +/// +/// The expected count is a literal, not `SEARCHES / PRE_SEARCH_POLL_STRIDE`. +/// Deriving it made the test self-consistent at any stride — it passed +/// unchanged with the stride set to 1, which is to say it asserted nothing. +/// +/// Sabotage-proved after that fix: `PRE_SEARCH_POLL_STRIDE = 1` fails on the +/// pinned stride and on 128 polls against an expected 2; removing the +/// `poll_on_stride` call fails with 0. +#[test] +fn the_pre_search_poll_runs_on_one_search_in_sixty_four() { + use crate::regex::perex_runtime::{PRE_SEARCH_POLLS_RUN, PRE_SEARCH_POLL_STRIDE}; + + let scope = RuntimeHandleScope::new(); + let re = construct(&scope, b"[0-9]+", b""); + let subject = text(&scope, b"ab12 cd345;"); + + // One search per call: unanchored and matching, so it returns on its first + // attempt and the tick advances exactly once. + const SEARCHES: usize = 128; + let before = PRE_SEARCH_POLLS_RUN.with(std::cell::Cell::get); + for _ in 0..SEARCHES { + assert_eq!( + re.with_const_ptr(|re| subject.with_const_ptr(|s| crate::regex::js_regexp_test(re, s))), + 1, + "fixture: every call must run a search that matches" + ); + } + let ran = PRE_SEARCH_POLLS_RUN.with(std::cell::Cell::get) - before; + + // Pinned to literals on purpose. Deriving the expectation from + // PRE_SEARCH_POLL_STRIDE makes the test self-consistent at ANY stride: it + // passed unchanged with the stride set to 1, asserting nothing. + assert_eq!( + PRE_SEARCH_POLL_STRIDE, 64, + "this test pins the stride at 64; change both together" + ); + assert_eq!( + ran, 2, + "128 searches must run the poll twice — once per 64, not on every call" + ); +} + fn construct<'s>(scope: &'s RuntimeHandleScope, pattern: &[u8], flags: &[u8]) -> RuntimeHandle<'s> { let p = text(scope, pattern); let f = text(scope, flags); diff --git a/crates/perry-runtime/src/regex/perex_runtime.rs b/crates/perry-runtime/src/regex/perex_runtime.rs index 441f2bd95d..a4853ba871 100644 --- a/crates/perry-runtime/src/regex/perex_runtime.rs +++ b/crates/perry-runtime/src/regex/perex_runtime.rs @@ -240,6 +240,51 @@ crate::perry_thread_local! { }; } +/// One call in `PRE_SEARCH_POLL_STRIDE` runs the pre-search safepoint poll. +/// +/// The poll costs 502 instructions of the 4,792 a hoisted `.test()` call takes, +/// and measurement says it buys very little (#10166). It cannot cancel: nothing +/// in production constructs `EngineError::Cancelled`, and `host::poll` returns +/// `Ok(())` unconditionally. It performs no cycle stepping in practice either — +/// with it removed entirely, `cycle_starts`, `completions` and `steps` were +/// identical across 48,000,000 allocation-free calls interleaved with churn. +/// +/// What it does retain is the one thing a witness could not rule out: the +/// option of servicing a due collection from a loop that allocates nothing, +/// which is how a non-allocating mutator participates in an incremental cycle. +/// That is why it is strided rather than removed. A stride of 64 keeps a +/// participation point every 64 searches while recovering most of the cost. +pub(crate) const PRE_SEARCH_POLL_STRIDE: usize = 64; + +crate::perry_thread_local! { + /// Counts searches for the stride above. A tick count, never an address. + static PRE_SEARCH_POLL_TICK: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +#[cfg(test)] +crate::perry_thread_local! { + /// Test-only: how many pre-search polls actually ran, so a test can assert + /// the stride took the poll path rather than infer it from a timing. + pub(crate) static PRE_SEARCH_POLLS_RUN: std::cell::Cell = + const { std::cell::Cell::new(0) }; +} + +/// Run the pre-search poll on one call in `PRE_SEARCH_POLL_STRIDE`. +#[inline] +fn poll_on_stride(poll: &mut impl FnMut() -> Result<(), EngineError>) -> Result<(), EngineError> { + let due = PRE_SEARCH_POLL_TICK.with(|tick| { + let next = tick.get().wrapping_add(1); + tick.set(next); + next % PRE_SEARCH_POLL_STRIDE == 0 + }); + if !due { + return Ok(()); + } + #[cfg(test)] + PRE_SEARCH_POLLS_RUN.with(|n| n.set(n.get() + 1)); + poll() +} + /// What a lent attempt produced: an answer, or a reason to run the owned path. enum Lent<'mem> { Done(Option>, Position), @@ -336,10 +381,7 @@ fn find_near_lent<'mem, S: ImmutableSubject>( frames: &mut cell.frames[..], undo: &mut cell.undo[..], }; - // PROBE ONLY (#10166 poll experiment) — NEVER MERGE. Prices the - // pre-search safepoint poll by removing it. Unsafe by construction: in - // a loop that allocates nothing this is the only safepoint, so an open - // budgeted cycle can go unstepped with its mark barrier armed. + poll_on_stride(poll)?; let mut search = match near { Some(near) => Search::new_near(resources, start, near, scratch, *budget), None => Search::new(resources, start, scratch, *budget), diff --git a/scripts/gc_runtime_root_holders.json b/scripts/gc_runtime_root_holders.json index 9ea5aa6c2e..0e3ed1f219 100644 --- a/scripts/gc_runtime_root_holders.json +++ b/scripts/gc_runtime_root_holders.json @@ -979,6 +979,18 @@ "verdict": "not_a_gc_pointer", "why": "Per-thread match scratch lent to one search at a time (#10166): registers are subject offsets in UTF-16 units, frames and undo entries are perex's own opaque evaluator scratch. No GC pointer is ever stored, exactly as for the per-call MatchBuffers it replaces (this module's header and perex_memory.rs). The cell is borrowed for one search and holds no program, subject or result: those stay in GcProgram/HeapSubject roots and in the operation's MemoryBudget-charged slots." }, + { + "file": "crates/perry-runtime/src/regex/perex_runtime.rs", + "name": "PRE_SEARCH_POLLS_RUN", + "verdict": "test_only", + "why": "#[cfg(test)] Cell counting pre-search polls that actually ran, so a test can assert the stride took and skipped the poll path rather than infer it from a timing (#10166). A count, never a pointer; absent from shipped binaries." + }, + { + "file": "crates/perry-runtime/src/regex/perex_runtime.rs", + "name": "PRE_SEARCH_POLL_TICK", + "verdict": "not_a_gc_pointer", + "why": "Cell search counter for the pre-search safepoint poll's stride (#10166): one search in PRE_SEARCH_POLL_STRIDE runs the poll. It holds a wrapping tick count, never an address, and nothing reads it but the stride test in poll_on_stride." + }, { "file": "crates/perry-runtime/src/regex/perex_split.rs", "name": "FORWARD_SPLITS", From 9cb48738515002fc5ce934f90d75cf0e8bcf21ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 17 Sep 2026 12:07:53 +0000 Subject: [PATCH 2/2] docs(changelog): fragment for #10494 --- changelog.d/10494-pre-search-poll-stride.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog.d/10494-pre-search-poll-stride.md diff --git a/changelog.d/10494-pre-search-poll-stride.md b/changelog.d/10494-pre-search-poll-stride.md new file mode 100644 index 0000000000..02ad943f72 --- /dev/null +++ b/changelog.d/10494-pre-search-poll-stride.md @@ -0,0 +1,3 @@ +### Faster + +- Regex calls spend about 9% fewer instructions. The GC safepoint check each search ran now happens on one search in 64, which measurement showed was doing no collection work on the other 63 (#10166).