HOLD (RSS trade): perf(regex): stop charging operation scratch to GC pressure; grow lent registers (bounded)

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.

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;

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::<u8>::new(&memory, 64).unwrap();
-        assert_eq!(external_side_live_bytes(), before + memory.live_bytes());
+        assert_eq!(memory.live_bytes(), 64);
         assert!(matches!(
             Buffer::<u8>::new(&memory, 65),
             Err(StorageError::Limit)
         ));
         let replacement = Buffer::<u8>::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(|| {

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::<StringHeader, _>(|pattern| {
+        flags.with_const_ptr::<StringHeader, _>(|flags| crate::regex::js_regexp_new(pattern, flags))
+    }))
+}
+
+fn search(
+    re: &RuntimeHandle<'_>,
+    input: &RuntimeHandle<'_>,
+) -> Result<Option<(usize, usize)>, EngineError> {
+    re.with_mut_ptr::<RegExpHeader, _>(|re| {
+        input.with_const_ptr::<StringHeader, _>(|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::<usize>::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::<usize>::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"
+    );
+}

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(_));

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,37 +84,31 @@ 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,
 }
 impl<'a> Reservation<'a> {
     pub(super) fn new(budget: &'a MemoryBudget, bytes: usize) -> Result<Self, StorageError> {
         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<T>,
     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<T: Copy + Default> DerefMut for Buffer<'_, T> {
 impl<T: Copy + Default> 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);
-        }
     }
 }

crates/perry-runtime/src/regex/perex_runtime.rs

@@ -201,10 +201,21 @@ impl<T: Copy + Default, const N: usize> 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<usize>,
     frames: Vec<Frame>,
     undo: Vec<Undo>,
 }
@@ -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<ScratchCell> = 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<usize> = 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<Error = OwnerError>>(
             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<Error = OwnerError>>(
         }
     }
 
+    #[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,