-
-
Notifications
You must be signed in to change notification settings - Fork 161
perf(regex): construct a RegExp without allocating its flags string #9819
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
89be3b1
perf(regex): close the backtracking cliff, allocation-free cache prob…
bddf7f0
docs(changelog): key the three regex fragments to PR 9796
57f5c0b
fix(regex): reconcile #9801's repair block with the borrowed cache keys
5f17264
perf(regex): construct a RegExp without allocating its flags string
f4bcb1e
docs(changelog): key the flags-allocation fragment to PR 9819
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| ### Performance | ||
|
|
||
| - **A capture group no longer turns a pattern into a ReDoS.** | ||
| `repeat_matcher::capture_layout` takes a pattern off the linear `regex` | ||
| engine when ECMA-262's RepeatMatcher capture semantics are observable — a | ||
| capture group directly under a quantifier, or a capture inside a negative | ||
| lookaround. That routing is a correctness requirement (the linear engine | ||
| keeps the last value of a capture nested in a quantified group; the spec | ||
| clears it on every iteration), but the engine it routes to, `regress`, is a | ||
| classical backtracker with no step budget. So adding parentheses was enough | ||
| to fall off a linear-time path onto an exponential one: | ||
|
|
||
| | pattern | node | perry (before) | perry (after) | | ||
| |---|---|---|---| | ||
| | `/^(a+)+$/.test("a"×28 + "!")` | 4,798 ms | **16,522 ms** | **0 ms** | | ||
| | `/^(?:a+)+$/.test(…)` (same language, no capture) | 4,288 ms | 0 ms | 0 ms | | ||
|
|
||
| **6.3 %** of the 4,463 distinct regex literals across seven real bundles | ||
| take that route — claude-code 7.1 %, dayjs 25 %, luxon 29 % — including | ||
| shapes like `^[a-z][a-z0-9]*(-[a-z0-9]+)*$`. | ||
|
|
||
| The two engines accept exactly the same LANGUAGE for a pattern they both | ||
| compile; they disagree only about which capture assignment to report. So the | ||
| linear program is asked first (`linear_rules_out_match`), and when it proves | ||
| there is no match at or after the search offset — which is what every ReDoS | ||
| input is, a subject that ALMOST matches and then fails — the backtracker is | ||
| never entered. Every `&str`-subject entry point goes through | ||
| `lookup_repeat_matcher_for`: `test`, `exec`, `match`, `matchAll`, `search`, | ||
| `split` and `replace` with a string replacement. The gate disables itself | ||
| where the linear engine has no opinion (a pattern it could not compile holds | ||
| the never-match placeholder), which is exactly the lookaround shapes. | ||
|
|
||
| **This removes the reachable exponential case; it does not BOUND the worst | ||
| case.** A real step budget has to be counted by the backtracker, and | ||
| `regress` has none today (`fancy-regex`, by contrast, ships | ||
| `backtrack_limit: 1_000_000`). A 101-line patch adding one has been measured | ||
| — worst hostile search 51 s → 124 ms at a budget of 1,000,000, zero answers | ||
| changed across 13,389 real searches, upstream's own 544 tests unchanged — and | ||
| is open upstream as | ||
| [ridiculousfish/regress#177](https://github.com/ridiculousfish/regress/pull/177). | ||
| Until it lands and perry picks it up, do not read "cliff fixed" as "worst | ||
| case bounded". | ||
| (`quantified_capture_pattern_does_not_backtrack_on_a_non_matching_subject`) |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| ### Performance | ||
|
|
||
| - **Probing the compiled-program caches no longer materialises the key.** The | ||
| three thread-local caches were `HashMap<(String, String), _>`, and | ||
| `HashMap::get` needs a `&(String, String)` — so **every probe allocated two | ||
| Strings and copied the pattern text into them**, on a path that runs once per | ||
| RegExp OBJECT, and a JS regex literal evaluates to a fresh object every time | ||
| it is reached. A native-churn census of the claude-code binary (2026-09-05) | ||
| put `js_regexp_test` → `lookup_repeat_matcher` → `build_and_install_programs` | ||
| at **6,044 MB of 8,334 MB of estimated allocation with zero live bytes** — | ||
| 73 % of all remaining native churn — split across the three probe sites: the | ||
| `get_or_compile_regex` probe (2,071 MB) and two `core::fmt::Formatter::pad` | ||
| frames (1,989 MB and 1,984 MB), which is what `.to_string()` on an `Arc<str>` | ||
| lowers to. | ||
|
|
||
| The caches are now keyed by `ProgramKey = (Arc<str>, Arc<str>)`. Every caller | ||
| that matters already holds those `Arc`s — `REGEX_SOURCE_TABLE` and | ||
| `regex::site_cache` share one allocation of a literal's text with every | ||
| header built from it — so a probe is two refcount increments and no | ||
| allocation at all. The two remaining `Arc::from` materialisations are on cold | ||
| paths: the syntax-error fallback in `js_regexp_new` (a pattern the linear | ||
| engine's parser refused, 7.7 % of real literals, once each) and | ||
| `RegExp.prototype.compile` (once per call from user code). | ||
|
|
||
| Hashing still walks the pattern bytes; the allocation is what the census | ||
| measured and what this removes. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| ### Internal | ||
|
|
||
| - **`PERRY_REGEX_ENGINE=regress` — a measurable tier-0 engine prototype.** | ||
| Routes every pattern through `regress` (the ECMAScript backtracker perry | ||
| already links for RepeatMatcher capture semantics) instead of only the ones | ||
| whose capture semantics require it, and installs a shared never-match | ||
| placeholder as the standard program so no NFA is built. Every exec-family | ||
| entry point already consults the repeat matcher first, so this exercises the | ||
| whole engine surface — `exec`, `test`, `match`, `matchAll`, `search`, | ||
| `split`, `replace` — without a second implementation. | ||
|
|
||
| It exists so the engine question is settled on measurements from a real | ||
| binary rather than on a corpus harness. Measured over 4,463 distinct regex | ||
| literals extracted from seven real bundles (two claude-code builds, ethers, | ||
| moment, dayjs, luxon, mongodb) with a tracking allocator and the programs | ||
| held live: | ||
|
|
||
| | engine | accepted | compile µs (med) | bytes/program (med) | corpus total | | ||
| |---|---|---|---|---| | ||
| | `regex` crate (tier 1 today) | 92.3 % | 48.5 | 12,492 | 136.7 MB | | ||
| | `regress` | **100 %** | **2.2** | **512** | **4.9 MB** | | ||
| | `fancy-regex` (tier 2 today) | 97.8 % | 59.2 | 12,623 | 146.6 MB | | ||
|
|
||
| node/V8, measured the same session, is ~2,600 bytes per program. A | ||
| differential over 4,119 patterns × 13 subjects (53,547 comparisons of match | ||
| presence, span and every capture span) found **0 disagreements** between the | ||
| linear engine and `regress`. | ||
|
|
||
| **Not a supported configuration**: the backtracker has no step budget, so a | ||
| pathological pattern can run unbounded. Off by default, one relaxed atomic | ||
| load when unset. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| ### Performance | ||
|
|
||
| - **Constructing a `RegExp` no longer allocates its flags string.** A JS regex | ||
| literal evaluates to a fresh `RegExp` object every time it is reached, and | ||
| `js_regexp_new` materialized the canonical flags twice per construction: once | ||
| as a Rust `String` from `validate_and_canonicalize_flags`, and once as a fresh | ||
| GC `StringHeader` for `flags_ptr`. On the claude-code TUI that is **161,897 | ||
| constructions per 400-character reply** (`PERRY_REGEX_DIAG`) — ~5.2 MB of | ||
| identical one- and two-byte GC strings per reply, ~44 MB on a 3300-character | ||
| one, and ~1.4 million allocations. | ||
|
|
||
| Neither copy is needed. There are eight legal flags, each may appear once, so | ||
| the canonical form is at most eight ASCII bytes and now lives inline in a | ||
| `CanonicalFlags` value instead of on the heap. And JS strings are immutable | ||
| with no identity semantics, so when the caller's flags text already IS the | ||
| canonical text — which it is for a literal, whose flags the author wrote in | ||
| spec order — the header shares the caller's string rather than duplicating | ||
| it. Only a non-canonical spelling (`/x/ig` → `"gi"`) or a computed | ||
| `new RegExp(p, f)` still materializes one; the new `flags_alloc` counter in | ||
| `PERRY_REGEX_DIAG` reports how often that happens. | ||
|
|
||
| This is a **below-the-line** allocation fix by the campaign's own ~10 % rule: | ||
| at ~2-3 % of arena traffic per turn it cannot change the collection schedule, | ||
| and the cc rig is expected to read flat. It is worth doing because the | ||
| allocation is pure waste, not because it moves a benchmark. | ||
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the allocation claim for computed flags.
js_regexp_newshares any valid caller string whose contents already equal the canonical flags. Therefore, a computednew RegExp(pattern, f)can also avoid the canonical-flags allocation whenfis already a canonical string. A conversion to a string may allocate separately. Update this sentence to describe the actual condition.Suggested wording
🤖 Prompt for AI Agents