Skip to content

Merge train 185: #10225 - #10227

Merged
proggeramlug merged 3 commits into
mainfrom
train185
Sep 13, 2026
Merged

proggeramlug merged 3 commits into
mainfrom
train185

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Merge train 185: lands #10225 (RegExp replace without exec result objects when every step is the builtin; part of #10165) at head 7a7f1d4621, plus the workspace version bump to 0.5.1561.

Both PR commits were cherry-picked onto bb9aa5a64 without conflicts. main has not moved since the PR's base, so the tree before the bump is byte-identical to the PR head, and the PR head's validation applies unchanged.

Validation on that tree (perrymaster, Linux x86_64, --locked):

  • cargo fmt --all -- --check
  • cargo check -p perry-runtime --no-default-features --features full --lib (regex feature off): 0 warnings
  • cargo check -p perry-runtime --lib --tests: no warnings outside global_this_webassembly.rs and main's own ic_slow.rs:544
  • cargo test -p perry-runtime --lib -- --test-threads=1: 3754 passed, 1 failed. The failure is native_stack::tests::stack_top_respects_custom_thread_stack_sizes, red on main.
  • cargo clippy -p perry-runtime --lib --tests: identical to main at bb9aa5a64
  • scripts/run_lint_gates.sh script tier: 76 of 77 pass. The failure is public benchmark evidence freshness, identical on main.
  • gc_runtime_root_holders.py, check_file_size.sh, check_changeset_fragment.sh PerryTS/perry 10225: pass
  • All 8 fault injections from the PR were caught.

Re-checked on this branch after the bump: cargo metadata --locked, fmt, holders, file size.

Measured on #10225 (perf stat, 400,000 matches, vs main):

  • callback replace: CPU 2.7× lower, −58.9 % instructions
  • "[$&]" template: 3.0× lower, −52.6 %
  • "$2$1" template: 4.5× lower, −69.5 %
  • non-ASCII callback: 2.3× lower, −44.2 %

A 1M-record replace still fails on both main and this branch because of #10215; this change does not alter that.

https://claude.ai/code/session_01Da12JXeG5XuVBma5yWp5C9

Summary by CodeRabbit

  • Performance

    • Improved regular expression replacements for eligible patterns, reducing processing overhead while preserving replacement results, callback behavior, and lastIndex handling.
  • Compatibility

    • Maintained support for template and callback replacements, including empty matches and large match ranges.
    • The optimization automatically falls back to the existing behavior when patterns are not eligible.
  • Documentation

    • Updated the documented and workspace version to 0.5.1561.

Ralph Küpper added 3 commits September 13, 2026 20:55
…he builtin (#10165)

RegExp @@replace materialized a full exec result array per match, then read
length, 0, index, each capture and groups back through generic property
gets, and pushed every capture into a traced list before building the
output. For a receiver whose exec is the builtin (regexp_view_uses_builtin)
and whose program has no named groups, those objects and reads cannot be
observed, so this path collects each match's capture spans natively instead
and builds the output from spans of the input.

The specification's order is kept: every match is collected before the first
replacer call, so a replacer that changes lastIndex, exec or the pattern
cannot change which matches are replaced. flags and the lastIndex reset still
run first, and admission is decided after them because either can run user
code; inside the collection loop no user code can run. Templates are parsed
once (GetSubstitution without named groups) and emit input spans, so $&, $n,
$` and $' allocate nothing per match. Replacer calls get the same arguments.

The spans follow the subject (matches x captures), so they live in a plain
Vec reported to the collector as external bytes and are not charged to the
operation's MemoryBudget (#10164/#10207). execute_with_resources gains an
ExecOutput::Spans mode; its signature is unchanged.

Tests: the direct path against the ordinary loop (output and final lastIndex)
over templates with every $ form, zero-width global matches with and without
u, sticky and non-global receivers, non-ASCII input; replacer arguments; a
replacer that rewinds lastIndex and installs an own exec; admission declining
for an own exec and for named groups; span storage one match past a
SCRATCH_BYTES/8-entry cap. A debug assertion bounds the collection loop.

Claude-Session: https://claude.ai/code/session_01Da12JXeG5XuVBma5yWp5C9
@proggeramlug
proggeramlug merged commit 6000a00 into main Sep 13, 2026
20 of 22 checks passed
@proggeramlug
proggeramlug deleted the train185 branch September 13, 2026 20:56
@coderabbitai

coderabbitai Bot commented Sep 13, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: a9b3154a-d625-499c-a225-bbc1a9b68793

📥 Commits

Reviewing files that changed from the base of the PR and between bb9aa5a and b293dd7.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (10)
  • CLAUDE.md
  • Cargo.toml
  • changelog.d/10225-regex-replace-direct.md
  • crates/perry-runtime/src/gc/tests/runtime_roots.rs
  • crates/perry-runtime/src/gc/tests/runtime_roots/perex_replace.rs
  • crates/perry-runtime/src/gc/tests/runtime_roots/perex_replace_direct.rs
  • crates/perry-runtime/src/regex.rs
  • crates/perry-runtime/src/regex/perex_api.rs
  • crates/perry-runtime/src/regex/perex_replace.rs
  • crates/perry-runtime/src/regex/perex_replace_direct.rs

📝 Walkthrough

Walkthrough

Changes

Direct RegExp replacement

Layer / File(s) Summary
Exec output span support
crates/perry-runtime/src/regex/perex_api.rs
execute_output now supports Test, Object, and Spans outputs. Span output stores UTF-16 capture offsets without creating result objects.
Direct replacement implementation and dispatch
crates/perry-runtime/src/regex/perex_replace_direct.rs, crates/perry-runtime/src/regex/perex_replace.rs, crates/perry-runtime/src/regex.rs
Eligible builtin regular expressions without named groups use direct replacement. The path parses templates, collects spans, handles global and empty matches, invokes callbacks, and preserves the existing fallback loop.
Replacement validation and release metadata
crates/perry-runtime/src/gc/tests/runtime_roots/*, changelog.d/10225-regex-replace-direct.md, CLAUDE.md, Cargo.toml
Runtime-root tests compare direct and ordinary replacement behavior, callback arguments, fallback conditions, receiver mutation, and large span storage. The changelog and version metadata are updated.

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Refactor

Sequence Diagram(s)

sequenceDiagram
  participant RegExpReplace
  participant DirectReplace
  participant RegexExecution
  participant Replacer
  RegExpReplace->>DirectReplace: Check receiver eligibility
  RegExpReplace->>DirectReplace: Start direct replacement
  DirectReplace->>RegexExecution: Collect match and capture spans
  RegexExecution-->>DirectReplace: Return spans
  DirectReplace->>Replacer: Apply template or callback
  DirectReplace-->>RegExpReplace: Return output string
Loading
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch train185

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant