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

[proggeramlug]

Part of #10166. Ralph chose the stride and the value (64, a fixed constant rather than an env knob); the evidence behind that choice is recorded on the issue and summarised here.

What the poll was worth

The poll()? before each search in find_near_lent costs 502 of the 4,792 instructions a hoisted .test() call takes. Two things it was assumed to do, it does not:

• It cannot cancel. host::poll returns Ok(()) unconditionally, and EngineError::Cancelled has no producer anywhere in perry-runtime outside tests — every construction is a test supplying its own cancelling closure to prove the engine's paths release scratch and preserve consumed work.
• It performs no cycle stepping in practice. With the poll removed entirely, cycle_starts, completions and steps were identical — 5,286 on both arms — across 48,000,000 allocation-free .test() calls interleaved with allocation churn. Every step comes from an allocation-site assist; the perf(gc): make the "nothing due" GC check cheap on safepoint polls and trigger checks #10253 due-check answers "nothing due" and returns.

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, not removed, keeping a participation point every 64 searches.

Measurements

perrymaster, both arms from the same base (e6dcb6274d), perf stat -e instructions:u, string-building control subtracted, outputs identical on every row.

per call	before	after
hoisted re.test(v)	4,792	4,354	−438 (−9.1 %)
literal in loop	6,108	5,670	−438
re.exec(v) + m[2]	7,951	7,523	−428

438 of the available 502 recovered; the remaining 64 is the tick and branch.

The allocating arm is unchanged. regex-replace-callback at n=700,000, whole-program instruction counts because wall clock on this host swung 5,914–9,098 ms under load:

	baseline	stride
instructions	1,340,694,130,055	1,333,440,636,135 (−0.54 %)
cycle_starts	189	189
completions	189	189
steps	1,474,773	1,474,816
share_permille	674	679
checksum	203210458	203210458

Test

the_pre_search_poll_runs_on_one_search_in_sixty_four counts the polls that actually ran over 128 searches and asserts 2 — the path was taken and skipped, not merely "nothing broke".

The expected count is pinned to a literal on purpose. My first version derived it as SEARCHES / PRE_SEARCH_POLL_STRIDE, which made the test self-consistent at any stride: it passed unchanged with the stride set to 1, asserting nothing. That is recorded in the test's own comment so the next person does not reintroduce it.

Sabotage-proved after the fix, both directions run rather than reasoned:

sabotage	result
PRE_SEARCH_POLL_STRIDE = 1	fails the pinned stride (1 against 64)
remove the poll_on_stride call	fails the count (0 against 2)

Validation

check	result
cargo build --locked	OK
cargo fmt --all --check	OK
cargo check -p perry-runtime --no-default-features --features full, -D warnings	OK
cargo check -p perry --bins, -D warnings	OK
cargo test -p perry-runtime --lib -- --test-threads=1	3969 passed, 0 failed
scripts/gc_runtime_root_holders.py (+ --self-test)	OK — 1464 declarations, 407 classified
scripts/check_file_size.sh	OK
scripts/run_lint_gates.sh	1 of 83 FAILED

The single failure is Public benchmark evidence freshness, the long-standing CI-only red. Both new thread-locals carry holder verdicts: the tick is not_a_gc_pointer (a wrapping count), the test counter is test_only.

Scope

The stride is a fixed constant, so it owes the GC knob policy no OFF-state CI arm. Polls between quanta are untouched, so a long single search behaves exactly as before. This does not close #10166 — a hoisted .test() goes from 10.1× Node to 9.2×, against an acceptance bar of the pre-Perex engine's 1.25×, which is not reachable without the engine's own fixed per-search cost coming down.

Summary by CodeRabbit

• Performance

  • Regex searches now use approximately 9% fewer instructions.
  • Garbage-collection safepoint checks before searches run once every 64 searches, reducing unnecessary overhead while preserving periodic checks.
  • Repeated regex searches may experience improved efficiency with no change to search results.

• Tests

  • Added coverage verifying the scheduled safepoint behavior across repeated regex searches and ensuring checks continue to occur at the expected interval.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 98ff106d-2e41-4f9d-a07d-a19592b08421

📥 Commits

Reviewing files that changed from the base of the PR and between b519fec and 9cb4873.

📒 Files selected for processing (4)

• changelog.d/10494-pre-search-poll-stride.md
• crates/perry-runtime/src/gc/tests/runtime_roots/perex_construction.rs
• crates/perry-runtime/src/regex/perex_runtime.rs
• scripts/gc_runtime_root_holders.json

Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review.

---

📝 Walkthrough

Walkthrough

The regex runtime adds a strided pre-search safepoint poll for lent-scratch searches. Tests verify two polls across 128 searches. GC runtime-root metadata records the new counter and tick.

Changes

Pre-search polling

Layer / File(s)	Summary
Strided pre-search polling crates/perry-runtime/src/regex/perex_runtime.rs	Adds a stride constant, per-thread tick counter, test-only poll counter, and poll_on_stride helper. The lent-scratch search path now polls once every 64 searches.
Polling validation and census crates/perry-runtime/src/gc/tests/runtime_roots/perex_construction.rs, scripts/gc_runtime_root_holders.json, changelog.d/10494-pre-search-poll-stride.md	Adds a test for two polls across 128 searches, records the new runtime-root holders, and documents the measured performance change.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 9cb48

The bounded polling change has no established current-head correctness or GC-safety defect. It is ready to merge after normal checks.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR implements a scoped performance improvement for #10166. find_near_lent now calls poll_on_stride, with a fixed stride of 64. The new test verifies two polls across 128 searches. The source r…	Run and record the complete #10166 benchmark matrix on one host. Include literal, Unicode, and hoisted workloads for all sizes, with Node and pre-Perex comparisons. Update the public benchmark evidence and pass the freshness gate. Use those…
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (2 skipped: 2…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: changing the regex pre-search safepoint poll to run once every 64 searches.
Description check	✅ Passed	The description provides a detailed summary, rationale, related issue, implementation details, measurements, tests, validation results, and known lint failure. It does not use the template headings or…
Out of Scope Changes check	✅ Passed	The changed runtime code is confined to crates/perry-runtime/src/regex/perex_runtime.rs, which is the Perex runtime adapter. The added test directly verifies the new poll stride. The GC holder inven…

Full details: Linked Issues check

Explanation

The PR implements a scoped performance improvement for #10166. find_near_lent now calls poll_on_stride, with a fixed stride of 64. The new test verifies two polls across 128 searches. The source records a 438-instruction reduction for a hoisted .test() call. Reported runtime and GC-root checks support correctness and moving-GC safety. However, the evidence does not satisfy the issue's full benchmark requirements. It does not provide the required Node and pre-Perex comparisons for literal, Unicode, and hoisted workloads at all sizes. It does not establish the required performance ratios. The lint gate also reports a public benchmark evidence freshness failure.

Resolution

Run and record the complete #10166 benchmark matrix on one host. Include literal, Unicode, and hoisted workloads for all sizes, with Node and pre-Perex comparisons. Update the public benchmark evidence and pass the freshness gate. Use those results to demonstrate the required performance ratios.

Full details: Docstring Coverage

Explanation

Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (2 skipped: 2 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[proggeramlug]

Rebased onto 5030e6eed6 — and this PR now also removes a debug hack that reached main

New head 9cb4873851. The rebase conflicted on exactly one hunk, and the conflict is worth recording because of what it exposed.

main currently carries a "NEVER MERGE" probe of mine. At perex_runtime.rs:339 on 5030e6eed6:

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

The poll()? before Search::new in find_near_lent is gone from main. git log -S "PROBE ONLY" names the carrier: 1d8e3e21c6, the guard commit in #10412.

How it got there. I built the null probe by editing perex_runtime.rs and never committed it — it only had to exist for one measurement. Later I ran git checkout -B perf/10411-native-pieces origin/main with that edit still in the working tree; checkout carries uncommitted changes across branches. git add -A crates then swept it into an unrelated commit.

Nothing caught it because the removal is functionally invisible: 3969 tests pass either way, every gate is green, and a witness had already shown identical cycle_starts, completions and steps with the poll gone across 48M allocation-free calls. A commented hunk also reads as deliberate to a reviewer — the comment helped it survive rather than flagging it.

This PR is the fix. Resolving the conflict replaces the probe with poll_on_stride(poll)?, which restores a poll every 64 searches — the reviewed, measured intent. Verified after rebase: no PROBE ONLY or NEVER MERGE anywhere in crates/ on this branch, and the diff against main is 4 files, +116/−4, exactly the stride change plus the restoration.

Revalidated on the rebased head:

check	result
cargo build --locked	OK
cargo fmt --all --check	OK
cargo test -p perry-runtime --lib -- --test-threads=1	3970 passed, 0 failed
scripts/gc_runtime_root_holders.py (+ --self-test)	OK — 1465 declarations, 408 classified
scripts/run_lint_gates.sh	1 of 83 (public-baseline, the standing CI-only red)

Ralph's call was no revert: land the stride rather than restoring the unconditional poll first.

[proggeramlug]

Landed via merge train #10559 (v0.5.1592). All source commits preserve authorship; merged main matches the validated train exactly.