Skip to content

Review follow-ups for rust-lang/regex#1350 (literal-prefix capture fast path) - #1

Draft
Dandandan wants to merge 9 commits into
masterfrom
claude/pr-review-benchmarking-d83d3u
Draft

Dandandan wants to merge 9 commits into
masterfrom
claude/pr-review-benchmarking-d83d3u

Conversation

@Dandandan

Copy link
Copy Markdown
Owner

This branch is rust-lang#1350 (6 commits) with 3 follow-up commits on top, for cherry-picking into the upstream PR. Only the last 3 commits are new.

Follow-up commits

  1. automata: validate only Unicode-class spans in literal prefix capture (correctness)
    The fast path validated the whole haystack as UTF-8 whenever the capture class or the .* class was a Unicode class. When UTF-8 mode is off, the literal prefix and any (?-u:…) span may legitimately contain invalid UTF-8, so valid matches were dropped:

    • ^((?-u:[^/])+)/.*$ on b"\xA9/" returned no match; PikeVM matches 0..2
    • ^([^\n]+)\n(?-u:.)*$ on b"x\na\xA9bb" returned no match
    • ^(?-u:\xFF)([^/]+)/.*$ on b"\xFFab/x" returned no match

    This only happens through regex_automata::meta with syntax.utf8(false) and utf8_empty(true). The regex crate's defaults never reach it. The fix validates exactly the capture span and the tail span. A failure now skips to the next prefix instead of returning None. Because validation is now exact, Unicode classes no longer need gating on utf8_empty, so regex::bytes::Regex also gets the fast path (Q28 bytes replace: 1411 → 298 ns).

  2. automata: use a single memchr to find the capture terminator (simplification)
    [^X]+ may contain \n, so the memchr2(X, '\n') loop always ends at the first X. A plain memchr gives the same result. Performance is unchanged.

  3. regex: avoid Captures allocation in single-match replace (perf)
    For replace (limit == 1): a $N replacement with N < 4 uses stack slots via meta::Regex::search_slots. Other templates call captures() directly instead of captures_iter, which allocates a Captures and then clones it per match.

Benchmarks

The harness is a standalone binary, not committed. It uses 200k synthetic Referer-style URLs (60% https, 50% www., 20–160 bytes, some non-ASCII), pinned to one core, and reports the best of 3×7 runs.

workload master rust-lang#1350 rust-lang#1350 + fixes rust-lang#1350 vs master fixes vs rust-lang#1350
Q28 replace(s, "$1") 1371 ns 164 ns 129 ns 8.3x 1.28x
Q28 replace(s, "x${1}") 1375 ns 259 ns 202 ns 5.3x 1.28x
Q28 captures 1227 ns 106 ns 99 ns 11.6x 1.07x
Q28 is_match 191 ns 63 ns 61 ns 3.0x –
Q28 replace, 2 KB URLs 24.0 µs 369 ns 324 ns 65x 1.14x
Q28 replace, non-matching 162 ns 109 ns 78 ns 1.5x 1.39x
Q28 bytes::Regex replace 1390 ns 1411 ns 298 ns 1.0x 4.74x
Q28 bytes::Regex (?-u) replace 1364 ns 247 ns 232 ns 5.5x –
(\w+)@ first-match replace "$1" (not Q28 shape) 93 ns 90 ns 61 ns 1.0x 1.49x
replace_all "$1", 40k matches 5.57 ms 4.83 ms 4.83 ms 1.15x –

Ablations on rust-lang#1350 itself:

  • Dropping Input::new_utf8, so the fast path always validates: +30 ns on short URLs, 2x slower on 2 KB URLs. The new public API is worth something.
  • Dropping single_capture_ref: Q28 replace goes 164 → 256 ns.

Testing

  • Added regression tests for the UTF-8 span bug and for the stack-slot replace path (tests/misc.rs, tests/replace.rs).
  • Ran a differential fuzzer (random recognized-shape patterns × random byte haystacks × 3 syntax configs × utf8_empty on/off) comparing meta::Regex capture spans against PikeVM. Before fix: 4957 mismatches in 2.8M checks. After: 0 in about 17M checks across 4 seeds.
  • cargo test passes for regex and for regex-automata --all-features. --no-default-features builds pass.

Other review notes on rust-lang#1350 (no code change here)

  • Public API surface: Input::new_utf8 (regex-automata) and Replacer::single_capture_ref (regex) are both new public items. Upstream may prefer #[doc(hidden)] or a narrower design.
  • The MAX_PREFIX_VARIANTS comment says 32 Box<[u8]> fit "on one cache line". They take 512 bytes.
  • Alternations of single bytes get lowered to classes ((?:H|h)ttps? becomes [Hh]ttps?), so they are not recognized. Expanding small byte classes in prefix_variants would widen coverage.
  • The full Core is still built, but it is only used for which_overlapping_matches and the cache. Build time and memory are unchanged, which is fine.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JKuwE1LGom89mpJoo8YDtj


Generated by Claude Code

Dandandan and others added 9 commits May 19, 2026 09:57
For anchored patterns of the shape

    ^<literal-prefix-set>([^X]+)X.*$    with replacement `${1}` (or `$1`)

capture 1's bounds are structurally trivial — skip the prefix, find the
terminator with memchr — so the engine doesn't need to track captures
at all.

Two changes work together:

1. A new `LiteralPrefixCapture` strategy in `regex-automata`'s meta
   engine recognizes the shape via HIR walking (single-pattern only,
   anchored at both ends, default flags, ASCII terminator, finite
   literal-alternation prefix set capped at 32 variants). Strategy
   methods extract the match and capture-1 slots directly with memchr,
   bypassing PikeVM / BoundedBacktracker. Wires in alongside the
   existing reverse strategies.

2. `Regex::replacen` gets a borrowed-output fast path for replacements
   that are exactly `$N` / `${N}`. Detected via a new
   `Replacer::single_capture_ref` method (default `None`, opted into
   for `&str`/`String`/`Cow<str>`). For `limit == 1` with a match
   covering the whole haystack, returns `Cow::Borrowed` of the
   captured slice — no `Captures::expand`, no output string
   allocation.

Bench (500k synthetic Referer rows, 5-iter mean, on the same machine):

    Regex::replacen, q28 pattern, 80% match
        before:  281 ms
        after:    39 ms   (7.3x)

    Regex::replacen, ^key=([^,]+),.*$, 100% match
        before:  113 ms
        after:    27 ms   (4.2x)

Tests: 257 / 257 pass (regex-automata --lib + --test integration, regex
--test integration). No regressions.
The fast path validated the *whole* haystack as UTF-8 whenever either
the capture class or the `.*` class was a Unicode class. That is too
strict when UTF-8 mode is disabled (e.g. `meta::Builder` with
`syntax::Config::new().utf8(false)`): the literal prefix and any
`(?-u:...)` span may legitimately contain invalid UTF-8. For example,
`^((?-u:[^/])+)/.*$` failed to match `b"\xA9/"` and
`^(?-u:\xFF)([^/]+)/.*$` failed to match `b"\xFFab/x"`.

Now we record separately whether the capture and the tail need UTF-8,
validate exactly those spans, and treat a failure as "this prefix does
not match" instead of "no match", since another prefix yields a
different capture span.

Because the validation is now exact, Unicode classes no longer need to
be gated on `utf8_empty`, which lets `regex::bytes::Regex` (Unicode
mode) use the fast path too.

Found by differential fuzzing against the PikeVM.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JKuwE1LGom89mpJoo8YDtj
The memchr2 loop over the terminator and `\n` stopped at each newline
only to resume the scan, because `[^X]+` may contain newlines. The
result is always the first terminator, so a plain memchr is equivalent
and simpler. (Benchmarks are neutral on URL-shaped inputs.)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JKuwE1LGom89mpJoo8YDtj
For `replace` (limit == 1):

* With a `$N` replacement for a small N, search with stack-allocated
  slots via `meta::Regex::search_slots` instead of allocating a
  `Captures` value.
* With any other template, call `captures` directly instead of going
  through `captures_iter`, which allocates a `Captures` on construction
  and then clones it for each match.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JKuwE1LGom89mpJoo8YDtj
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.

2 participants