From ddab9387df3b4ce78d245eef67530a9b14aed0a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 28 Sep 2026 08:37:34 +0000 Subject: [PATCH 1/2] perf(regex): stop charging operation scratch to GC external pressure; grow the lent registers to a fixed bound Regex operation scratch (perex_memory::Buffer / Reservation: the owned search path's match buffers, compile scratch, KMP failure tables, replacer argument slots) is freed by the operation that allocated it and bounded by that operation's MemoryBudget. Reporting it through gc_note_external_side_alloc/free put every per-call release into GC_EXTERNAL_SIDE_DRAINED_SINCE_FULL, which old-reclaim holds as pressure until the next full, so a loop over a program with more than 32 registers (dotenv's LINE has 42) ran a full mark-sweep every few hundred calls on bytes no collection could free. The lent per-thread scratch cell now grows its registers on demand up to LENT_REGISTERS = 1024 (8 KiB per thread), so such programs stop building per-call buffers at all. Programs past the bound keep the owned path. Subject-proportional storage (replace Spans, native piece records) is unchanged and still reported. Part of #11549 --- .../src/gc/tests/runtime_roots.rs | 2 + .../gc/tests/runtime_roots/perex_execution.rs | 10 +- .../runtime_roots/perex_scratch_pressure.rs | 140 ++++++++++++++++++ crates/perry-runtime/src/regex/perex_api.rs | 4 +- .../perry-runtime/src/regex/perex_memory.rs | 61 ++++---- .../perry-runtime/src/regex/perex_runtime.rs | 47 +++++- 6 files changed, 222 insertions(+), 42 deletions(-) create mode 100644 crates/perry-runtime/src/gc/tests/runtime_roots/perex_scratch_pressure.rs diff --git a/crates/perry-runtime/src/gc/tests/runtime_roots.rs b/crates/perry-runtime/src/gc/tests/runtime_roots.rs index b78396beca..906a9ef2a5 100644 --- a/crates/perry-runtime/src/gc/tests/runtime_roots.rs +++ b/crates/perry-runtime/src/gc/tests/runtime_roots.rs @@ -48,6 +48,8 @@ mod perex_replace_direct; #[cfg(feature = "regex-engine")] mod perex_reuse; #[cfg(feature = "regex-engine")] +mod perex_scratch_pressure; +#[cfg(feature = "regex-engine")] mod perex_split; #[cfg(feature = "regex-engine")] mod perex_strings; diff --git a/crates/perry-runtime/src/gc/tests/runtime_roots/perex_execution.rs b/crates/perry-runtime/src/gc/tests/runtime_roots/perex_execution.rs index 572fa5b9f6..c2cdfac93b 100644 --- a/crates/perry-runtime/src/gc/tests/runtime_roots/perex_execution.rs +++ b/crates/perry-runtime/src/gc/tests/runtime_roots/perex_execution.rs @@ -41,20 +41,24 @@ fn compile<'s>( fn perex_host_buffers_account_overlap_failure_and_unwind() { let _guard = CopyingNurseryTestGuard::new(0); let _triggers = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + // #11549: operation scratch is bounded by its budget and released by the + // operation, so the budget accounts the overlap and the collector's + // external-side reading never moves. let before = external_side_live_bytes(); let memory = MemoryBudget::new(128); { let first = Buffer::::new(&memory, 64).unwrap(); - assert_eq!(external_side_live_bytes(), before + memory.live_bytes()); + assert_eq!(memory.live_bytes(), 64); assert!(matches!( Buffer::::new(&memory, 65), Err(StorageError::Limit) )); let replacement = Buffer::::new(&memory, 64).unwrap(); assert_eq!(memory.peak_bytes(), 128); - assert_eq!(external_side_live_bytes(), before + 128); + assert_eq!(memory.live_bytes(), 128); + assert_eq!(external_side_live_bytes(), before); drop(first); - assert_eq!(external_side_live_bytes(), before + 64); + assert_eq!(memory.live_bytes(), 64); drop(replacement); } let unwind = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { diff --git a/crates/perry-runtime/src/gc/tests/runtime_roots/perex_scratch_pressure.rs b/crates/perry-runtime/src/gc/tests/runtime_roots/perex_scratch_pressure.rs new file mode 100644 index 0000000000..0589419798 --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/runtime_roots/perex_scratch_pressure.rs @@ -0,0 +1,140 @@ +//! #11549: a regex operation's own scratch is not collector pressure. +//! +//! Operation scratch (`perex_memory::Buffer`, the owned search path's match +//! buffers, the lent cell) is freed by the operation, never by a collection. +//! Reporting it as external side bytes put every per-call buffer's release into +//! `GC_EXTERNAL_SIDE_DRAINED_SINCE_FULL`, which old-reclaim holds as pressure +//! until the next full: dotenv's `LINE` (42 registers, past the lent cell's old +//! fixed 32) ran a budgeted full mark-sweep every ~250 parses on phantom bytes. +use super::*; +use crate::gc::policy::{external_side_live_bytes, GC_EXTERNAL_SIDE_DRAINED_SINCE_FULL}; +use crate::regex::perex_api as api; +use crate::regex::perex_memory::{Buffer, MemoryBudget}; +use crate::regex::perex_runtime::{EngineError, OWNED_SEARCHES}; +use crate::regex::RegExpHeader; +use crate::string::StringHeader; + +fn text<'s>(scope: &'s RuntimeHandleScope, bytes: &[u8]) -> RuntimeHandle<'s> { + scope.root_string_ptr(crate::string::js_string_from_bytes( + bytes.as_ptr(), + bytes.len() as u32, + )) +} + +fn regex<'s>(scope: &'s RuntimeHandleScope, pattern: &str) -> RuntimeHandle<'s> { + let pattern = text(scope, pattern.as_bytes()); + let flags = text(scope, b""); + scope.root_raw_mut_ptr(pattern.with_const_ptr::(|pattern| { + flags.with_const_ptr::(|flags| crate::regex::js_regexp_new(pattern, flags)) + })) +} + +fn search( + re: &RuntimeHandle<'_>, + input: &RuntimeHandle<'_>, +) -> Result, EngineError> { + re.with_mut_ptr::(|re| { + input.with_const_ptr::(|input| { + api::execute(re, input, false, &mut || Ok(())) + .map(|found| found.map(|m| (m.full.start(), m.full.end()))) + }) + }) +} + +/// `groups` capturing groups of one `a` each: `(a)(a)...`. A program's register +/// count is at least twice its capture count (group zero included), so this +/// needs at least `2 * (groups + 1)` registers. +fn groups_pattern(groups: usize) -> String { + "(a)".repeat(groups) +} + +fn external_readings() -> (usize, usize) { + ( + external_side_live_bytes(), + GC_EXTERNAL_SIDE_DRAINED_SINCE_FULL.with(TriggerInput::get), + ) +} + +fn owned_searches() -> usize { + OWNED_SEARCHES.with(std::cell::Cell::get) +} + +#[test] +fn perex_operation_buffer_is_not_reported_as_external_side_bytes() { + let _guard = CopyingNurseryTestGuard::new(0); + let _triggers = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + let before = external_readings(); + let memory = MemoryBudget::new(1 << 20); + { + let buffer = Buffer::::new(&memory, 4096).expect("fits the budget"); + assert_eq!(buffer.len(), 4096); + assert_eq!(memory.live_bytes(), 4096 * 8, "the budget still sees it"); + assert_eq!(external_readings(), before, "allocation noted nothing"); + } + assert_eq!(memory.live_bytes(), 0); + assert_eq!(memory.peak_bytes(), 4096 * 8); + assert_eq!( + external_readings(), + before, + "a released operation buffer is not drained external pressure (#11549)" + ); + // The budget remains the bound: past it, nothing is allocated. + assert!(Buffer::::new(&memory, (1 << 20) / 8 + 1).is_err()); +} + +#[test] +fn perex_search_past_32_registers_uses_the_lent_cell_and_notes_nothing() { + let _guard = CopyingNurseryTestGuard::new(0); + let _triggers = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + super::perex_public::register_host_roots(); + let scope = RuntimeHandleScope::new(); + // 20 groups: 42+ registers, which the lent cell's old fixed 32 could not + // hold, so every call built and dropped owned match buffers. + let re = regex(&scope, &groups_pattern(20)); + let input = text(&scope, "a".repeat(25).as_bytes()); + // The first call may grow the thread's cell; every later one must not + // build anything. + assert_eq!(search(&re, &input).unwrap(), Some((0, 20))); + let before = external_readings(); + let owned = owned_searches(); + for _ in 0..64 { + assert_eq!(search(&re, &input).unwrap(), Some((0, 20))); + } + assert_eq!( + owned_searches(), + owned, + "a program within the lent bound searches on the lent cell" + ); + assert_eq!( + external_readings(), + before, + "and reports no scratch to the collector" + ); +} + +#[test] +fn perex_search_past_the_lent_bound_takes_the_owned_path_and_notes_nothing() { + let _guard = CopyingNurseryTestGuard::new(0); + let _triggers = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + super::perex_public::register_host_roots(); + let scope = RuntimeHandleScope::new(); + // 600 groups: 1,202+ registers, past what the lent cell may retain. The + // cell must not grow to it; the operation builds, and frees, its own. + let re = regex(&scope, &groups_pattern(600)); + let input = text(&scope, "a".repeat(600).as_bytes()); + let before = external_readings(); + let owned = owned_searches(); + for _ in 0..4 { + assert_eq!(search(&re, &input).unwrap(), Some((0, 600))); + } + assert_eq!( + owned_searches(), + owned + 4, + "a program past the lent bound runs the owned path every call" + ); + assert_eq!( + external_readings(), + before, + "owned match buffers are released by the search, not drained into GC pressure" + ); +} diff --git a/crates/perry-runtime/src/regex/perex_api.rs b/crates/perry-runtime/src/regex/perex_api.rs index a3cc464fe2..e061698480 100644 --- a/crates/perry-runtime/src/regex/perex_api.rs +++ b/crates/perry-runtime/src/regex/perex_api.rs @@ -48,9 +48,7 @@ fn raise(error: EngineError) -> ! { if let EngineError::Abrupt(value) = error { crate::exception::js_throw(value); } - if let EngineError::Build(BuildError::Abrupt(bits)) - | EngineError::Storage(StorageError::Abrupt(bits)) = error - { + if let EngineError::Build(BuildError::Abrupt(bits)) = error { crate::exception::js_throw(f64::from_bits(bits)); } let type_error = matches!(error, EngineError::Type(_)); diff --git a/crates/perry-runtime/src/regex/perex_memory.rs b/crates/perry-runtime/src/regex/perex_memory.rs index 5a72bbedcc..c8afa4b7ad 100644 --- a/crates/perry-runtime/src/regex/perex_memory.rs +++ b/crates/perry-runtime/src/regex/perex_memory.rs @@ -1,6 +1,22 @@ -//! Operation-owned native scratch, charged to Perry's external-byte budget. -//! No GC pointer may be stored in these buffers. Allocation/accounting can -//! collect, so callers must release all program/subject views first. +//! Operation-owned native scratch, bounded by the operation's `MemoryBudget`. +//! No GC pointer may be stored in these buffers. +//! +//! This scratch is NOT reported to the collector as external side bytes +//! (#11549). Everything here is freed by the operation that allocated it, +//! when that operation returns or unwinds; no collection can ever reclaim a +//! byte of it, and no collection is needed for it to be released. Reporting it +//! told the old-reclaim pacing the opposite: every per-call buffer's release +//! landed in `GC_EXTERNAL_SIDE_DRAINED_SINCE_FULL`, which is held as pressure +//! until the next FULL collection, so a regex loop that took the owned search +//! path (a program with more registers than the lent cell holds) was paced by +//! phantom bytes into a budgeted full mark-sweep every few hundred calls. +//! +//! What bounds it instead is the budget: every buffer, inline charge and +//! reservation is checked against the operation's hard limit +//! (`perex_api::SCRATCH_BYTES`) before it exists, so one operation can never +//! hold more than that. Storage whose size follows the SUBJECT rather than +//! that limit (`perex_replace_direct::Spans`, `perex_replace_storage`'s +//! native piece records) is not scratch in this sense and is still reported. use std::cell::Cell; use std::ops::{Deref, DerefMut}; @@ -9,7 +25,6 @@ use std::ops::{Deref, DerefMut}; pub(crate) enum StorageError { Limit, Allocation, - Abrupt(u64), } /// One operation's hard scratch/result-metadata limit. Simultaneous old/new @@ -69,8 +84,11 @@ impl Drop for Charge<'_> { } } -/// Account a stable native allocation whose GC-bearing slots are separately -/// registered with the host's mutable root scanner before this can collect. +/// Charge a stable native allocation the caller owns, such as a replacer +/// call's argument slots, to the operation's limit. Its GC-bearing slots are +/// registered with the shadow stack separately; like every other buffer here +/// it is released by the operation, not by a collection, so the collector is +/// not told about it. pub(super) struct Reservation<'a> { budget: &'a MemoryBudget, bytes: usize, @@ -78,28 +96,19 @@ pub(super) struct Reservation<'a> { impl<'a> Reservation<'a> { pub(super) fn new(budget: &'a MemoryBudget, bytes: usize) -> Result { let live = budget.check(bytes)?; - let owned = Self { budget, bytes }; budget.live.set(live); budget.peak.set(budget.peak.get().max(live)); - if bytes != 0 { - crate::exception::catch_js_throw(|| crate::gc::gc_note_external_side_alloc(bytes)) - .map_err(|value| StorageError::Abrupt(value.to_bits()))?; - } - Ok(owned) + Ok(Self { budget, bytes }) } } impl Drop for Reservation<'_> { fn drop(&mut self) { self.budget.live.set(self.budget.live.get() - self.bytes); - if self.bytes != 0 { - crate::gc::gc_note_external_side_free(self.bytes); - } } } -/// Stable initialized native allocation. Its accounting owner is established -/// before notifying the collector, so a collecting/unwinding notification -/// cannot strand a buffer or leave its bytes charged. +/// Stable initialized native allocation, charged to the operation's limit for +/// as long as it lives. Creating one never collects. pub(crate) struct Buffer<'a, T: Copy + Default> { data: Vec, budget: &'a MemoryBudget, @@ -121,18 +130,13 @@ impl<'a, T: Copy + Default> Buffer<'a, T> { .ok_or(StorageError::Limit)?; let live = budget.check(bytes)?; data.resize(count, T::default()); - let owned = Self { + budget.live.set(live); + budget.peak.set(budget.peak.get().max(live)); + Ok(Self { data, budget, bytes, - }; - budget.live.set(live); - budget.peak.set(budget.peak.get().max(live)); - if bytes != 0 { - crate::exception::catch_js_throw(|| crate::gc::gc_note_external_side_alloc(bytes)) - .map_err(|value| StorageError::Abrupt(value.to_bits()))?; - } - Ok(owned) + }) } } @@ -152,8 +156,5 @@ impl DerefMut for Buffer<'_, T> { impl Drop for Buffer<'_, T> { fn drop(&mut self) { self.budget.live.set(self.budget.live.get() - self.bytes); - if self.bytes != 0 { - crate::gc::gc_note_external_side_free(self.bytes); - } } } diff --git a/crates/perry-runtime/src/regex/perex_runtime.rs b/crates/perry-runtime/src/regex/perex_runtime.rs index 9cafb56082..8b8cdb628f 100644 --- a/crates/perry-runtime/src/regex/perex_runtime.rs +++ b/crates/perry-runtime/src/regex/perex_runtime.rs @@ -201,10 +201,21 @@ impl std::ops::DerefMut for Slots<'_, T, N> { /// fewer: `/^[a-z]+_[0-9]+$/` needs 2. Frames and undo entries start empty and /// only grow through `rebuffer`, so they are never inline. const INLINE_REGISTERS: usize = 8; -/// Registers the lent cell holds. This array is allocated once per thread, not -/// per call, so it is sized for the programs a search may bring rather than -/// for what is cheap to move. -const LENT_REGISTERS: usize = 32; +/// The most registers the lent cell will grow to hold (#11549). The cell's +/// registers are allocated once per thread and kept, so a program up to this +/// size searches without building per-call buffers after its first call. +/// +/// This is the bound on what the cell retains for registers, and the reason +/// it needs no collector accounting: at most `LENT_REGISTERS * 8` bytes +/// (8 KiB) per thread, whatever programs run. dotenv's `LINE` needs 42 (the +/// old fixed 32 sent every one of its searches down the owned path); a +/// program past the bound (hundreds of capture groups) still takes the owned +/// path, whose buffers the operation's `MemoryBudget` bounds and the +/// operation frees. +/// +/// A register count is a property of the program, so the choice between the +/// two paths is made before any work, never by a failed attempt. +const LENT_REGISTERS: usize = 1024; /// Capture spans an `exec` result can have and still be read without /// allocating. const INLINE_CAPTURES: usize = 16; @@ -253,7 +264,9 @@ impl ScratchOwner for MatchBuffers<'_> { /// frames and undo entries are the engine's own opaque scratch, exactly as in /// the owned buffers this replaces (see this module's header). struct ScratchCell { - registers: [usize; LENT_REGISTERS], + /// Grown on demand to the largest register count a search on this thread + /// has needed, never past `LENT_REGISTERS`. + registers: Vec, frames: Vec, undo: Vec, } @@ -270,7 +283,7 @@ crate::perry_thread_local! { /// costing a `_tlv_get_addr` call — the opposite of what this change is for. static LENT_SCRATCH: std::cell::RefCell = const { std::cell::RefCell::new(ScratchCell { - registers: [0; LENT_REGISTERS], + registers: Vec::new(), frames: Vec::new(), undo: Vec::new(), }) @@ -306,6 +319,13 @@ crate::perry_thread_local! { const { std::cell::Cell::new(0) }; } +#[cfg(test)] +crate::perry_thread_local! { + /// Test-only: how many searches took the owned path (built per-call + /// `MatchBuffers`), so a test can assert which path a program took. + pub(crate) static OWNED_SEARCHES: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + /// Run the pre-search poll on one call in `PRE_SEARCH_POLL_STRIDE`. #[inline] fn poll_on_stride(poll: &mut impl FnMut() -> Result<(), EngineError>) -> Result<(), EngineError> { @@ -398,6 +418,19 @@ fn find_near_lent<'mem, S: ImmutableSubject>( return Ok(Lent::Fallback); }; let cell = &mut *cell; + if cell.registers.len() < registers { + // Once per thread per new high-water mark; `find_near` has already + // checked `registers <= LENT_REGISTERS`. A search initializes the + // registers it reads, so the fill value is never observed. + if cell + .registers + .try_reserve_exact(registers - cell.registers.len()) + .is_err() + { + return Ok(Lent::Fallback); + } + cell.registers.resize(registers, 0); + } // Charged exactly like the owner it replaces: the operation's limit // sees the slots a search may use, whether or not they were allocated // for it. The thread keeps the memory; the operation only borrows it. @@ -552,6 +585,8 @@ pub(crate) fn find_near<'mem, S: ImmutableSubject>( } } + #[cfg(test)] + OWNED_SEARCHES.with(|n| n.set(n.get() + 1)); poll()?; let buffers = MatchBuffers::new(memory, size)?; // On an error that is not a capacity request -- WorkLimit, From a7bdf8ab9124a1fd7c5aeaa998971101fd780379 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 28 Sep 2026 08:38:37 +0000 Subject: [PATCH 2/2] changelog: #11612 regex scratch is not GC pressure --- changelog.d/11612-regex-scratch-not-gc-pressure.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/11612-regex-scratch-not-gc-pressure.md diff --git a/changelog.d/11612-regex-scratch-not-gc-pressure.md b/changelog.d/11612-regex-scratch-not-gc-pressure.md new file mode 100644 index 0000000000..275eb59a86 --- /dev/null +++ b/changelog.d/11612-regex-scratch-not-gc-pressure.md @@ -0,0 +1 @@ +perf(regex): regex operation scratch is no longer reported to the collector as external side bytes (#11549). The owned search path's per-call match buffers, compile scratch, KMP tables and replacer argument slots are freed by the operation and bounded by its `MemoryBudget`, but each release fed `GC_EXTERNAL_SIDE_DRAINED_SINCE_FULL` and paced loops over >32-register programs (dotenv's `LINE`) into a full mark-sweep every few hundred calls. The lent per-thread scratch cell now grows its registers on demand up to 1024 (8 KiB per thread), so such programs stop building per-call buffers. Subject-proportional replace storage is still reported. Instructions: dotenv/parse −30.1%, moment/parse_format −13.8%, validator/batch −1.9%; peak RSS rises on moment/parse_format (+29%) because those loops now pace by the 16 MB nursery like other allocating programs — held pending the young-generation pacing half of #11549.