perf(regex): run the pre-search safepoint poll on one search in 64 (#10166)

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).

crates/perry-runtime/src/gc/tests/runtime_roots/perex_construction.rs

@@ -15,6 +15,61 @@ fn bytes(ptr: *const StringHeader) -> Vec<u8> {
             .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);

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<usize> = 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<usize> =
+        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<Match<'mem>>, Position),
@@ -336,10 +381,7 @@ fn find_near_lent<'mem, S: ImmutableSubject<Error = OwnerError>>(
             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),

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<usize> 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<usize> 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",