perf: improve InspectorStack (foundry-rs#11077)

* perf: clean up some step/step_end annotations

* chore: clean up call_inspectors!

* perf: use clone_from

* perf: inline step and step_end dispatchers

Author: DaniPopes

crates/anvil/src/eth/backend/mem/inspector.rs

@@ -129,9 +129,9 @@ where
         });
     }
 
+    #[allow(clippy::redundant_clone)]
     fn log(&mut self, interp: &mut Interpreter, ecx: &mut CTX, log: Log) {
         call_inspectors!([&mut self.tracer, &mut self.log_collector], |inspector| {
-            // TODO: rm the log.clone
             inspector.log(interp, ecx, log.clone());
         });
     }

crates/cheatcodes/src/inspector.rs

@@ -664,7 +664,7 @@ impl Cheatcodes {
         &mut self,
         ecx: Ecx,
         call: &mut CallInputs,
-        executor: &mut impl CheatcodesExecutor,
+        executor: &mut dyn CheatcodesExecutor,
     ) -> Option<CallOutcome> {
         let gas = Gas::new(call.gas_limit);
         let curr_depth = ecx.journaled_state.depth();
@@ -1061,7 +1061,6 @@ impl Inspector<EthEvmContext<&mut dyn DatabaseExt>> for Cheatcodes {
         }
     }
 
-    #[inline]
     fn step(&mut self, interpreter: &mut Interpreter, ecx: Ecx) {
         self.pc = interpreter.bytecode.pc();
 
@@ -1104,7 +1103,6 @@ impl Inspector<EthEvmContext<&mut dyn DatabaseExt>> for Cheatcodes {
         }
     }
 
-    #[inline]
     fn step_end(&mut self, interpreter: &mut Interpreter, ecx: Ecx) {
         if self.gas_metering.paused {
             self.meter_gas_end(interpreter);

crates/evm/core/src/utils.rs

@@ -14,6 +14,13 @@ use revm::primitives::{
 };
 pub use revm::state::EvmState as StateChangeset;
 
+/// Hints to the compiler that this is a cold path, i.e. unlikely to be taken.
+#[cold]
+#[inline(always)]
+pub fn cold_path() {
+    // TODO: remove `#[cold]` and call `std::hint::cold_path` once stable.
+}
+
 /// Depending on the configured chain id and block number this should apply any specific changes
 ///
 /// - checks for prevrandao mixhash after merge

crates/evm/coverage/src/inspector.rs

@@ -43,7 +43,6 @@ where
         self.insert_map(interpreter);
     }
 
-    #[inline]
     fn step(&mut self, interpreter: &mut Interpreter, _context: &mut CTX) {
         let map = self.get_or_insert_map(interpreter);
         map.hit(interpreter.bytecode.pc() as u32);

crates/evm/evm/src/inspectors/script.rs

@@ -28,7 +28,6 @@ where
     CTX: ContextTr<Db = D>,
     CTX::Journal: JournalExt,
 {
-    #[inline]
     fn step(&mut self, interpreter: &mut Interpreter, _ecx: &mut CTX) {
         // Check if both target and bytecode address are the same as script contract address
         // (allow calling external libraries when bytecode address is different).
@@ -42,8 +41,5 @@ where
                 interpreter.gas,
             ));
         }
-        // Note: We don't return anything here as step returns void.
-        // The original check returned InstructionResult::Continue, but that's the default
-        // behavior.
     }
 }

crates/evm/evm/src/inspectors/stack.rs

@@ -235,17 +235,19 @@ impl InspectorStackBuilder {
 /// dispatch.
 #[macro_export]
 macro_rules! call_inspectors {
-    ([$($inspector:expr),+ $(,)?], |$id:ident $(,)?| $call:expr $(,)?) => {
+    ([$($inspector:expr),+ $(,)?], |$id:ident $(,)?| $body:expr $(,)?) => {
         $(
             if let Some($id) = $inspector {
-                ({ #[inline(always)] #[cold] || $call })();
+                $crate::utils::cold_path();
+                $body;
             }
         )+
     };
-    (#[ret] [$($inspector:expr),+ $(,)?], |$id:ident $(,)?| $call:expr $(,)?) => {{
+    (#[ret] [$($inspector:expr),+ $(,)?], |$id:ident $(,)?| $body:expr $(,)?) => {{
         $(
             if let Some($id) = $inspector {
-                if let Some(result) = ({ #[inline(always)] #[cold] || $call })() {
+                $crate::utils::cold_path();
+                if let Some(result) = $body {
                     return result;
                 }
             }
@@ -297,19 +299,21 @@ pub struct InspectorStack {
 /// See [`InspectorStack`].
 #[derive(Default, Clone, Debug)]
 pub struct InspectorStackInner {
+    // Inspectors.
     pub chisel_state: Option<ChiselState>,
-    pub line_coverage: Option<LineCoverageCollector>,
     pub edge_coverage: Option<EdgeCovInspector>,
     pub fuzzer: Option<Fuzzer>,
+    pub line_coverage: Option<LineCoverageCollector>,
     pub log_collector: Option<LogCollector>,
     pub printer: Option<CustomPrintTracer>,
-    pub tracer: Option<TracingInspector>,
+    pub revert_diag: Option<RevertDiagnostic>,
     pub script_execution_inspector: Option<ScriptExecutionInspector>,
+    pub tracer: Option<TracingInspector>,
+
+    // InspectorExt and other internal data.
     pub enable_isolation: bool,
     pub odyssey: bool,
     pub create2_deployer: Address,
-    pub revert_diag: Option<RevertDiagnostic>,
-
     /// Flag marking if we are in the inner EVM context.
     pub in_inner_context: bool,
     pub inner_context_data: Option<InnerContextData>,
@@ -759,7 +763,7 @@ impl InspectorStackRefMut<'_> {
         if self.enable_isolation {
             // If we're in isolation mode, we need to keep track of the state at the beginning of
             // the frame to be able to roll back on revert
-            self.top_frame_journal = ecx.journaled_state.state.clone();
+            self.top_frame_journal.clone_from(&ecx.journaled_state.state);
         }
     }
 
@@ -786,63 +790,84 @@ impl InspectorStackRefMut<'_> {
             ecx.journaled_state.state = std::mem::take(&mut self.top_frame_journal);
         }
     }
-}
 
-impl Inspector<EthEvmContext<&mut dyn DatabaseExt>> for InspectorStackRefMut<'_> {
-    fn initialize_interp(
+    #[inline(always)]
+    fn step_inlined(
         &mut self,
         interpreter: &mut Interpreter,
         ecx: &mut EthEvmContext<&mut dyn DatabaseExt>,
     ) {
         call_inspectors!(
             [
-                &mut self.line_coverage,
+                &mut self.fuzzer,
                 &mut self.tracer,
-                &mut self.cheatcodes,
+                &mut self.line_coverage,
+                &mut self.edge_coverage,
                 &mut self.script_execution_inspector,
-                &mut self.printer
+                &mut self.printer,
+                &mut self.revert_diag,
+                // Keep `cheatcodes` last to make use of the tail call.
+                &mut self.cheatcodes,
             ],
-            |inspector| inspector.initialize_interp(interpreter, ecx),
+            |inspector| (*inspector).step(interpreter, ecx),
         );
     }
 
-    fn step(
+    #[inline(always)]
+    fn step_end_inlined(
         &mut self,
         interpreter: &mut Interpreter,
         ecx: &mut EthEvmContext<&mut dyn DatabaseExt>,
     ) {
         call_inspectors!(
             [
-                &mut self.fuzzer,
                 &mut self.tracer,
-                &mut self.line_coverage,
-                &mut self.edge_coverage,
-                &mut self.cheatcodes,
-                &mut self.script_execution_inspector,
+                &mut self.chisel_state,
                 &mut self.printer,
-                &mut self.revert_diag
+                &mut self.revert_diag,
+                // Keep `cheatcodes` last to make use of the tail call.
+                &mut self.cheatcodes,
             ],
-            |inspector| inspector.step(interpreter, ecx),
+            |inspector| (*inspector).step_end(interpreter, ecx),
         );
     }
+}
 
-    fn step_end(
+impl Inspector<EthEvmContext<&mut dyn DatabaseExt>> for InspectorStackRefMut<'_> {
+    fn initialize_interp(
         &mut self,
         interpreter: &mut Interpreter,
         ecx: &mut EthEvmContext<&mut dyn DatabaseExt>,
     ) {
         call_inspectors!(
             [
+                &mut self.line_coverage,
                 &mut self.tracer,
                 &mut self.cheatcodes,
-                &mut self.chisel_state,
-                &mut self.printer,
-                &mut self.revert_diag
+                &mut self.script_execution_inspector,
+                &mut self.printer
             ],
-            |inspector| inspector.step_end(interpreter, ecx),
+            |inspector| inspector.initialize_interp(interpreter, ecx),
         );
     }
 
+    fn step(
+        &mut self,
+        interpreter: &mut Interpreter,
+        ecx: &mut EthEvmContext<&mut dyn DatabaseExt>,
+    ) {
+        self.step_inlined(interpreter, ecx);
+    }
+
+    fn step_end(
+        &mut self,
+        interpreter: &mut Interpreter,
+        ecx: &mut EthEvmContext<&mut dyn DatabaseExt>,
+    ) {
+        self.step_end_inlined(interpreter, ecx);
+    }
+
+    #[allow(clippy::redundant_clone)]
     fn log(
         &mut self,
         interpreter: &mut Interpreter,
@@ -1060,22 +1085,20 @@ impl InspectorExt for InspectorStackRefMut<'_> {
 }
 
 impl Inspector<EthEvmContext<&mut dyn DatabaseExt>> for InspectorStack {
-    #[inline]
     fn step(
         &mut self,
         interpreter: &mut Interpreter,
         ecx: &mut EthEvmContext<&mut dyn DatabaseExt>,
     ) {
-        self.as_mut().step(interpreter, ecx)
+        self.as_mut().step_inlined(interpreter, ecx)
     }
 
-    #[inline]
     fn step_end(
         &mut self,
         interpreter: &mut Interpreter,
         ecx: &mut EthEvmContext<&mut dyn DatabaseExt>,
     ) {
-        self.as_mut().step_end(interpreter, ecx)
+        self.as_mut().step_end_inlined(interpreter, ecx)
     }
 
     fn call(

crates/evm/fuzz/src/inspector.rs

@@ -21,16 +21,13 @@ impl<CTX> Inspector<CTX> for Fuzzer
 where
     CTX: ContextTr<Journal: JournalExt>,
 {
-    #[inline]
     fn step(&mut self, interp: &mut Interpreter, _context: &mut CTX) {
         // We only collect `stack` and `memory` data before and after calls.
         if self.collect {
             self.collect_data(interp);
-            self.collect = false;
         }
     }
 
-    #[inline]
     fn call(&mut self, ecx: &mut CTX, inputs: &mut CallInputs) -> Option<CallOutcome> {
         // We don't want to override the very first call made to the test contract.
         if self.call_generator.is_some() && ecx.tx().caller() != inputs.caller {
@@ -44,7 +41,6 @@ where
         None
     }
 
-    #[inline]
     fn call_end(&mut self, _context: &mut CTX, _inputs: &CallInputs, _outcome: &mut CallOutcome) {
         if let Some(ref mut call_generator) = self.call_generator {
             call_generator.used = false;
@@ -58,6 +54,7 @@ where
 
 impl Fuzzer {
     /// Collects `stack` and `memory` values into the fuzz dictionary.
+    #[cold]
     fn collect_data(&mut self, interpreter: &Interpreter) {
         self.fuzz_state.collect_values(interpreter.stack.data().iter().copied().map(Into::into));
 
@@ -68,6 +65,8 @@ impl Fuzzer {
 
         //     state.insert(slot);
         // }
+
+        self.collect = false;
     }
 
     /// Overrides an external call and tries to call any method of msg.sender.