Skip to content

Commit 575279d

Browse files
DaniPopesDaniPopes
authored
perf: improve InspectorStack (#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
1 parent ae262ec commit 575279d

File tree

7 files changed

+68
-46
lines changed

7 files changed

+68
-46
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,9 +129,9 @@ where
129129
});
130130
}
131131

132+
#[allow(clippy::redundant_clone)]
132133
fn log(&mut self, interp: &mut Interpreter, ecx: &mut CTX, log: Log) {
133134
call_inspectors!([&mut self.tracer, &mut self.log_collector], |inspector| {
134-
// TODO: rm the log.clone
135135
inspector.log(interp, ecx, log.clone());
136136
});
137137
}

‎crates/cheatcodes/src/inspector.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -664,7 +664,7 @@ impl Cheatcodes {
664664
&mut self,
665665
ecx: Ecx,
666666
call: &mut CallInputs,
667-
executor: &mut impl CheatcodesExecutor,
667+
executor: &mut dyn CheatcodesExecutor,
668668
) -> Option<CallOutcome> {
669669
let gas = Gas::new(call.gas_limit);
670670
let curr_depth = ecx.journaled_state.depth();
@@ -1061,7 +1061,6 @@ impl Inspector<EthEvmContext<&mut dyn DatabaseExt>> for Cheatcodes {
10611061
}
10621062
}
10631063

1064-
#[inline]
10651064
fn step(&mut self, interpreter: &mut Interpreter, ecx: Ecx) {
10661065
self.pc = interpreter.bytecode.pc();
10671066

@@ -1104,7 +1103,6 @@ impl Inspector<EthEvmContext<&mut dyn DatabaseExt>> for Cheatcodes {
11041103
}
11051104
}
11061105

1107-
#[inline]
11081106
fn step_end(&mut self, interpreter: &mut Interpreter, ecx: Ecx) {
11091107
if self.gas_metering.paused {
11101108
self.meter_gas_end(interpreter);

‎crates/evm/core/src/utils.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,13 @@ use revm::primitives::{
1414
};
1515
pub use revm::state::EvmState as StateChangeset;
1616

17+
/// Hints to the compiler that this is a cold path, i.e. unlikely to be taken.
18+
#[cold]
19+
#[inline(always)]
20+
pub fn cold_path() {
21+
// TODO: remove `#[cold]` and call `std::hint::cold_path` once stable.
22+
}
23+
1724
/// Depending on the configured chain id and block number this should apply any specific changes
1825
///
1926
/// - checks for prevrandao mixhash after merge

‎crates/evm/coverage/src/inspector.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ where
4343
self.insert_map(interpreter);
4444
}
4545

46-
#[inline]
4746
fn step(&mut self, interpreter: &mut Interpreter, _context: &mut CTX) {
4847
let map = self.get_or_insert_map(interpreter);
4948
map.hit(interpreter.bytecode.pc() as u32);

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ where
2828
CTX: ContextTr<Db = D>,
2929
CTX::Journal: JournalExt,
3030
{
31-
#[inline]
3231
fn step(&mut self, interpreter: &mut Interpreter, _ecx: &mut CTX) {
3332
// Check if both target and bytecode address are the same as script contract address
3433
// (allow calling external libraries when bytecode address is different).
@@ -42,8 +41,5 @@ where
4241
interpreter.gas,
4342
));
4443
}
45-
// Note: We don't return anything here as step returns void.
46-
// The original check returned InstructionResult::Continue, but that's the default
47-
// behavior.
4844
}
4945
}

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

Lines changed: 56 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -235,17 +235,19 @@ impl InspectorStackBuilder {
235235
/// dispatch.
236236
#[macro_export]
237237
macro_rules! call_inspectors {
238-
([$($inspector:expr),+ $(,)?], |$id:ident $(,)?| $call:expr $(,)?) => {
238+
([$($inspector:expr),+ $(,)?], |$id:ident $(,)?| $body:expr $(,)?) => {
239239
$(
240240
if let Some($id) = $inspector {
241-
({ #[inline(always)] #[cold] || $call })();
241+
$crate::utils::cold_path();
242+
$body;
242243
}
243244
)+
244245
};
245-
(#[ret] [$($inspector:expr),+ $(,)?], |$id:ident $(,)?| $call:expr $(,)?) => {{
246+
(#[ret] [$($inspector:expr),+ $(,)?], |$id:ident $(,)?| $body:expr $(,)?) => {{
246247
$(
247248
if let Some($id) = $inspector {
248-
if let Some(result) = ({ #[inline(always)] #[cold] || $call })() {
249+
$crate::utils::cold_path();
250+
if let Some(result) = $body {
249251
return result;
250252
}
251253
}
@@ -297,19 +299,21 @@ pub struct InspectorStack {
297299
/// See [`InspectorStack`].
298300
#[derive(Default, Clone, Debug)]
299301
pub struct InspectorStackInner {
302+
// Inspectors.
300303
pub chisel_state: Option<ChiselState>,
301-
pub line_coverage: Option<LineCoverageCollector>,
302304
pub edge_coverage: Option<EdgeCovInspector>,
303305
pub fuzzer: Option<Fuzzer>,
306+
pub line_coverage: Option<LineCoverageCollector>,
304307
pub log_collector: Option<LogCollector>,
305308
pub printer: Option<CustomPrintTracer>,
306-
pub tracer: Option<TracingInspector>,
309+
pub revert_diag: Option<RevertDiagnostic>,
307310
pub script_execution_inspector: Option<ScriptExecutionInspector>,
311+
pub tracer: Option<TracingInspector>,
312+
313+
// InspectorExt and other internal data.
308314
pub enable_isolation: bool,
309315
pub odyssey: bool,
310316
pub create2_deployer: Address,
311-
pub revert_diag: Option<RevertDiagnostic>,
312-
313317
/// Flag marking if we are in the inner EVM context.
314318
pub in_inner_context: bool,
315319
pub inner_context_data: Option<InnerContextData>,
@@ -759,7 +763,7 @@ impl InspectorStackRefMut<'_> {
759763
if self.enable_isolation {
760764
// If we're in isolation mode, we need to keep track of the state at the beginning of
761765
// the frame to be able to roll back on revert
762-
self.top_frame_journal = ecx.journaled_state.state.clone();
766+
self.top_frame_journal.clone_from(&ecx.journaled_state.state);
763767
}
764768
}
765769

@@ -786,63 +790,84 @@ impl InspectorStackRefMut<'_> {
786790
ecx.journaled_state.state = std::mem::take(&mut self.top_frame_journal);
787791
}
788792
}
789-
}
790793

791-
impl Inspector<EthEvmContext<&mut dyn DatabaseExt>> for InspectorStackRefMut<'_> {
792-
fn initialize_interp(
794+
#[inline(always)]
795+
fn step_inlined(
793796
&mut self,
794797
interpreter: &mut Interpreter,
795798
ecx: &mut EthEvmContext<&mut dyn DatabaseExt>,
796799
) {
797800
call_inspectors!(
798801
[
799-
&mut self.line_coverage,
802+
&mut self.fuzzer,
800803
&mut self.tracer,
801-
&mut self.cheatcodes,
804+
&mut self.line_coverage,
805+
&mut self.edge_coverage,
802806
&mut self.script_execution_inspector,
803-
&mut self.printer
807+
&mut self.printer,
808+
&mut self.revert_diag,
809+
// Keep `cheatcodes` last to make use of the tail call.
810+
&mut self.cheatcodes,
804811
],
805-
|inspector| inspector.initialize_interp(interpreter, ecx),
812+
|inspector| (*inspector).step(interpreter, ecx),
806813
);
807814
}
808815

809-
fn step(
816+
#[inline(always)]
817+
fn step_end_inlined(
810818
&mut self,
811819
interpreter: &mut Interpreter,
812820
ecx: &mut EthEvmContext<&mut dyn DatabaseExt>,
813821
) {
814822
call_inspectors!(
815823
[
816-
&mut self.fuzzer,
817824
&mut self.tracer,
818-
&mut self.line_coverage,
819-
&mut self.edge_coverage,
820-
&mut self.cheatcodes,
821-
&mut self.script_execution_inspector,
825+
&mut self.chisel_state,
822826
&mut self.printer,
823-
&mut self.revert_diag
827+
&mut self.revert_diag,
828+
// Keep `cheatcodes` last to make use of the tail call.
829+
&mut self.cheatcodes,
824830
],
825-
|inspector| inspector.step(interpreter, ecx),
831+
|inspector| (*inspector).step_end(interpreter, ecx),
826832
);
827833
}
834+
}
828835

829-
fn step_end(
836+
impl Inspector<EthEvmContext<&mut dyn DatabaseExt>> for InspectorStackRefMut<'_> {
837+
fn initialize_interp(
830838
&mut self,
831839
interpreter: &mut Interpreter,
832840
ecx: &mut EthEvmContext<&mut dyn DatabaseExt>,
833841
) {
834842
call_inspectors!(
835843
[
844+
&mut self.line_coverage,
836845
&mut self.tracer,
837846
&mut self.cheatcodes,
838-
&mut self.chisel_state,
839-
&mut self.printer,
840-
&mut self.revert_diag
847+
&mut self.script_execution_inspector,
848+
&mut self.printer
841849
],
842-
|inspector| inspector.step_end(interpreter, ecx),
850+
|inspector| inspector.initialize_interp(interpreter, ecx),
843851
);
844852
}
845853

854+
fn step(
855+
&mut self,
856+
interpreter: &mut Interpreter,
857+
ecx: &mut EthEvmContext<&mut dyn DatabaseExt>,
858+
) {
859+
self.step_inlined(interpreter, ecx);
860+
}
861+
862+
fn step_end(
863+
&mut self,
864+
interpreter: &mut Interpreter,
865+
ecx: &mut EthEvmContext<&mut dyn DatabaseExt>,
866+
) {
867+
self.step_end_inlined(interpreter, ecx);
868+
}
869+
870+
#[allow(clippy::redundant_clone)]
846871
fn log(
847872
&mut self,
848873
interpreter: &mut Interpreter,
@@ -1060,22 +1085,20 @@ impl InspectorExt for InspectorStackRefMut<'_> {
10601085
}
10611086

10621087
impl Inspector<EthEvmContext<&mut dyn DatabaseExt>> for InspectorStack {
1063-
#[inline]
10641088
fn step(
10651089
&mut self,
10661090
interpreter: &mut Interpreter,
10671091
ecx: &mut EthEvmContext<&mut dyn DatabaseExt>,
10681092
) {
1069-
self.as_mut().step(interpreter, ecx)
1093+
self.as_mut().step_inlined(interpreter, ecx)
10701094
}
10711095

1072-
#[inline]
10731096
fn step_end(
10741097
&mut self,
10751098
interpreter: &mut Interpreter,
10761099
ecx: &mut EthEvmContext<&mut dyn DatabaseExt>,
10771100
) {
1078-
self.as_mut().step_end(interpreter, ecx)
1101+
self.as_mut().step_end_inlined(interpreter, ecx)
10791102
}
10801103

10811104
fn call(

‎crates/evm/fuzz/src/inspector.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,13 @@ impl<CTX> Inspector<CTX> for Fuzzer
2121
where
2222
CTX: ContextTr<Journal: JournalExt>,
2323
{
24-
#[inline]
2524
fn step(&mut self, interp: &mut Interpreter, _context: &mut CTX) {
2625
// We only collect `stack` and `memory` data before and after calls.
2726
if self.collect {
2827
self.collect_data(interp);
29-
self.collect = false;
3028
}
3129
}
3230

33-
#[inline]
3431
fn call(&mut self, ecx: &mut CTX, inputs: &mut CallInputs) -> Option<CallOutcome> {
3532
// We don't want to override the very first call made to the test contract.
3633
if self.call_generator.is_some() && ecx.tx().caller() != inputs.caller {
@@ -44,7 +41,6 @@ where
4441
None
4542
}
4643

47-
#[inline]
4844
fn call_end(&mut self, _context: &mut CTX, _inputs: &CallInputs, _outcome: &mut CallOutcome) {
4945
if let Some(ref mut call_generator) = self.call_generator {
5046
call_generator.used = false;
@@ -58,6 +54,7 @@ where
5854

5955
impl Fuzzer {
6056
/// Collects `stack` and `memory` values into the fuzz dictionary.
57+
#[cold]
6158
fn collect_data(&mut self, interpreter: &Interpreter) {
6259
self.fuzz_state.collect_values(interpreter.stack.data().iter().copied().map(Into::into));
6360

@@ -68,6 +65,8 @@ impl Fuzzer {
6865

6966
// state.insert(slot);
7067
// }
68+
69+
self.collect = false;
7170
}
7271

7372
/// Overrides an external call and tries to call any method of msg.sender.

0 commit comments

Comments
 (0)