From 57259be1c91923aba8fede91a71bbc16959d65ba Mon Sep 17 00:00:00 2001 From: Danil Date: Fri, 29 Sep 2023 16:54:55 +0200 Subject: [PATCH] Mirrors branch deniallugo-vm-perf-optimization from private at 008df6deb6bd627ec9dc901a78aca374f16fd14e --- CODEOWNERS | 1 - core/lib/vm/src/implementation/execution.rs | 31 +++++++++--- core/lib/vm/src/old_vm/memory.rs | 14 +----- core/lib/vm/src/tracers/default_tracers.rs | 54 +++++++++++++++------ core/lib/vm/src/tracers/refunds.rs | 17 ++++--- 5 files changed, 73 insertions(+), 44 deletions(-) diff --git a/CODEOWNERS b/CODEOWNERS index 981a2db39116..12cd26187090 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -2,4 +2,3 @@ .github/release-please/** @RomanBrodetski @perekopskiy @Deniallugo @popzxc **/CHANGELOG.md @RomanBrodetski @perekopskiy @Deniallugo @popzxc CODEOWNERS @RomanBrodetski @perekopskiy @Deniallugo @popzxc -.github/workflows/** @matter-labs/devops diff --git a/core/lib/vm/src/implementation/execution.rs b/core/lib/vm/src/implementation/execution.rs index 9944a37f7e83..333693b4833f 100644 --- a/core/lib/vm/src/implementation/execution.rs +++ b/core/lib/vm/src/implementation/execution.rs @@ -19,13 +19,12 @@ impl Vm { mut tracers: Vec>>, execution_mode: VmExecutionMode, ) -> VmExecutionResultAndLogs { + let mut enable_refund_tracer = false; if let VmExecutionMode::OneTx = execution_mode { - // For correct results we have to include refunds tracer to the desired tracers - tracers.push(RefundsTracer::new(self.batch_env.clone()).into_boxed()); - // Move the pointer to the next transaction self.bootloader_state.move_tx_to_execute_pointer(); + enable_refund_tracer = true; } - let (_, result) = self.inspect_and_collect_results(tracers, execution_mode); + let (_, result) = self.inspect_and_collect_results(tracers, execution_mode, enable_refund_tracer); result } @@ -35,12 +34,19 @@ impl Vm { &mut self, tracers: Vec>>, execution_mode: VmExecutionMode, + with_refund_tracer: bool, ) -> (VmExecutionStopReason, VmExecutionResultAndLogs) { + let refund_tracers = if with_refund_tracer { + Some(RefundsTracer::new(self.batch_env.clone())) + } else { + None + }; let mut tx_tracer: DefaultExecutionTracer = DefaultExecutionTracer::new( self.system_env.default_validation_computational_gas_limit, execution_mode, tracers, self.storage.clone(), + refund_tracers, ); let timestamp_initial = Timestamp(self.state.local_state.timestamp); @@ -66,11 +72,17 @@ impl Vm { let result = tx_tracer.result_tracer.into_result(); + let refunds = if let Some(refund_tracer) = tx_tracer.refund_tracer { + refund_tracer.get_refunds() + } else { + Default::default() + }; + let mut result = VmExecutionResultAndLogs { result, logs, statistics, - refunds: Default::default(), + refunds, }; for tracer in tx_tracer.custom_tracers.iter_mut() { @@ -83,7 +95,14 @@ impl Vm { fn execute_with_default_tracer( &mut self, tracer: &mut DefaultExecutionTracer, - ) -> VmExecutionStopReason { + ) -> VmExecutionResultAndLogs { + let (_stop_reason, result) = + self.inspect_and_collect_results(tracers, ExecutionMode::Block, false); + result + } + + /// Execute vm with given tracers until the stop reason is reached. + fn execute(&mut self, tracer: &mut DefaultExecutionTracer) -> VmExecutionStopReason { tracer.initialize_tracer(&mut self.state); let result = loop { // Sanity check: we should never reach the maximum value, because then we won't be able to process the next cycle. diff --git a/core/lib/vm/src/old_vm/memory.rs b/core/lib/vm/src/old_vm/memory.rs index 8569c135d1e2..76d6ca42d5e1 100644 --- a/core/lib/vm/src/old_vm/memory.rs +++ b/core/lib/vm/src/old_vm/memory.rs @@ -269,24 +269,12 @@ impl Memory for SimpleMemory { fn finish_global_frame( &mut self, - base_page: MemoryPage, + _base_page: MemoryPage, returndata_fat_pointer: FatPointer, timestamp: Timestamp, ) { - // Safe to unwrap here, since `finish_global_frame` is never called with empty stack - let current_observable_pages = self.observable_pages.inner().current_frame(); let returndata_page = returndata_fat_pointer.memory_page; - for &page in current_observable_pages { - // If the page's number is greater than or equal to the base_page, - // it means that it was created by the internal calls of this contract. - // We need to add this check as the calldata pointer is also part of the - // observable pages. - if page >= base_page.0 && page != returndata_page { - self.memory.clear_page(page as usize, timestamp); - } - } - self.observable_pages.clear_frame(timestamp); self.observable_pages.merge_frame(timestamp); diff --git a/core/lib/vm/src/tracers/default_tracers.rs b/core/lib/vm/src/tracers/default_tracers.rs index 7cc1e19869cf..d6a0323043f5 100644 --- a/core/lib/vm/src/tracers/default_tracers.rs +++ b/core/lib/vm/src/tracers/default_tracers.rs @@ -1,12 +1,12 @@ use std::fmt::{Debug, Formatter}; -use zk_evm::witness_trace::DummyTracer; -use zk_evm::zkevm_opcode_defs::{Opcode, RetOpcode}; use zk_evm::{ tracing::{ AfterDecodingData, AfterExecutionData, BeforeExecutionData, Tracer, VmLocalStateData, }, vm_state::VmLocalState, + witness_trace::DummyTracer, + zkevm_opcode_defs::{decoding::EncodingModeProduction, Opcode, RetOpcode}, }; use zksync_state::{StoragePtr, WriteStorage}; use zksync_types::Timestamp; @@ -21,7 +21,7 @@ use crate::tracers::utils::{ computational_gas_price, gas_spent_on_bytecodes_and_long_messages_this_opcode, print_debug_if_needed, VmHook, }; -use crate::tracers::ResultTracer; +use crate::tracers::{RefundsTracer, ResultTracer}; use crate::types::internals::ZkSyncVmState; use crate::{VmExecutionMode, VmExecutionStopReason}; @@ -38,6 +38,7 @@ pub(crate) struct DefaultExecutionTracer { in_account_validation: bool, final_batch_info_requested: bool, pub(crate) result_tracer: ResultTracer, + pub(crate) refund_tracer: Option, pub(crate) custom_tracers: Vec>>, ret_from_the_bootloader: Option, storage: StoragePtr, @@ -50,17 +51,17 @@ impl Debug for DefaultExecutionTracer { } impl Tracer for DefaultExecutionTracer { - const CALL_BEFORE_DECODING: bool = true; + const CALL_BEFORE_DECODING: bool = false; const CALL_AFTER_DECODING: bool = true; const CALL_BEFORE_EXECUTION: bool = true; const CALL_AFTER_EXECUTION: bool = true; type SupportedMemory = SimpleMemory; - fn before_decoding(&mut self, state: VmLocalStateData<'_>, memory: &Self::SupportedMemory) { - >::before_decoding(&mut self.result_tracer, state, memory); - for tracer in self.custom_tracers.iter_mut() { - tracer.before_decoding(state, memory) - } + fn before_decoding( + &mut self, + _state: VmLocalStateData<'_, 8, EncodingModeProduction>, + _memory: &Self::SupportedMemory, + ) { } fn after_decoding( @@ -75,6 +76,11 @@ impl Tracer for DefaultExecutionTracer { data, memory, ); + + if let Some(refund_tracer) = &mut self.refund_tracer { + >::after_decoding(refund_tracer, state, data, memory); + } + for tracer in self.custom_tracers.iter_mut() { tracer.after_decoding(state, data, memory) } @@ -107,6 +113,10 @@ impl Tracer for DefaultExecutionTracer { gas_spent_on_bytecodes_and_long_messages_this_opcode(&state, &data); self.result_tracer .before_execution(state, data, memory, self.storage.clone()); + + if let Some(refund_tracer) = &mut self.refund_tracer { + refund_tracer.before_execution(state, data, memory, self.storage.clone()); + } for tracer in self.custom_tracers.iter_mut() { tracer.before_execution(state, data, memory, self.storage.clone()); } @@ -134,6 +144,9 @@ impl Tracer for DefaultExecutionTracer { self.result_tracer .after_execution(state, data, memory, self.storage.clone()); + if let Some(refund_tracer) = &mut self.refund_tracer { + refund_tracer.after_execution(state, data, memory, self.storage.clone()) + } for tracer in self.custom_tracers.iter_mut() { tracer.after_execution(state, data, memory, self.storage.clone()); } @@ -148,6 +161,10 @@ impl ExecutionEndTracer for DefaultExecution VmExecutionMode::Bootloader => self.ret_from_the_bootloader == Some(RetOpcode::Ok), }; should_stop = should_stop || self.validation_run_out_of_gas(); + if let Some(refund_tracer) = &self.refund_tracer { + should_stop = should_stop + || >::should_stop_execution(refund_tracer) + } for tracer in self.custom_tracers.iter() { should_stop = should_stop || tracer.should_stop_execution(); } @@ -161,6 +178,7 @@ impl DefaultExecutionTracer { execution_mode: VmExecutionMode, custom_tracers: Vec>>, storage: StoragePtr, + refund_tracer: Option, ) -> Self { Self { tx_has_been_processed: false, @@ -171,6 +189,7 @@ impl DefaultExecutionTracer { in_account_validation: false, final_batch_info_requested: false, result_tracer: ResultTracer::new(execution_mode), + refund_tracer, custom_tracers, ret_from_the_bootloader: None, storage, @@ -211,15 +230,11 @@ impl DynTracer for DefaultExecutionTracer {} impl ExecutionProcessing for DefaultExecutionTracer { fn initialize_tracer(&mut self, state: &mut ZkSyncVmState) { self.result_tracer.initialize_tracer(state); - for processor in self.custom_tracers.iter_mut() { - processor.initialize_tracer(state); + if let Some(refund_tracer) = &mut self.refund_tracer { + refund_tracer.initialize_tracer(state); } - } - - fn before_cycle(&mut self, state: &mut ZkSyncVmState) { - self.result_tracer.before_cycle(state); for processor in self.custom_tracers.iter_mut() { - processor.before_cycle(state); + processor.initialize_tracer(state); } } @@ -232,6 +247,9 @@ impl ExecutionProcessing for DefaultExecu for processor in self.custom_tracers.iter_mut() { processor.after_cycle(state, bootloader_state); } + if let Some(refund_tracer) = &mut self.refund_tracer { + refund_tracer.after_cycle(state, bootloader_state); + } if self.final_batch_info_requested { self.set_fictive_l2_block(state, bootloader_state) } @@ -245,6 +263,10 @@ impl ExecutionProcessing for DefaultExecu ) { self.result_tracer .after_vm_execution(state, bootloader_state, stop_reason); + + if let Some(refund_tracer) = &mut self.refund_tracer { + refund_tracer.after_vm_execution(state, bootloader_state, stop_reason); + } for processor in self.custom_tracers.iter_mut() { processor.after_vm_execution(state, bootloader_state, stop_reason); } diff --git a/core/lib/vm/src/tracers/refunds.rs b/core/lib/vm/src/tracers/refunds.rs index 47e3f83e20f8..4e463f4dff4e 100644 --- a/core/lib/vm/src/tracers/refunds.rs +++ b/core/lib/vm/src/tracers/refunds.rs @@ -81,6 +81,13 @@ impl RefundsTracer { 0 } + pub(crate) fn get_refunds(&self) -> Refunds { + Refunds { + gas_refunded: self.refund_gas, + operator_suggested_refund: self.operator_refund.unwrap_or_default(), + } + } + pub(crate) fn tx_body_refund( &self, bootloader_refund: u32, @@ -147,6 +154,7 @@ impl DynTracer for RefundsTracer { memory: &SimpleMemory, _storage: StoragePtr, ) { + self.timestamp_before_cycle = Timestamp(state.vm_local_state.timestamp); let hook = VmHook::from_opcode_memory(&state, &data); match hook { VmHook::NotifyAboutRefund => self.refund_gas = get_vm_hook_params(memory)[0].as_u32(), @@ -170,10 +178,6 @@ impl ExecutionProcessing for RefundsTrace self.spent_pubdata_counter_before = state.local_state.spent_pubdata_counter; } - fn before_cycle(&mut self, state: &mut ZkSyncVmState) { - self.timestamp_before_cycle = Timestamp(state.local_state.timestamp); - } - fn after_cycle( &mut self, state: &mut ZkSyncVmState, @@ -386,9 +390,6 @@ fn pubdata_published_for_writes( impl VmTracer for RefundsTracer { fn save_results(&mut self, result: &mut VmExecutionResultAndLogs) { - result.refunds = Refunds { - gas_refunded: self.refund_gas, - operator_suggested_refund: self.operator_refund.unwrap_or_default(), - } + result.refunds = self.get_refunds(); } }