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. 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,