From 0ef019794534d07f8dd31b409f2713cce33e3589 Mon Sep 17 00:00:00 2001 From: rakita Date: Mon, 9 Jan 2023 13:31:17 +0100 Subject: [PATCH] Cleanup, move hot fields toggether in Interpreter (#321) --- bins/revm-test/src/bin/snailtracer.rs | 24 +++----- crates/interpreter/src/gas.rs | 14 +++-- crates/interpreter/src/interpreter.rs | 56 +++++++++++-------- .../interpreter/src/interpreter/bytecode.rs | 32 +++++++---- .../interpreter/src/interpreter/contract.rs | 2 +- crates/revm/src/evm_impl.rs | 11 ++-- 6 files changed, 76 insertions(+), 63 deletions(-) diff --git a/bins/revm-test/src/bin/snailtracer.rs b/bins/revm-test/src/bin/snailtracer.rs index 89564584af..96e841044e 100644 --- a/bins/revm-test/src/bin/snailtracer.rs +++ b/bins/revm-test/src/bin/snailtracer.rs @@ -1,7 +1,7 @@ use std::time::Duration; use bytes::Bytes; -use revm::{db::BenchmarkDB, Bytecode, CallContext, TransactTo, B160, U256}; +use revm::{db::BenchmarkDB, Bytecode, TransactTo}; use revm_interpreter::{specification::BerlinSpec, DummyHost}; extern crate alloc; @@ -11,7 +11,7 @@ pub fn simple_example() { // BenchmarkDB is dummy state that implements Database trait. let mut evm = revm::new(); - let bytecode = Bytecode::new_raw(contract_data); + let bytecode = Bytecode::new_raw(contract_data).to_analysed::(); evm.database(BenchmarkDB::new_bytecode(bytecode.clone())); // execution globals block hash/gas_limit/coinbase/timestamp.. @@ -38,23 +38,15 @@ pub fn simple_example() { ); // revm interpreter - - let contract = revm_interpreter::Contract::new_with_context::( - evm.env.tx.data, - bytecode, - &CallContext { - address: B160::zero(), - caller: evm.env.tx.caller, - code_address: B160::zero(), - apparent_value: U256::from(0), - scheme: revm::CallScheme::Call, - }, - ); + let contract = revm_interpreter::Contract { + input: evm.env.tx.data, + bytecode: bytecode.lock_analysed().unwrap(), + ..Default::default() + }; let mut host = DummyHost::new(env); microbench::bench(&bench_options, "Snailtracer Interpreter benchmark", || { - let mut interpreter = - revm_interpreter::Interpreter::new::(contract.clone(), u64::MAX, false); + let mut interpreter = revm_interpreter::Interpreter::new(contract.clone(), u64::MAX, false); interpreter.run::<_, BerlinSpec>(&mut host); host.clear() }); diff --git a/crates/interpreter/src/gas.rs b/crates/interpreter/src/gas.rs index 7bc0d716f2..453dabcac3 100644 --- a/crates/interpreter/src/gas.rs +++ b/crates/interpreter/src/gas.rs @@ -5,11 +5,16 @@ pub use calc::*; pub use constants::*; #[derive(Clone, Copy, Debug)] pub struct Gas { + /// Gas Limit limit: u64, + /// used+memory gas. + all_used_gas: u64, + /// Used gas without memory used: u64, + /// Used gas for memory expansion memory: u64, + /// Refunded gas. This gas is used only at the end of execution. refunded: i64, - all_used_gas: u64, } impl Gas { pub fn new(limit: u64) -> Self { @@ -51,7 +56,7 @@ impl Gas { self.refunded += refund; } - /// Record an explict cost. + /// Record an explicit cost. #[inline(always)] pub fn record_cost(&mut self, cost: u64) -> bool { let (all_used_gas, overflow) = self.all_used_gas.overflowing_add(cost); @@ -64,7 +69,7 @@ impl Gas { true } - /// used in memory_resize! macro + /// used in memory_resize! macro to record gas used for memory expansion. pub fn record_memory(&mut self, gas_memory: u64) -> bool { if gas_memory > self.memory { let (all_used_gas, overflow) = self.used.overflowing_add(gas_memory); @@ -77,7 +82,8 @@ impl Gas { true } - /// used in gas_refund! macro + /// used in gas_refund! macro to record refund value. + /// Refund can be negative but self.refunded is always positive. pub fn gas_refund(&mut self, refund: i64) { self.refunded += refund; } diff --git a/crates/interpreter/src/interpreter.rs b/crates/interpreter/src/interpreter.rs index 3711f22621..ce597b4781 100644 --- a/crates/interpreter/src/interpreter.rs +++ b/crates/interpreter/src/interpreter.rs @@ -19,35 +19,37 @@ pub const STACK_LIMIT: u64 = 1024; pub const CALL_STACK_LIMIT: u64 = 1024; pub struct Interpreter { - /// Contract information and invoking data - pub contract: Contract, /// Instruction pointer. pub instruction_pointer: *const u8, + /// Return is main control flag, it tell us if we should continue interpreter or break from it + pub instruction_result: Return, + /// left gas. Memory gas can be found in Memory field. + pub gas: Gas, /// Memory. pub memory: Memory, /// Stack. pub stack: Stack, - /// left gas. Memory gas can be found in Memory field. - pub gas: Gas, /// After call returns, its return data is saved here. pub return_data_buffer: Bytes, /// Return value. pub return_range: Range, - /// Return is main control flag, it tell us if we should continue interpreter or break from it - pub instruction_result: Return, /// Is interpreter call static. pub is_static: bool, + /// Contract information and invoking data + pub contract: Contract, /// Memory limit. See [`crate::CfgEnv`]. #[cfg(feature = "memory_limit")] pub memory_limit: u64, } impl Interpreter { + /// Current opcode pub fn current_opcode(&self) -> u8 { unsafe { *self.instruction_pointer } } - pub fn new(contract: Contract, gas_limit: u64, is_static: bool) -> Self { + /// Create new interpreter + pub fn new(contract: Contract, gas_limit: u64, is_static: bool) -> Self { #[cfg(not(feature = "memory_limit"))] { Self { @@ -65,12 +67,12 @@ impl Interpreter { #[cfg(feature = "memory_limit")] { - Self::new_with_memory_limit::(contract, gas_limit, is_static, u64::MAX) + Self::new_with_memory_limit(contract, gas_limit, is_static, u64::MAX) } } #[cfg(feature = "memory_limit")] - pub fn new_with_memory_limit( + pub fn new_with_memory_limit( contract: Contract, gas_limit: u64, is_static: bool, @@ -98,7 +100,12 @@ impl Interpreter { &self.gas } - /// Reference of interp stack. + /// Reference of interpreter memory. + pub fn memory(&self) -> &Memory { + &self.memory + } + + /// Reference of interpreter stack. pub fn stack(&self) -> &Stack { &self.stack } @@ -123,6 +130,18 @@ impl Interpreter { } } + /// Execute next instruction + #[inline(always)] + pub fn step(&mut self, host: &mut H) { + // step. + let opcode = unsafe { *self.instruction_pointer }; + // Safety: In analysis we are doing padding of bytecode so that we are sure that last + // byte instruction is STOP so we are safe to just increment program_counter bcs on last instruction + // it will do noop and just stop execution of this contract + self.instruction_pointer = unsafe { self.instruction_pointer.offset(1) }; + eval::(opcode, self, host); + } + /// loop steps until we are finished with execution pub fn run(&mut self, host: &mut H) -> Return { // add first gas_block @@ -130,13 +149,7 @@ impl Interpreter { return Return::OutOfGas; } while self.instruction_result == Return::Continue { - // step. - let opcode = unsafe { *self.instruction_pointer }; - // Safety: In analysis we are doing padding of bytecode so that we are sure that last - // byte instruction is STOP so we are safe to just increment program_counter bcs on last instruction - // it will do noop and just stop execution of this contract - self.instruction_pointer = unsafe { self.instruction_pointer.offset(1) }; - eval::(opcode, self, host); + self.step::(host) } self.instruction_result } @@ -153,12 +166,7 @@ impl Interpreter { if ret != Return::Continue { return ret; } - let opcode = unsafe { *self.instruction_pointer }; - // Safety: In analysis we are doing padding of bytecode so that we are sure that last. - // byte instruction is STOP so we are safe to just increment program_counter bcs on last instruction - // it will do noop and just stop execution of this contract - self.instruction_pointer = unsafe { self.instruction_pointer.offset(1) }; - eval::(opcode, self, host); + self.step::(host); // step ends let ret = host.step_end(self, self.is_static, self.instruction_result); @@ -169,7 +177,7 @@ impl Interpreter { self.instruction_result } - /// Copy and get the return value of the interp, if any. + /// Copy and get the return value of the interpreter, if any. pub fn return_value(&self) -> Bytes { // if start is usize max it means that our return len is zero and we need to return empty if self.return_range.start == usize::MAX { diff --git a/crates/interpreter/src/interpreter/bytecode.rs b/crates/interpreter/src/interpreter/bytecode.rs index b1fef52f8a..7245933001 100644 --- a/crates/interpreter/src/interpreter/bytecode.rs +++ b/crates/interpreter/src/interpreter/bytecode.rs @@ -175,24 +175,24 @@ impl Bytecode { } } - pub fn lock(self) -> BytecodeLocked { - let Bytecode { - bytecode, - hash, - state, - } = self.to_analysed::(); - if let BytecodeState::Analysed { len, jumptable } = state { - BytecodeLocked { - bytecode, + pub fn lock_analysed(self) -> Option { + if let BytecodeState::Analysed { len, jumptable } = self.state { + Some(BytecodeLocked { + bytecode: self.bytecode, len, - hash, + hash: self.hash, jumptable, - } + }) } else { - unreachable!("to_analysed transforms state to analysed"); + None } } + pub fn lock(self) -> BytecodeLocked { + let bytecode = self.to_analysed::(); + bytecode.lock_analysed().expect("We have analysed bytecode") + } + /// Analyze bytecode to get jumptable and gas blocks. fn analyze(code: &[u8]) -> ValidJumpAddress { let opcode_gas = spec_opcode_gas(SPEC::SPEC_ID); @@ -270,6 +270,14 @@ pub struct BytecodeLocked { jumptable: ValidJumpAddress, } +impl Default for BytecodeLocked { + fn default() -> Self { + Bytecode::default() + .lock_analysed() + .expect("Bytecode default is analysed code") + } +} + impl BytecodeLocked { pub fn as_ptr(&self) -> *const u8 { self.bytecode.as_ptr() diff --git a/crates/interpreter/src/interpreter/contract.rs b/crates/interpreter/src/interpreter/contract.rs index b2aad58faa..4397d9ca47 100644 --- a/crates/interpreter/src/interpreter/contract.rs +++ b/crates/interpreter/src/interpreter/contract.rs @@ -3,7 +3,7 @@ use crate::{alloc::vec::Vec, CallContext, Spec, B160, U256}; use bytes::Bytes; use std::sync::Arc; -#[derive(Clone)] +#[derive(Clone, Default)] pub struct Contract { /// Contracts data pub input: Bytes, diff --git a/crates/revm/src/evm_impl.rs b/crates/revm/src/evm_impl.rs index f017b421a5..8ca4ce1b65 100644 --- a/crates/revm/src/evm_impl.rs +++ b/crates/revm/src/evm_impl.rs @@ -490,7 +490,7 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, ); #[cfg(feature = "memory_limit")] - let mut interpreter = Interpreter::new_with_memory_limit::( + let mut interpreter = Interpreter::new_with_memory_limit( contract, gas.limit(), false, @@ -498,7 +498,7 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, ); #[cfg(not(feature = "memory_limit"))] - let mut interpreter = Interpreter::new::(contract, gas.limit(), false); + let mut interpreter = Interpreter::new(contract, gas.limit(), false); if INSPECT { self.inspector @@ -546,7 +546,7 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, } // if we have enought gas self.data.journaled_state.checkpoint_commit(); - // Do analasis of bytecode streight away. + // Do analysis of bytecode streight away. let bytecode = match self.data.env.cfg.perf_analyse_created_bytecodes { AnalysisKind::Raw => Bytecode::new_raw(bytes.clone()), AnalysisKind::Check => Bytecode::new_raw(bytes.clone()).to_checked(), @@ -692,7 +692,7 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, ); #[cfg(feature = "memory_limit")] - let mut interpreter = Interpreter::new_with_memory_limit::( + let mut interpreter = Interpreter::new_with_memory_limit( contract, gas.limit(), inputs.is_static, @@ -700,8 +700,7 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, ); #[cfg(not(feature = "memory_limit"))] - let mut interpreter = - Interpreter::new::(contract, gas.limit(), inputs.is_static); + let mut interpreter = Interpreter::new(contract, gas.limit(), inputs.is_static); if INSPECT { // create is always no static call.