perf(regex): cut split and replace host overhead on ASCII subjects - #11543
Conversation
The package profile (benchmarks/packages/PROFILE.md) charges three host-side costs to regex. None of them is in the matcher: - A split piece, or a replace capture, over an ASCII subject is now one byte copy (`copy_ascii_span`, which exec captures already used). It used to be two unit-by-unit cursor passes with two safepoint polls per piece. - A string-template replacement whose subject and template are both ASCII now builds its output as one allocation plus a copy per native piece. It used to decode and re-encode every unit twice. - The forward split loop polls once per 512 units of pieces (the POLL_UNITS trade #10657 measured for replace), not once per piece. It asks the search for captures only when the splitter has capture groups. The output list has not been seen by any code outside the operation yet, so it appends without a catch frame whenever the append fits the list's capacity. Adds test-files/test_gap_regex_engine_package_shapes.ts. It covers the regex shapes the package workloads spend their time in, plus the lastIndex, sticky, fold, split and replace-template edges these paths must keep exact.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe regex runtime adds stride-based polling and updates split capture and list handling. It adds direct ASCII span-copy and replacement-output paths. A new package-shaped test covers regex matching, splitting, and replacement. ChangesRegex span handling
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Refactor Merge Risk: ⚪ Minimal · up to No actionable merge-blocking risk was established for the regex split and replacement optimizations. Merge after normal checks pass. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The inspected paths retain checks around ASCII copying and split output construction. Split reaches garbage-collection checkpoints less often, but other checkpoints remain. No new privilege or external boundary was identified; security coverage is incomplete. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
Ready to merge once CI is clean. It is Perry-side regex host overhead: ASCII split pieces and replace captures become single byte copies, template replacements use one allocation, the safepoint in the split loop fires every 512 units, there are no capture requests for group-free patterns, and no catch frame. Split −43%, short global replace −15%, validator −3 to −4%, RSS within 1%. The regex/split/replace gap tests are identical on main and the branch (65 pass). The engine-side win is perex#3, adopted via #11548 once released. |
1 participant
Part of #10165
Part of #10518
This PR cuts host-side regex costs in
String.prototype.splitand in string-templatereplace. None of the changed code is in the matcher. The matcher side of the package regex cost is PerryTS/perex#3; a Perry PR adopting it is linked below.Changes
SpanCopies::copynow usescopy_ascii_span, whichexeccaptures already used, whenever the subject is ASCII. Before, each piece took two unit-by-unit cursor passes and two safepoint polls.Pieces::finish_asciihandles native pieces over an ASCII subject and an ASCII template (or no template) with one allocation plus one copy per piece. Before, every unit was decoded and re-encoded twice. Any other combination still takes the existing two-pass path.PieceStride). That isPOLL_UNITS, the trade perf(regex): poll the safepoint on units read, not on pieces #10657 measured for the replace passes.CaptureMode::Fullwhen the splitter has no capture groups. Before, every piece built, filled and copied a capture slot array (plus a poll) only to find it empty.List::push_unseenis used only for a split's output list, which no code outside the operation has seen yet, so nothing can have frozen, sealed or proxied it. When the append fits capacity it cannot throw, so it skips thecatch_js_throwframe. An append that has to grow still takespush.test-files/test_gap_regex_engine_package_shapes.tscovers the package regex shapes: uuid validate, JWS_REGEX, dotenv LINE, validator'ssplit(/%..|./), cron tokens, and string-template replaces with ASCII and non-ASCII on each side, every$form, empty pieces, astral characters and lone surrogates. It also covers the fold, counted, lazy, sticky and global edge cases the perex fast paths must keep exact. Output is byte-identical to Node 26.5.1 on unpatchedmainand on this branch. It is a regression guard, not a fails-without-fix test: this PR changes cost, not answers.Measurements (qb2: AMD EPYC 9254, idle; both arms built here with
--release;PERRY_NO_AUTO_OPTIMIZE=1)Instruction counts come from
perf stat -e instructions:uas a two-N differential (median of 3 at each N). The base arm ismainat d57f513; the only difference in the other arm is this diff. Main has since moved to 98dd568, which touches none of these files.Microbenchmarks, instructions per iteration:
encodeURI(s).split(/%..|./), about 60 pieces"a-b-c".replace(/-/g, "+")"Hello".replace(/l/, "L")(literals in loop)LINE.execloop (control)REGEX.testuuid (control)Package workloads (
scripts/package_bench.py run --arms node,perry --modes instr), instructions per iteration. Every Perry run's output was byte-identical to Node 26.5.1:Peak RSS, median of 5
/usr/bin/timeruns at the harness's n2: validator/sanitize +0.4%, date-fns/format_add −0.0%, commander +0.6%, validator/batch −0.0%, moment +0.7%, dotenv +0.0%. Microbench peak RSS: split +0.6%, replace +0.0%. None of these is an RSS-for-compute trade.A change I left out: RSS cost
Growing the lent match scratch past its fixed 32 registers removes the owned-path rebuild on every search for bigger programs. dotenv's LINE has 42 registers. With it, dotenv/parse is −31% instructions and moment/parse_format −16%, but peak RSS rises +27.6% and +13.4%. The owned path notes its per-call buffers with
gc_note_external_side_alloc, and that phantom external pressure is what keeps these loops in frequent budgeted full cycles (82 per run against about 12 copying minors without it). Taking the phantom pressure away moves them to the nursery-cap regime. That is a GC pacing decision, not a regex one, so it is not in this PR. The measurement is on branchwip/perf-regex-matcher(commit "grow the lent scratch's registers").Tests (qb2)
cargo test --release -p perry-runtime --lib -- perex split replace regexp(RUST_TEST_THREADS=1): 222 passed.run_parity_tests.shwithPERRY_SKIP_BUILD=1 PERRY_NO_AUTO_OPTIMIZE=1, filtersregex,regexp,split,replace,match, against Node 26.5.1. Both main and this branch: 65 PASS, plus the same pre-existing COMPILE_FAIL (test_gap_regex_replace_dyn_regex_with_http).SKIP_COMPILE_GATES=1 scripts/run_lint_gates.sh: 92 of 94 script gates passed; the compile tier was not run. The two failures arecargo xwinnot installed on this host, and "Public benchmark evidence freshness", which is known-red on main.git diff --statwas clean afterwards.cargo fmt --all -- --check: clean.Not run: the full gap sweep,
cargo test --workspace, macOS, the auto-optimize (defaultperry compile) arm, and the Windows xwin check.No test outside the touched regex modules is expected to change.
Related
min-publish-agesoak.Summary by CodeRabbit
String.prototype.splitand string-templateString.prototype.replacenow handle ASCII strings with less host work, including more efficient copying and output construction.