perf(regex): resume searches and capture reads from the previous position (#10164) - #10181
perf(regex): resume searches and capture reads from the previous position (#10164)#10181proggeramlug wants to merge 7 commits into
Conversation
…10165) String split, replace and global match run a search at every position or match of one string with one matcher, but execute_with_resources bound both afresh for each search. Binding a subject decodes the entire string (perex Input::wtf8, uncharged) and binding a program revalidates every word, so each of those operations did O(n) binding work per search and O(n²) overall. On ASCII input this is why `str.split(/[,; ]+/)` took 8.1 s for a 150,000-unit string. The three loops already held a BoundSubject over their input. A new perex_api::Reuse carries it, plus a program binding taken from the receiver before the loop, into execute_with_resources. Perex's binding contract allows a binding to outlive allocation, collection and JS callbacks: Perry's owners hold registered roots and reacquire their base on every view. A search uses a reused binding only while it is provably the same object (same string; same receiver still holding the same program cell); otherwise it binds afresh exactly as before, which covers an exec override, RegExp.prototype.compile and a different string. Reuse is built before each loop because runtime handle scopes are a stack. matchAll's next(), JS-level exec/test and search are one search per JS call and are unchanged; cross-call reuse is a separate contract. The non-ASCII seek charge behind the work-limit RangeError (#10164) is on the Perex side and needs its search-from-position API; this change does not affect it. Tests: gc::tests::runtime_roots::perex_reuse covers a whole global loop with a minor collection at every poll under forced evacuation (subject and program cell both relocate; fresh work equals reused work plus six program validations, proving reuse engages), a recompile between searches, and a different string. Each test fails when its guard is sabotaged. Claude-Session: https://claude.ai/code/session_01Da12JXeG5XuVBma5yWp5C9
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 8 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe regex runtime now reuses bound programs and subjects across compound operations, resumes searches from prior positions, and adds a forward-search split path for intrinsic built-in ChangesPEREx execution
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant RegexOperation
participant dispatch_execute
participant Reuse
participant perex_runtime
participant ResultMaterialization
RegexOperation->>dispatch_execute: execute with optional Reuse
dispatch_execute->>Reuse: reuse matching program and subject
Reuse->>perex_runtime: find_near from prior Position
perex_runtime-->>Reuse: return match and new Position
Reuse->>ResultMaterialization: materialize with nearby position
ResultMaterialization-->>RegexOperation: return regex result
Merge Risk: ⚪ Minimal · up to The regex performance changes preserve tested fallback and rebinding behavior, with no established production risk; the change is mergeable. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 61.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 54 functions across 15 files. (3 skipped: 3 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
…10165) RegExp.prototype[@@split] tries a sticky match at every position q. Each attempt starts a whole search, so split pays a search's fixed setup per subject unit: about 27.6 work units per unit for `/[,; ]+/`, against about 9 for a global exec loop over the same subject. A non-sticky search from q returns the leftmost position s >= q where the pattern matches, with the same match a sticky attempt at s finds. The attempts at q..s-1 can therefore be skipped without changing any piece or capture, and empty matches and Unicode advancement line up. Skipping them is unobservable only when nothing can see a RegExpExec: - the species is absent or the intrinsic RegExp (recognised by its call thunk), so the splitter is a fresh object no user code holds and its skipped lastIndex writes cannot be seen; a user species could return a real RegExp and read lastIndex afterwards; - the splitter's exec resolves, without running a getter, to the builtin data property (regexp_view_uses_builtin), so the skipped Get(exec) calls cannot be seen either. When both hold, split compiles a program from the splitter's own internal source and canonical flags without `y` (never from the receiver, whose program a limit valueOf could replace via RegExp.prototype.compile after the splitter was built) and searches forward with it, reusing the operation's subject binding. Anything else runs the unchanged per-position sticky loop. Tests (gc::tests::runtime_roots::perex_split): - forward search matches ten results derived by hand from the sticky algorithm (repeated and unmatched captures, empty matches, `$` at the end, limits inside captures, Unicode empty-match advancement, the non-ASCII #10164 record), each asserted to take the forward path; - a user species returning a real RegExp keeps the sticky loop and leaves the splitter's lastIndex at 2, as the specification requires; - the existing species-factory and custom-exec tests now also assert the sticky loop ran. Sabotage: admitting any species fails the user-species test; trying the end of the input or dropping captures fails the forward-search test. Claude-Session: https://claude.ai/code/session_01Da12JXeG5XuVBma5yWp5C9
d175675 to
d601dd4
Compare
gc_runtime_root_holders.py flags the new #[cfg(test)] FORWARD_SPLITS Cell<usize> under rule B. It is a test-only count, never an address. Claude-Session: https://claude.ai/code/session_01Da12JXeG5XuVBma5yWp5C9
…tion (#10164) On non-ASCII (byte) storage a Perex search seeks to its start from the nearer end of the subject, up to half its length. A global replace or match starts a search per match, and split one per match or position, so the seeks summed to about n²/4: at 32,000 units that exceeded the former 100,000,000-unit allowance and threw, and without a cap it is quadratic time. Materializing captures seeked from an end the same way. Perex 0.1.2 adds a search-from-position API: Search::new_near and Search::position (the match end, or the start of the last attempt), and BoundSpan::new_near. perex_runtime::find_near and perex_strings::copy_span_near take an optional position; find and copy_span delegate to them with none. - perex_api::Reuse carries the last position of the reused subject, set only from and used only with that binding, because a position from another string with the same layout cannot be detected. execute_with_resources seeds each search and its capture materialization from it; global match copies each result from its search's end. - Split's forward search seeds each search from the previous one and materializes captures from the search's position. Tests: - perex_reuse: a non-ASCII global loop's work roughly doubles when the subject doubles (3.71x without positions); the #10164 reduction, a 32,000-unit split (6,001 pieces) and a 60,000-unit global replace (76,000 units), completes. - perex_split: a non-ASCII forward split's work roughly doubles when the input doubles (3.97x when it never resumes). Sabotage: each of those fails when positions are not used. Requires perex 0.1.2, published 2026-09-13 from PerryTS/perex d9f395d88931f0cfee1fe89ddf455cda606fd9e1 (crates.io checksum 21df239ee18f99de6abff50953f6f15be1b5ebd11e6ae9661acdd93026e983db). It is inside the workspace's 7-day min-publish-age window, so Cargo.lock was resolved once with CARGO_RESOLVER_INCOMPATIBLE_PUBLISH_AGE=allow, as for perex 0.1.0, with the maintainer's approval. Ordinary --locked builds use the locked version without the override. Claude-Session: https://claude.ai/code/session_01Da12JXeG5XuVBma5yWp5C9
gc_runtime_root_holders.py flags the new #[cfg(test)] LAST_FORWARD_WORK Cell<usize> under rule B. It is a test-only work count, never an address. Claude-Session: https://claude.ai/code/session_01Da12JXeG5XuVBma5yWp5C9
d601dd4 to
d5d1974
Compare
|
Restacked onto the landed #10174 and pushed as Full replay of
It lands via its merge train after #10176. |
Fixes the quadratic-time half of #10164. Stacked on #10174: this branch contains #10174's two commits, so please review only the top commit,
perf(regex): resume searches and capture reads from the previous position.Problem
On non-ASCII (byte) storage a Perex search seeks to its start from the nearer end of the subject, up to half its length. A global replace or match starts a search per match, and split one per match (or per position), so the seeks summed to about n²/4. At 32,000 units that exceeded the former 100,000,000-unit allowance and threw
RangeError; without a cap (#10176) it is quadratic time. Materializing captures seeked from an end the same way. The placement is on #10164.Change
Perex 0.1.2 adds a search-from-position API:
Search::new_nearandSearch::position(the match end, or the start of the last attempt), andBoundSpan::new_near.perex_runtime::find_nearandperex_strings::copy_span_neartake an optional position;findandcopy_spandelegate to them with none.perex_api::Reuse(from perf(regex): bind once per split/replace/match, and split searches forward (#10165) #10174) carries the last position of the reused subject. It is set only from, and used only with, that binding, because a position from another string with the same layout cannot be detected.execute_with_resourcesseeds each search and its capture materialization from it, and global match copies each result from its search's end.Results
Same host (AMD Ryzen 7 7700X, Node 26.8.1), release builds from source; checksums match Node at every completed size.
mainslopemainregex-split-unicode/[,;😀]+/uregex-replace-callback-unicode/[ä中😀Ö漢🦊]+/gu+ callbackAt n = 1,000,000 both rows still stop on the fixed 100M allowance in this branch alone; #10176 removes it.
Standalone reproducer, mean of two alternating rounds (bind once / positions interleaved):
split unicoden = 1,000split unicoden = 2,000split unicoden = 4,000split unicoden = 10,000replace cb unicoden = 1,000replace cb unicoden = 2,000replace cb unicoden = 4,000replace cb unicoden = 10,000ASCII rows are unchanged, as expected: ASCII storage already seeks in constant work. That was checked with interleaved rounds, after a first non-interleaved run was skewed by a concurrent build on the host.
Still quadratic: JS-level
execloops,matchAlland.test()do one search per JavaScript call. Their cross-call reuse is a separate, approved design being worked out with the Perex maintainer.Tests
perex_reuse:perex_split: a non-ASCII forward split's work roughly doubles when the input doubles (3.97× when it never resumes).WorkLimit.gc::tests::runtime_roots::perex*tests and all 71regex::unit tests pass on the crates.ioperex 0.1.2, and rustfmt is clean.Dependency
perex = "0.1.2"was published 2026-09-13 from PerryTS/perexd9f395d88931f0cfee1fe89ddf455cda606fd9e1, crates.io checksum21df239ee18f99de6abff50953f6f15be1b5ebd11e6ae9661acdd93026e983db.It is inside the workspace's 7-day
min-publish-agewindow, soCargo.lockwas resolved once withCARGO_RESOLVER_INCOMPATIBLE_PUBLISH_AGE=allow, as for 0.1.0, with the maintainer's approval. An ordinarycargo check --lockedwithout the override was verified to download and build the locked 0.1.2.https://claude.ai/code/session_01Da12JXeG5XuVBma5yWp5C9
Summary by CodeRabbit
Performance
Compatibility
execmethods, species constructors, subclasses, and Unicode text.Reliability