Merge train 177: #10174 - #10187
Merged
Merged
Merge train 177: #10174#10187
Conversation
added 5 commits
September 13, 2026 10:57
…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
…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
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
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (16)
📝 WalkthroughWalkthroughChangesRegExp performance changes
Estimated code review effort: 4 (Complex) | ~45 minutes Change: Refactor Sequence Diagram(s)sequenceDiagram
participant RegExpSplit
participant forward_program
participant host_find
participant StickyLoop
RegExpSplit->>forward_program: Check constructor and builtin exec
alt Forward path is eligible
forward_program-->>RegExpSplit: Return non-sticky program
RegExpSplit->>host_find: Search from current position
host_find-->>RegExpSplit: Return captures and match span
else Forward path is not eligible
forward_program-->>RegExpSplit: Return no program
RegExpSplit->>StickyLoop: Execute per-position sticky search
StickyLoop-->>RegExpSplit: Return match result
end
✨ Finishing Touches📝 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 |
This was referenced Sep 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merge train 177: lands #10174 (bind the subject and program once per split/replace/match, and split forward instead of trying every position; part of #10165) at head
d61fe85913, plus the workspace version bump to 0.5.1548.The four PR commits were cherry-picked onto
9b911855f8without conflicts and match bygit patch-id --stable.crates/perry-runtimeis byte-identical to the PR head; the tree differs only by main's own commits since the PR's base (HIR JSON-define work) and the bump.One commit was added to the PR after review,
gc: record the forward-split test counter's holder verdict.scripts/gc_runtime_root_holders.pyflagged the PR's new#[cfg(test)] FORWARD_SPLITS: Cell<usize>under rule B, and the commit records it astest_onlyinscripts/gc_runtime_root_holders.json. It is a count and never an address.Local validation (perrymaster, Linux x86_64;
--locked, no publish-age override in the environment):cargo metadata --locked,cargo fmt --all -- --checkcargo check -p perry-runtime --no-default-features --features full --lib(regex feature off): 0 warningscargo check -p perry-runtime --lib --tests: no warnings outside the knownglobal_this_webassembly.rsdead code on maincargo test -p perry-runtime --lib -- --test-threads=1: 3682 passed, 1 failed. The failure isnative_stack::tests::stack_top_respects_custom_thread_stack_sizes, red on main.cargo clippy -p perry-runtime --lib --tests: the same 12approx_constanterrors as main at9b911855f8. Diffed against main, the only change is onetype_complexitywarning ingc/tests/runtime_roots/perex_split.rs.scripts/run_lint_gates.shscript tier: 76 of 77 pass. The failure is public benchmark evidence freshness, which fails identically on main (main replay: 1 of 77, same gate). The compile tier was not run on this train.check_changeset_fragment.sh PerryTS/perry 10174: passBehavioural evidence (spec cases, fallback witnesses, fault injections and measurements) is on #10174. GitHub runners are down, so this local replay is the gate. Next in order: #10176, #10181, #10183.
https://claude.ai/code/session_01Da12JXeG5XuVBma5yWp5C9
Summary by CodeRabbit
Performance
RegExpsplit, replace, and global match operations by reducing repeated processing, delivering significantly faster execution on large inputs.RegExpsplit operations with forward searching while preserving specification-compatible behavior, including captures, limits, empty matches, and Unicode handling.Testing
Documentation