Skip to content

perf(regex): build each unique pattern once at construction, not twice #6534

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.

Sign up for GitHub

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

Merged
proggeramlug merged 1 commit into from
Jul 17, 2026
Merged

perf(regex): build each unique pattern once at construction, not twice #6534

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
171 changes: 116 additions & 55 deletions crates/perry-runtime/src/regex.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,60 +260,110 @@ fn evict_regex_cache_if_full<V>(cache: &mut HashMap<(String, String), V>) {
}
}

/// Compile `(pattern, flags)` into the caches if absent, reporting whether
/// SOME engine accepted the flag-prefixed pattern. One NFA build total —
/// `js_regexp_new` used to build every unique pattern TWICE (once discarded
/// for validation at construction, once here for the cache), which doubled
/// regex cost during bundle startup where every module-level literal
/// constructs eagerly (the emoji-regex class of pattern costs milliseconds
/// per build).
///
/// Returns `true` when the pattern is usable: compiled by the `regex` crate
/// (cached in `REGEX_CACHE`), or by `fancy-regex` (cached in `FANCY_CACHE`,
/// with the never-match placeholder in `REGEX_CACHE` so non-fancy callers
/// don't crash — the fancy fallback is handled in `js_regexp_exec_fancy`).
/// Returns `false` when BOTH engines reject it — nothing is cached and the
/// caller decides whether that is a SyntaxError (see `js_regexp_new`'s
/// bare-pattern fallback for the flag-prefix size edge).
#[cfg(feature = "regex-engine")]
fn compile_and_cache_regex_checked(pattern: &str, flags: &str) -> bool {
let already = REGEX_CACHE.with(|cache| {
cache
.borrow()
.contains_key(&(pattern.to_string(), flags.to_string()))
});
if already {
return true;
Comment on lines +280 to +286

@coderabbitai coderabbitai Bot Jul 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Keep fancy placeholders and compiled entries under the same eviction lifecycle.

FANCY_CACHE can clear while REGEX_CACHE retains its placeholder. A later lookup treats that placeholder as a complete hit, so a valid fancy-only pattern silently becomes never-match and fancy_ptr remains null.

Clear corresponding placeholders when evicting FANCY_CACHE, or use one cache entry enum holding either the standard or fancy program. Add a regression that reconstructs a fancy pattern after asymmetric cache churn.

Also applies to: 320-343, 348-356

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-runtime/src/regex.rs` around lines 280 - 286, Update the cache
handling around REGEX_CACHE and FANCY_CACHE so evicting a fancy entry also
removes its corresponding placeholder from REGEX_CACHE, preventing stale
placeholders from being treated as complete matches. Preserve standard compiled
entries, ensure a subsequent fancy lookup reconstructs the program and sets
fancy_ptr, and add a regression covering asymmetric cache churn followed by
reconstruction of a fancy-only pattern.

}
// Translate JS regex to Rust-compatible pattern
let translated = js_regex_to_rust(pattern);
let case_insensitive = flags.contains('i');
let multiline = flags.contains('m');
// #2828: the `s` (dotAll) flag maps directly onto the Rust `regex`
// crate's `(?s)` inline mode, so `.` matches newlines.
let dot_all = flags.contains('s');
let regex_pattern = if case_insensitive || multiline || dot_all {
let mut prefix = String::from("(?");
if case_insensitive {
prefix.push('i');
}
if multiline {
prefix.push('m');
}
if dot_all {
prefix.push('s');
}
prefix.push(')');
format!("{}{}", prefix, translated)
} else {
translated
};
let regex = match build_std_regex(&regex_pattern) {
Ok(re) => re,
Err(_) => {
// Pattern has features regex crate doesn't support
// (lookbehind, lookahead). Try fancy-regex which supports
// the full JS regex feature set, and if it compiles, wrap
// the result via a find-and-replace approach at the exec
// call sites. Store a never-matching pattern so existing
// callers don't crash.
let fancy_ok = FANCY_CACHE.with(|fc| {
if let Ok(fre) = build_fancy_regex(&regex_pattern) {
let mut fc = fc.borrow_mut();
evict_regex_cache_if_full(&mut fc);
fc.insert(
(pattern.to_string(), flags.to_string()),
std::sync::Arc::new(fre),
);
true
} else {
false
}
});
if !fancy_ok {
return false;
}
Regex::new(r"[^\s\S]").unwrap()
}
};
REGEX_CACHE.with(|cache| {
let mut cache = cache.borrow_mut();
evict_regex_cache_if_full(&mut cache);
cache.insert((pattern.to_string(), flags.to_string()), Arc::new(regex));
});
true
}

#[cfg(feature = "regex-engine")]
fn get_or_compile_regex(pattern: &str, flags: &str) -> Arc<Regex> {
let hit = REGEX_CACHE.with(|cache| {
cache
.borrow()
.get(&(pattern.to_string(), flags.to_string()))
.cloned()
});
if let Some(re) = hit {
return re;
}
let _ = compile_and_cache_regex_checked(pattern, flags);
REGEX_CACHE.with(|cache| {
let mut cache = cache.borrow_mut();
if let Some(re) = cache.get(&(pattern.to_string(), flags.to_string())) {
return re.clone();
}
// Translate JS regex to Rust-compatible pattern
let translated = js_regex_to_rust(pattern);
let case_insensitive = flags.contains('i');
let multiline = flags.contains('m');
// #2828: the `s` (dotAll) flag maps directly onto the Rust `regex`
// crate's `(?s)` inline mode, so `.` matches newlines.
let dot_all = flags.contains('s');
let regex_pattern = if case_insensitive || multiline || dot_all {
let mut prefix = String::from("(?");
if case_insensitive {
prefix.push('i');
}
if multiline {
prefix.push('m');
}
if dot_all {
prefix.push('s');
}
prefix.push(')');
format!("{}{}", prefix, translated)
} else {
translated
};
let regex = match build_std_regex(&regex_pattern) {
Ok(re) => re,
Err(_) => {
// Pattern has features regex crate doesn't support
// (lookbehind, lookahead). Try fancy-regex which supports
// the full JS regex feature set, and if it compiles, wrap
// the result via a find-and-replace approach at the exec
// call sites. For now, store a never-matching pattern so
// existing callers don't crash — the fancy-regex fallback
// is handled in js_regexp_exec_fancy below.
FANCY_CACHE.with(|fc| {
if let Ok(fre) = build_fancy_regex(&regex_pattern) {
let mut fc = fc.borrow_mut();
evict_regex_cache_if_full(&mut fc);
fc.insert(
(pattern.to_string(), flags.to_string()),
std::sync::Arc::new(fre),
);
}
});
Regex::new(r"[^\s\S]").unwrap()
}
};
let arc = Arc::new(regex);
// Both engines rejected it (validation normally throws before this
// point) — keep the historical behavior: cache + return never-match.
let arc = Arc::new(Regex::new(r"[^\s\S]").unwrap());
evict_regex_cache_if_full(&mut cache);
cache.insert((pattern.to_string(), flags.to_string()), arc.clone());
arc
Expand Down Expand Up @@ -651,14 +701,25 @@ pub extern "C" fn js_regexp_new(
pattern_str
));
}
// The expensive part of validation: compile the pattern with both
// engines just to confirm it is well-formed.
let translated = js_regex_to_rust(pattern_str);
if build_std_regex(&translated).is_err() && build_fancy_regex(&translated).is_err() {
throw_regexp_syntax_error(&format!(
"Invalid regular expression: /{}/: invalid pattern",
pattern_str
));
// The expensive part of validation: compile the pattern. This
// BUILDS AND CACHES in one step (`compile_and_cache_regex_checked`)
// so the `get_or_compile_regex` below is a guaranteed cache hit —
// previously every unique pattern was NFA-compiled twice (once
// discarded here, once for the cache), doubling startup regex cost.
if !compile_and_cache_regex_checked(pattern_str, flags_str) {
// Preserve the historical edge: validation used to test the
// BARE translated pattern (no `(?ims)` prefix). A pattern that
// compiles bare but blows the size limit with the flag prefix
// must stay a silent never-match (matching prior behavior),
// not a SyntaxError.
let translated = js_regex_to_rust(pattern_str);
if build_std_regex(&translated).is_err() && build_fancy_regex(&translated).is_err()
{
throw_regexp_syntax_error(&format!(
"Invalid regular expression: /{}/: invalid pattern",
pattern_str
));
}
}
}
}
Expand Down
Loading