perf(regex): two per-match overheads in the replace path

changelog.d/10605-replacer-native-args.md

@@ -0,0 +1,3 @@
+### Faster
+
+- `String.prototype.replace` with a function replacement spends about 27% fewer instructions, and roughly a quarter less on non-ASCII subjects. Each match used to build a JS array to carry the replacer's arguments, growing it one push at a time and then reading it straight back out into the native slots the call actually uses; the arguments now go directly into those slots, which were already the collector's roots for them. A proxy replacer, whose `apply` trap really does receive an arguments array, is unaffected (#10165).

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

@@ -20,7 +20,9 @@ use super::perex_api::{self as api, ExecOutput, Reuse};
 use super::perex_match_search::advance;
 use super::perex_memory::{MemoryBudget, StorageError};
 use super::perex_owner::HeapSubject;
-use super::perex_replace_storage::{boxed, call, length, text, List, Pieces, Units};
+use super::perex_replace_storage::{
+    boxed, call, call_native, length, text, List, NativeArgs, Pieces, Units,
+};
 use super::perex_runtime::{self as host, EngineError};
 use super::perex_strings::SpanCopies;
 use super::RegExpHeader;
@@ -251,6 +253,16 @@ pub(super) fn replace(
     } else {
         Pieces::new(scope)?
     };
+    // An ordinary replacer's arguments never reach user code as an array, so
+    // they are produced straight into shadow-stack slots. A proxy replacer's do
+    // reach it, through the `apply` trap, and keep the JS array. Sized once:
+    // the program's capture count fixes the argument count for every match.
+    let mut native_args =
+        if tokens.is_some() || crate::proxy::js_proxy_is_proxy(replacement.get_nanbox_f64()) == 1 {
+            None
+        } else {
+            Some(NativeArgs::new(captures + 3)?)
+        };
     let mut next_source = 0;
     for record in spans.values.chunks_exact(width) {
         let local = RuntimeHandleScope::new();
@@ -284,6 +296,30 @@ pub(super) fn replace(
                     }
                 }
             }
+        } else if let Some(args) = native_args.as_mut() {
+            let this = local.root_nanbox_f64(f64::from_bits(TAG_UNDEFINED));
+            let copies = &mut copies;
+            let value = call_native(replacement, &this, args, memory, |set| {
+                let matched = copies.copy(start, end, budget)?;
+                set(0, js_nanbox_string(matched as i64));
+                let mut slot = 1;
+                for pair in record[2..].as_chunks::<2>().0 {
+                    // An unset capture is the `undefined` the slot already holds.
+                    if pair[0] != u32::MAX {
+                        let capture = copies.copy(pair[0] as usize, pair[1] as usize, budget)?;
+                        set(slot, js_nanbox_string(capture as i64));
+                    }
+                    slot += 1;
+                }
+                set(slot, position as f64);
+                set(slot + 1, boxed(input));
+                Ok(())
+            })?;
+            let value = local.root_nanbox_f64(value);
+            let value = text(&local, &value)?;
+            if accepted {
+                output.whole(&value, budget)?;
+            }
         } else {
             let mut args = List::new(&local)?;
             let matched = copies.copy(start, end, budget)?;

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

@@ -130,6 +130,90 @@ pub(super) fn call(
     result
 }
 
+/// Arguments for a replacer call, produced straight into shadow-stack slots.
+///
+/// `call` builds its slots by copying a JS array the caller filled one push at
+/// a time. For a replacer invoked once per match that array is pure overhead:
+/// it is allocated, grown as each argument is pushed, read back out, and
+/// dropped, and no user code can observe it. Here the slots are bound to the
+/// shadow stack *before* any argument is produced, so a value is a traced root
+/// from the moment it is written and producing the next one may allocate and
+/// collect freely. The buffer is sized once and outlives the match loop.
+///
+/// Not usable for a proxy replacer: `js_proxy_apply` takes the arguments as a
+/// JS array, which the `apply` trap observes, so that path keeps `List`.
+pub(super) struct NativeArgs {
+    slots: Vec<std::cell::UnsafeCell<f64>>,
+}
+
+impl NativeArgs {
+    pub(super) fn new(count: usize) -> Result<Self, EngineError> {
+        let mut slots = Vec::new();
+        slots
+            .try_reserve_exact(count)
+            .map_err(|_| StorageError::Allocation)?;
+        for _ in 0..count {
+            slots.push(std::cell::UnsafeCell::new(f64::from_bits(
+                crate::value::TAG_UNDEFINED,
+            )));
+        }
+        Ok(Self { slots })
+    }
+}
+
+/// `call` for an ordinary replacer, with the arguments produced by `fill`
+/// directly into `args`.
+///
+/// Every slot is reset to `undefined` and bound before `fill` runs, so an
+/// argument `fill` does not write stays `undefined` (an unset capture), and one
+/// it does write is rooted immediately. `fill` may therefore allocate between
+/// arguments, which is what producing a match's capture strings does.
+pub(super) fn call_native(
+    method: &RuntimeHandle<'_>,
+    receiver: &RuntimeHandle<'_>,
+    args: &mut NativeArgs,
+    memory: &MemoryBudget,
+    fill: impl FnOnce(&mut dyn FnMut(usize, f64)) -> Result<(), EngineError>,
+) -> Result<f64, EngineError> {
+    struct Frame(u64);
+    impl Drop for Frame {
+        fn drop(&mut self) {
+            crate::gc::js_shadow_frame_pop(self.0);
+        }
+    }
+    let scope = RuntimeHandleScope::new();
+    let previous = scope.root_nanbox_f64(crate::object::js_implicit_this_get());
+    let slots = &args.slots;
+    for slot in slots {
+        // SAFETY: nothing else holds a reference to these cells, and the
+        // shadow stack is not yet bound to them.
+        unsafe { *slot.get() = f64::from_bits(crate::value::TAG_UNDEFINED) };
+    }
+    let frame = Frame(crate::gc::js_shadow_frame_push(slots.len() as u32));
+    for (i, slot) in slots.iter().enumerate() {
+        crate::gc::js_shadow_slot_bind(i as u32, slot.get().cast());
+    }
+    // SAFETY as above; the slots are bound, so a write publishes a root.
+    let mut set = |i: usize, value: f64| unsafe { *slots[i].get() = value };
+    fill(&mut set)?;
+    let reservation = Reservation::new(
+        memory,
+        slots.len().checked_mul(8).ok_or(StorageError::Limit)?,
+    )?;
+    let result = api::caught(|| unsafe {
+        crate::object::js_implicit_this_set(receiver.get_nanbox_f64());
+        crate::closure::js_native_call_value(
+            method.get_nanbox_f64(),
+            slots.as_ptr().cast(),
+            slots.len(),
+        )
+    });
+    crate::object::js_implicit_this_set(previous.get_nanbox_f64());
+    drop(reservation);
+    drop(frame);
+    result
+}
+
 /// A reusable original-input reader. A read retains only Perex offsets across
 /// collection, and adjacent reads do not repeat the initial Unicode seek.
 pub(super) struct Units<'a, 's> {

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

@@ -89,6 +89,53 @@ fn js_replacement_expands_special_patterns() {
     }
 }
 
+/// A replacer's argument buffer is reused across matches, so it must not carry
+/// one match's capture into the next. `(a)|(b)` over "aba" participates in
+/// capture 1, then capture 2, then capture 1 again, so each match has an unset
+/// capture that the previous match set.
+///
+/// This is a regression test with a measured reason to exist: with the reset in
+/// `call_native` removed, the whole 3,984-test lib suite still passed, and this
+/// case returns `<a,undefined><a,b><a,b>` instead of the correct answer.
+#[test]
+fn direct_replace_callback_does_not_carry_a_capture_between_matches() {
+    let _lock = crate::gc::global_side_table_test_lock();
+    let scope = crate::gc::RuntimeHandleScope::new();
+    let pattern = scope.root_string_ptr(make_string("(a)|(b)"));
+    let flags = scope.root_string_ptr(make_string("g"));
+    let re = scope.root_raw_mut_ptr(
+        pattern.with_const_ptr(|p| flags.with_const_ptr(|f| js_regexp_new(p, f))),
+    );
+    let subject = scope.root_string_ptr(make_string("aba"));
+    let replacer = scope.root_nanbox_f64(crate::dyn_eval::dyn_function_from_strings(&[
+        "m".to_string(),
+        "p1".to_string(),
+        "p2".to_string(),
+        "return '<' + String(p1) + ',' + String(p2) + '>';".to_string(),
+    ]));
+    let before = super::perex_replace_direct::direct_replaces();
+    let out = subject.with_const_ptr::<StringHeader, _>(|s| {
+        re.with_const_ptr::<RegExpHeader, _>(|r| {
+            super::perex_replace::js_string_replace_js(
+                crate::value::js_nanbox_string(s as i64),
+                crate::value::js_nanbox_pointer(r as i64),
+                replacer.get_nanbox_f64(),
+            )
+        })
+    });
+    // Without this the test would pass just as well against the exec-object
+    // fallback, which has no shared buffer and so cannot show the bug.
+    assert!(
+        super::perex_replace_direct::direct_replaces() > before,
+        "did not reach the direct replace path, so this test proves nothing"
+    );
+    let out = crate::value::js_nanbox_get_pointer(out) as *const StringHeader;
+    assert_eq!(
+        string_as_str(out),
+        "<a,undefined><undefined,b><a,undefined>"
+    );
+}
+
 fn replace_case(pattern: &str, subject: &str, replacement: &str) -> String {
     let scope = crate::gc::RuntimeHandleScope::new();
     let pattern = scope.root_string_ptr(make_string(pattern));