perf: gc refs for js tracing (#5264)

Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com>

crates/revm/revm-inspectors/src/tracing/js/bindings.rs

@@ -3,8 +3,8 @@
 use crate::tracing::{
     js::{
         builtins::{
-            address_to_buf, bytes_to_address, bytes_to_hash, from_buf, to_bigint, to_bigint_array,
+            address_to_buf, bytes_to_address, bytes_to_hash, from_buf, to_bigint, to_buf,
-            to_buf, to_buf_value,
+            to_buf_value,
         },
         JsDbRequest,
     },
@@ -15,7 +15,7 @@ use boa_engine::{
     object::{builtins::JsArrayBuffer, FunctionObjectBuilder},
     Context, JsArgs, JsError, JsNativeError, JsObject, JsResult, JsValue,
 };
-use boa_gc::{empty_trace, Finalize, Gc, Trace};
+use boa_gc::{empty_trace, Finalize, Trace};
 use reth_primitives::{Account, Address, Bytes, B256, KECCAK_EMPTY, U256};
 use revm::{
     interpreter::{
@@ -24,7 +24,7 @@ use revm::{
     },
     primitives::State,
 };
-use std::{borrow::Borrow, sync::mpsc::channel};
+use std::{cell::RefCell, rc::Rc, sync::mpsc::channel};
 use tokio::sync::mpsc;
 
 /// A macro that creates a native function that returns via [JsValue::from]
@@ -54,15 +54,79 @@ macro_rules! js_value_capture_getter {
     };
 }
 
+/// A reference to a value that can be garbagae collected, but will not give access to the value if
+/// it has been dropped.
+///
+/// This is used to allow the JS tracer functions to access values at a certain point during
+/// inspection by ref without having to clone them and capture them in the js object.
+///
+/// JS tracer functions get access to evm internals via objects or function arguments, for example
+/// `function step(log,evm)` where log has an object `stack` that has a function `peek(number)` that
+/// returns a value from the stack.
+///
+/// These functions could get garbage collected, however the data accessed by the function is
+/// supposed to be ephemeral and only valid for the duration of the function call.
+///
+/// This type supports garbage collection of (rust) references and prevents access to the value if
+/// it has been dropped.
+#[derive(Debug, Clone)]
+pub(crate) struct GuardedNullableGcRef<Val: 'static> {
+    /// The lifetime is a lie to make it possible to use a reference in boa which requires 'static
+    inner: Rc<RefCell<Option<&'static Val>>>,
+}
+
+impl<Val: 'static> GuardedNullableGcRef<Val> {
+    /// Creates a garbage collectible reference to the given reference.
+    ///
+    /// SAFETY; the caller must ensure that the guard is dropped before the value is dropped.
+    pub(crate) fn new(val: &Val) -> (Self, RefGuard<'_, Val>) {
+        let inner = Rc::new(RefCell::new(Some(val)));
+        let guard = RefGuard { inner: Rc::clone(&inner) };
+
+        // SAFETY: guard enforces that the value is removed from the refcell before it is dropped
+        let this = Self { inner: unsafe { std::mem::transmute(inner) } };
+
+        (this, guard)
+    }
+
+    /// Executes the given closure with a reference to the inner value if it is still present.
+    pub(crate) fn with_inner<F, R>(&self, f: F) -> Option<R>
+    where
+        F: FnOnce(&Val) -> R,
+    {
+        self.inner.borrow().map(f)
+    }
+}
+
+impl<Val: 'static> Finalize for GuardedNullableGcRef<Val> {}
+
+unsafe impl<Val: 'static> Trace for GuardedNullableGcRef<Val> {
+    empty_trace!();
+}
+
+/// Guard the inner references, once this value is dropped the inner reference is also removed.
+///
+/// This type guarantees that it never outlives the wrapped reference.
+#[derive(Debug)]
+pub(crate) struct RefGuard<'a, Val> {
+    inner: Rc<RefCell<Option<&'a Val>>>,
+}
+
+impl<'a, Val> Drop for RefGuard<'a, Val> {
+    fn drop(&mut self) {
+        self.inner.borrow_mut().take();
+    }
+}
+
 /// The Log object that is passed to the javascript inspector.
 #[derive(Debug)]
 pub(crate) struct StepLog {
     /// Stack before step execution
-    pub(crate) stack: StackObj,
+    pub(crate) stack: StackRef,
     /// Opcode to be executed
     pub(crate) op: OpObj,
     /// All allocated memory in a step
-    pub(crate) memory: MemoryObj,
+    pub(crate) memory: MemoryRef,
     /// Program counter before step execution
     pub(crate) pc: u64,
     /// Remaining gas before step execution
@@ -131,15 +195,23 @@ impl StepLog {
 }
 
 /// Represents the memory object
-#[derive(Debug)]
+#[derive(Debug, Clone)]
-pub(crate) struct MemoryObj(pub(crate) SharedMemory);
+pub(crate) struct MemoryRef(pub(crate) GuardedNullableGcRef<SharedMemory>);
+
+impl MemoryRef {
+    /// Creates a new stack reference
+    pub(crate) fn new(mem: &SharedMemory) -> (Self, RefGuard<'_, SharedMemory>) {
+        let (inner, guard) = GuardedNullableGcRef::new(mem);
+        (MemoryRef(inner), guard)
+    }
+
+    fn len(&self) -> usize {
+        self.0.with_inner(|mem| mem.len()).unwrap_or_default()
+    }
 
-impl MemoryObj {
     pub(crate) fn into_js_object(self, context: &mut Context<'_>) -> JsResult<JsObject> {
         let obj = JsObject::default();
-        let len = self.0.len();
+        let len = self.len();
-        // TODO: add into data <https://github.com/bluealloy/revm/pull/516>
-        let value = to_buf(self.0.slice(0, len).to_vec(), context)?;
 
         let length = FunctionObjectBuilder::new(
             context,
@@ -150,13 +222,14 @@ impl MemoryObj {
         .length(0)
         .build();
 
+        // slice returns the requested range of memory as a byte slice.
         let slice = FunctionObjectBuilder::new(
             context,
             NativeFunction::from_copy_closure_with_captures(
-                |_this, args, memory, ctx| {
+                move |_this, args, memory, ctx| {
                     let start = args.get_or_undefined(0).to_number(ctx)?;
                     let end = args.get_or_undefined(1).to_number(ctx)?;
-                    if end < start || start < 0. {
+                    if end < start || start < 0. || (end as usize) < memory.len() {
                         return Err(JsError::from_native(JsNativeError::typ().with_message(
                             format!(
                                 "tracer accessed out of bound memory: offset {start}, end {end}"
@@ -165,12 +238,15 @@ impl MemoryObj {
                     }
                     let start = start as usize;
                     let end = end as usize;
+                    let size = end - start;
+                    let slice = memory
+                        .0
+                        .with_inner(|mem| mem.slice(start, size).to_vec())
+                        .unwrap_or_default();
 
-                    let mut mem = memory.take()?;
-                    let slice = mem.drain(start..end).collect::<Vec<u8>>();
                     to_buf_value(slice, ctx)
                 },
-                value.clone(),
+                self.clone(),
             ),
         )
         .length(2)
@@ -179,21 +255,19 @@ impl MemoryObj {
         let get_uint = FunctionObjectBuilder::new(
             context,
             NativeFunction::from_copy_closure_with_captures(
-                 |_this, args, memory, ctx|  {
+                move |_this, args, memory, ctx|  {
                     let offset_f64 = args.get_or_undefined(0).to_number(ctx)?;
-
+                     let len = memory.len();
-                     let mut mem = memory.take()?;
                      let offset = offset_f64 as usize;
-                     if mem.len() < offset+32 || offset_f64 < 0. {
+                     if len < offset+32 || offset_f64 < 0. {
                          return Err(JsError::from_native(
-                             JsNativeError::typ().with_message(format!("tracer accessed out of bound memory: available {}, offset {}, size 32", mem.len(), offset))
+                             JsNativeError::typ().with_message(format!("tracer accessed out of bound memory: available {len}, offset {offset}, size 32"))
                          ));
                      }
-
+                    let slice = memory.0.with_inner(|mem| mem.slice(offset, 32).to_vec()).unwrap_or_default();
-                    let slice = mem.drain(offset..offset+32).collect::<Vec<u8>>();
                      to_buf_value(slice, ctx)
                 },
-                value
+                 self
             ),
         )
             .length(1)
@@ -206,6 +280,40 @@ impl MemoryObj {
     }
 }
 
+impl Finalize for MemoryRef {}
+
+unsafe impl Trace for MemoryRef {
+    empty_trace!();
+}
+
+/// Represents the state object
+#[derive(Debug, Clone)]
+pub(crate) struct StateRef(pub(crate) GuardedNullableGcRef<State>);
+
+impl StateRef {
+    /// Creates a new stack reference
+    pub(crate) fn new(state: &State) -> (Self, RefGuard<'_, State>) {
+        let (inner, guard) = GuardedNullableGcRef::new(state);
+        (StateRef(inner), guard)
+    }
+
+    fn get_account(&self, address: &Address) -> Option<Account> {
+        self.0.with_inner(|state| {
+            state.get(address).map(|acc| Account {
+                nonce: acc.info.nonce,
+                balance: acc.info.balance,
+                bytecode_hash: Some(acc.info.code_hash),
+            })
+        })?
+    }
+}
+
+impl Finalize for StateRef {}
+
+unsafe impl Trace for StateRef {
+    empty_trace!();
+}
+
 /// Represents the opcode object
 #[derive(Debug)]
 pub(crate) struct OpObj(pub(crate) u8);
@@ -264,15 +372,39 @@ impl From<u8> for OpObj {
 }
 
 /// Represents the stack object
-#[derive(Debug, Clone)]
+#[derive(Debug)]
-pub(crate) struct StackObj(pub(crate) Stack);
+pub(crate) struct StackRef(pub(crate) GuardedNullableGcRef<Stack>);
+
+impl StackRef {
+    /// Creates a new stack reference
+    pub(crate) fn new(stack: &Stack) -> (Self, RefGuard<'_, Stack>) {
+        let (inner, guard) = GuardedNullableGcRef::new(stack);
+        (StackRef(inner), guard)
+    }
+
+    fn peek(&self, idx: usize, ctx: &mut Context<'_>) -> JsResult<JsValue> {
+        self.0
+            .with_inner(|stack| {
+                let value = stack.peek(idx).map_err(|_| {
+                    JsError::from_native(JsNativeError::typ().with_message(format!(
+                        "tracer accessed out of bound stack: size {}, index {}",
+                        stack.len(),
+                        idx
+                    )))
+                })?;
+                to_bigint(value, ctx)
+            })
+            .ok_or_else(|| {
+                JsError::from_native(JsNativeError::typ().with_message(format!(
+                    "tracer accessed out of bound stack: size 0, index {}",
+                    idx
+                )))
+            })?
+    }
 
-impl StackObj {
     pub(crate) fn into_js_object(self, context: &mut Context<'_>) -> JsResult<JsObject> {
         let obj = JsObject::default();
-        let stack = self.0;
+        let len = self.0.with_inner(|stack| stack.len()).unwrap_or_default();
-        let len = stack.len();
-        let stack_arr = to_bigint_array(stack.data(), context)?;
         let length = FunctionObjectBuilder::new(
             context,
             NativeFunction::from_copy_closure(move |_this, _args, _ctx| Ok(JsValue::from(len))),
@@ -284,7 +416,7 @@ impl StackObj {
         let peek = FunctionObjectBuilder::new(
             context,
             NativeFunction::from_copy_closure_with_captures(
-                move |_this, args, stack_arr, ctx| {
+                move |_this, args, stack, ctx| {
                     let idx_f64 = args.get_or_undefined(0).to_number(ctx)?;
                     let idx = idx_f64 as usize;
                     if len <= idx || idx_f64 < 0. {
@@ -294,12 +426,9 @@ impl StackObj {
                             ),
                         )))
                     }
-                    // idx is from the top of the stack, so we need to reverse it
+                    stack.peek(idx, ctx)
-                    // SAFETY: bounds checked above
-                    let idx = len - idx - 1;
-                    stack_arr.get(idx as u64, ctx)
                 },
-                stack_arr,
+                self,
             ),
         )
         .length(1)
@@ -311,6 +440,12 @@ impl StackObj {
     }
 }
 
+impl Finalize for StackRef {}
+
+unsafe impl Trace for StackRef {
+    empty_trace!();
+}
+
 /// Represents the contract object
 #[derive(Debug, Clone, Default)]
 pub(crate) struct Contract {
@@ -555,123 +690,28 @@ impl EvmContext {
 }
 
 /// DB is the object that allows the js inspector to interact with the database.
-pub(crate) struct EvmDb {
+#[derive(Debug, Clone)]
-    db: EvmDBInner,
+pub(crate) struct EvmDbRef {
-}
+    state: StateRef,
-
-impl EvmDb {
-    pub(crate) fn new(state: State, to_db: mpsc::Sender<JsDbRequest>) -> Self {
-        Self { db: EvmDBInner { state, to_db } }
-    }
-}
-
-impl EvmDb {
-    pub(crate) fn into_js_object(self, context: &mut Context<'_>) -> JsResult<JsObject> {
-        let obj = JsObject::default();
-
-        let db = Gc::new(self.db);
-        let exists = FunctionObjectBuilder::new(
-            context,
-            NativeFunction::from_copy_closure_with_captures(
-                move |_this, args, db, ctx| {
-                    let val = args.get_or_undefined(0).clone();
-                    let db: &EvmDBInner = db.borrow();
-                    let acc = db.read_basic(val, ctx)?;
-                    let exists = acc.is_some();
-                    Ok(JsValue::from(exists))
-                },
-                db.clone(),
-            ),
-        )
-        .length(1)
-        .build();
-
-        let get_balance = FunctionObjectBuilder::new(
-            context,
-            NativeFunction::from_copy_closure_with_captures(
-                move |_this, args, db, ctx| {
-                    let val = args.get_or_undefined(0).clone();
-                    let db: &EvmDBInner = db.borrow();
-                    let acc = db.read_basic(val, ctx)?;
-                    let balance = acc.map(|acc| acc.balance).unwrap_or_default();
-                    to_bigint(balance, ctx)
-                },
-                db.clone(),
-            ),
-        )
-        .length(1)
-        .build();
-
-        let get_nonce = FunctionObjectBuilder::new(
-            context,
-            NativeFunction::from_copy_closure_with_captures(
-                move |_this, args, db, ctx| {
-                    let val = args.get_or_undefined(0).clone();
-                    let db: &EvmDBInner = db.borrow();
-                    let acc = db.read_basic(val, ctx)?;
-                    let nonce = acc.map(|acc| acc.nonce).unwrap_or_default();
-                    Ok(JsValue::from(nonce))
-                },
-                db.clone(),
-            ),
-        )
-        .length(1)
-        .build();
-
-        let get_code = FunctionObjectBuilder::new(
-            context,
-            NativeFunction::from_copy_closure_with_captures(
-                move |_this, args, db, ctx| {
-                    let val = args.get_or_undefined(0).clone();
-                    let db: &EvmDBInner = db.borrow();
-                    Ok(db.read_code(val, ctx)?.into())
-                },
-                db.clone(),
-            ),
-        )
-        .length(1)
-        .build();
-
-        let get_state = FunctionObjectBuilder::new(
-            context,
-            NativeFunction::from_copy_closure_with_captures(
-                move |_this, args, db, ctx| {
-                    let addr = args.get_or_undefined(0).clone();
-                    let slot = args.get_or_undefined(1).clone();
-                    let db: &EvmDBInner = db.borrow();
-                    Ok(db.read_state(addr, slot, ctx)?.into())
-                },
-                db,
-            ),
-        )
-        .length(2)
-        .build();
-
-        obj.set("getBalance", get_balance, false, context)?;
-        obj.set("getNonce", get_nonce, false, context)?;
-        obj.set("getCode", get_code, false, context)?;
-        obj.set("getState", get_state, false, context)?;
-        obj.set("exists", exists, false, context)?;
-        Ok(obj)
-    }
-}
-
-#[derive(Clone)]
-struct EvmDBInner {
-    state: State,
     to_db: mpsc::Sender<JsDbRequest>,
 }
 
-impl EvmDBInner {
+impl EvmDbRef {
+    /// Creates a new DB reference
+    pub(crate) fn new(
+        state: &State,
+        to_db: mpsc::Sender<JsDbRequest>,
+    ) -> (Self, RefGuard<'_, State>) {
+        let (state, guard) = StateRef::new(state);
+        let this = Self { state, to_db };
+        (this, guard)
+    }
+
     fn read_basic(&self, address: JsValue, ctx: &mut Context<'_>) -> JsResult<Option<Account>> {
         let buf = from_buf(address, ctx)?;
         let address = bytes_to_address(buf);
-        if let Some(acc) = self.state.get(&address) {
+        if let acc @ Some(_) = self.state.get_account(&address) {
-            return Ok(Some(Account {
+            return Ok(acc)
-                nonce: acc.info.nonce,
-                balance: acc.info.balance,
-                bytecode_hash: Some(acc.info.code_hash),
-            }))
         }
         let (tx, rx) = channel();
         if self.to_db.try_send(JsDbRequest::Basic { address, resp: tx }).is_err() {
@@ -751,11 +791,93 @@ impl EvmDBInner {
         let value: B256 = value.into();
         to_buf(value.as_slice().to_vec(), ctx)
     }
+
+    pub(crate) fn into_js_object(self, context: &mut Context<'_>) -> JsResult<JsObject> {
+        let obj = JsObject::default();
+        let exists = FunctionObjectBuilder::new(
+            context,
+            NativeFunction::from_copy_closure_with_captures(
+                move |_this, args, db, ctx| {
+                    let val = args.get_or_undefined(0).clone();
+                    let acc = db.read_basic(val, ctx)?;
+                    let exists = acc.is_some();
+                    Ok(JsValue::from(exists))
+                },
+                self.clone(),
+            ),
+        )
+        .length(1)
+        .build();
+
+        let get_balance = FunctionObjectBuilder::new(
+            context,
+            NativeFunction::from_copy_closure_with_captures(
+                move |_this, args, db, ctx| {
+                    let val = args.get_or_undefined(0).clone();
+                    let acc = db.read_basic(val, ctx)?;
+                    let balance = acc.map(|acc| acc.balance).unwrap_or_default();
+                    to_bigint(balance, ctx)
+                },
+                self.clone(),
+            ),
+        )
+        .length(1)
+        .build();
+
+        let get_nonce = FunctionObjectBuilder::new(
+            context,
+            NativeFunction::from_copy_closure_with_captures(
+                move |_this, args, db, ctx| {
+                    let val = args.get_or_undefined(0).clone();
+                    let acc = db.read_basic(val, ctx)?;
+                    let nonce = acc.map(|acc| acc.nonce).unwrap_or_default();
+                    Ok(JsValue::from(nonce))
+                },
+                self.clone(),
+            ),
+        )
+        .length(1)
+        .build();
+
+        let get_code = FunctionObjectBuilder::new(
+            context,
+            NativeFunction::from_copy_closure_with_captures(
+                move |_this, args, db, ctx| {
+                    let val = args.get_or_undefined(0).clone();
+                    Ok(db.read_code(val, ctx)?.into())
+                },
+                self.clone(),
+            ),
+        )
+        .length(1)
+        .build();
+
+        let get_state = FunctionObjectBuilder::new(
+            context,
+            NativeFunction::from_copy_closure_with_captures(
+                move |_this, args, db, ctx| {
+                    let addr = args.get_or_undefined(0).clone();
+                    let slot = args.get_or_undefined(1).clone();
+                    Ok(db.read_state(addr, slot, ctx)?.into())
+                },
+                self,
+            ),
+        )
+        .length(2)
+        .build();
+
+        obj.set("getBalance", get_balance, false, context)?;
+        obj.set("getNonce", get_nonce, false, context)?;
+        obj.set("getCode", get_code, false, context)?;
+        obj.set("getState", get_state, false, context)?;
+        obj.set("exists", exists, false, context)?;
+        Ok(obj)
+    }
 }
 
-impl Finalize for EvmDBInner {}
+impl Finalize for EvmDbRef {}
 
-unsafe impl Trace for EvmDBInner {
+unsafe impl Trace for EvmDbRef {
     empty_trace!();
 }
 

crates/revm/revm-inspectors/src/tracing/js/builtins.rs

@@ -67,24 +67,6 @@ pub(crate) fn to_buf_value(bytes: Vec<u8>, context: &mut Context<'_>) -> JsResul
     Ok(to_buf(bytes, context)?.into())
 }
 
-/// Create a new array buffer object from byte block.
-pub(crate) fn to_bigint_array(items: &[U256], ctx: &mut Context<'_>) -> JsResult<JsArray> {
-    let arr = JsArray::new(ctx);
-    let bigint = ctx.global_object().get("bigint", ctx)?;
-    if !bigint.is_callable() {
-        return Err(JsError::from_native(
-            JsNativeError::typ().with_message("global object bigint is not callable"),
-        ))
-    }
-    let bigint = bigint.as_callable().unwrap();
-
-    for item in items {
-        let val = bigint.call(&JsValue::undefined(), &[JsValue::from(item.to_string())], ctx)?;
-        arr.push(val, ctx)?;
-    }
-    Ok(arr)
-}
-
 /// Converts a buffer type to an address.
 ///
 /// If the buffer is larger than the address size, it will be cropped from the left

crates/revm/revm-inspectors/src/tracing/js/mod.rs

@@ -3,7 +3,7 @@
 use crate::tracing::{
     js::{
         bindings::{
-            CallFrame, Contract, EvmContext, EvmDb, FrameResult, MemoryObj, StackObj, StepLog,
+            CallFrame, Contract, EvmContext, EvmDbRef, FrameResult, MemoryRef, StackRef, StepLog,
         },
         builtins::{register_builtins, PrecompileList},
     },
@@ -151,7 +151,7 @@ impl JsInspector {
     /// Calls the result function and returns the result.
     pub fn result(&mut self, res: ResultAndState, env: &Env) -> Result<JsValue, JsInspectorError> {
         let ResultAndState { result, state } = res;
-        let db = EvmDb::new(state, self.to_db_service.clone());
+        let (db, _db_guard) = EvmDbRef::new(&state, self.to_db_service.clone());
 
         let gas_used = result.gas_used();
         let mut to = None;
@@ -206,14 +206,14 @@ impl JsInspector {
         )?)
     }
 
-    fn try_fault(&mut self, step: StepLog, db: EvmDb) -> JsResult<()> {
+    fn try_fault(&mut self, step: StepLog, db: EvmDbRef) -> JsResult<()> {
         let step = step.into_js_object(&mut self.ctx)?;
         let db = db.into_js_object(&mut self.ctx)?;
         self.fault_fn.call(&(self.obj.clone().into()), &[step.into(), db.into()], &mut self.ctx)?;
         Ok(())
     }
 
-    fn try_step(&mut self, step: StepLog, db: EvmDb) -> JsResult<()> {
+    fn try_step(&mut self, step: StepLog, db: EvmDbRef) -> JsResult<()> {
         if let Some(step_fn) = &self.step_fn {
             let step = step.into_js_object(&mut self.ctx)?;
             let db = db.into_js_object(&mut self.ctx)?;
@@ -291,12 +291,15 @@ where
             return
         }
 
-        let db = EvmDb::new(data.journaled_state.state.clone(), self.to_db_service.clone());
+        let (db, _db_guard) =
+            EvmDbRef::new(&data.journaled_state.state, self.to_db_service.clone());
 
+        let (stack, _stack_guard) = StackRef::new(&interp.stack);
+        let (memory, _memory_guard) = MemoryRef::new(interp.shared_memory);
         let step = StepLog {
-            stack: StackObj(interp.stack.clone()),
+            stack,
             op: interp.current_opcode().into(),
-            memory: MemoryObj(interp.shared_memory.clone()),
+            memory,
             pc: interp.program_counter() as u64,
             gas_remaining: interp.gas.remaining(),
             cost: interp.gas.spend(),
@@ -326,12 +329,15 @@ where
         }
 
         if matches!(interp.instruction_result, return_revert!()) {
-            let db = EvmDb::new(data.journaled_state.state.clone(), self.to_db_service.clone());
+            let (db, _db_guard) =
+                EvmDbRef::new(&data.journaled_state.state, self.to_db_service.clone());
 
+            let (stack, _stack_guard) = StackRef::new(&interp.stack);
+            let (memory, _memory_guard) = MemoryRef::new(interp.shared_memory);
             let step = StepLog {
-                stack: StackObj(interp.stack.clone()),
+                stack,
                 op: interp.current_opcode().into(),
-                memory: MemoryObj(interp.shared_memory.clone()),
+                memory,
                 pc: interp.program_counter() as u64,
                 gas_remaining: interp.gas.remaining(),
                 cost: interp.gas.spend(),