Skip to content

Commit

Permalink
perf(vm): Fix VM performance regression on CI loadtest (matter-labs#2782
Browse files Browse the repository at this point in the history
)

## What ❔

Fixes VM performance regression on CI loadtest introduced in
matter-labs#2760.

## Why ❔

Changes in the VM interface made the VM eagerly clone compressed
bytecodes if compression hasn't failed. Compressed bytecodes aren't used
during sandboxed VM execution in the API server (the sandbox only checks
that compression is successful).For new VMs, bytecodes can be borrowed
from the VM state, which is what this PR does using `Cow`.

## Checklist

- [x] PR title corresponds to the body of PR (we generate changelog
entries from PRs).
- [x] Tests for the changes have been added / updated.
- [x] Documentation comments have been added / updated.
- [x] Code has been formatted via `zk fmt` and `zk lint`.
  • Loading branch information
slowli authored Sep 4, 2024
1 parent 4a4d87e commit bc0d7d5
Show file tree
Hide file tree
Showing 30 changed files with 118 additions and 72 deletions.
9 changes: 6 additions & 3 deletions .github/workflows/ci-core-reusable.yml
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,10 @@ jobs:
- name: Loadtest configuration
run: |
echo EXPECTED_TX_COUNT=${{ matrix.vm_mode == 'new' && 22000 || 16000 }} >> .env
echo ACCOUNTS_AMOUNT="150" >> .env
echo EXPECTED_TX_COUNT=${{ matrix.vm_mode == 'new' && 21000 || 16000 }} >> .env
echo ACCOUNTS_AMOUNT="100" >> .env
echo MAX_INFLIGHT_TXS="10" >> .env
echo SYNC_API_REQUESTS_LIMIT="15" >> .env
echo FAIL_FAST=true >> .env
echo IN_DOCKER=1 >> .env
echo DATABASE_MERKLE_TREE_MODE=lightweight >> .env
Expand All @@ -112,7 +114,8 @@ jobs:
- name: Run server
run: |
EXPERIMENTAL_VM_STATE_KEEPER_FAST_VM_MODE=${{ matrix.vm_mode }} \
PASSED_ENV_VARS="EXPERIMENTAL_VM_STATE_KEEPER_FAST_VM_MODE" \
CHAIN_MEMPOOL_DELAY_INTERVAL=50 \
PASSED_ENV_VARS="EXPERIMENTAL_VM_STATE_KEEPER_FAST_VM_MODE,CHAIN_MEMPOOL_DELAY_INTERVAL" \
ci_run zk server --uring --components api,tree,eth,state_keeper,housekeeper,commitment_generator,vm_runner_protective_reads &>server.log &
ci_run sleep 60
Expand Down
2 changes: 1 addition & 1 deletion core/lib/multivm/src/versions/shadow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ where
tracer: Self::TracerDispatcher,
tx: Transaction,
with_compression: bool,
) -> (BytecodeCompressionResult, VmExecutionResultAndLogs) {
) -> (BytecodeCompressionResult<'_>, VmExecutionResultAndLogs) {
let tx_hash = tx.hash();
let main_result = self.main.inspect_transaction_with_bytecode_compression(
tracer,
Expand Down
4 changes: 2 additions & 2 deletions core/lib/multivm/src/versions/vm_1_3_2/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ impl<S: WriteStorage, H: HistoryMode> VmInterface for Vm<S, H> {
tracer: Self::TracerDispatcher,
tx: Transaction,
with_compression: bool,
) -> (BytecodeCompressionResult, VmExecutionResultAndLogs) {
) -> (BytecodeCompressionResult<'_>, VmExecutionResultAndLogs) {
if let Some(storage_invocations) = tracer.storage_invocations {
self.vm
.execution_mode
Expand Down Expand Up @@ -156,7 +156,7 @@ impl<S: WriteStorage, H: HistoryMode> VmInterface for Vm<S, H> {
result,
)
} else {
(Ok(compressed_bytecodes), result)
(Ok(compressed_bytecodes.into()), result)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,11 +191,11 @@ impl BootloaderState {
l2_block.first_tx_index + l2_block.txs.len()
}

pub(crate) fn get_last_tx_compressed_bytecodes(&self) -> Vec<CompressedBytecodeInfo> {
pub(crate) fn get_last_tx_compressed_bytecodes(&self) -> &[CompressedBytecodeInfo] {
if let Some(tx) = self.last_l2_block().txs.last() {
tx.compressed_bytecodes.clone()
&tx.compressed_bytecodes
} else {
vec![]
&[]
}
}

Expand Down
7 changes: 5 additions & 2 deletions core/lib/multivm/src/versions/vm_1_4_1/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ impl<S: WriteStorage, H: HistoryMode> VmInterface for Vm<S, H> {
tracer: Self::TracerDispatcher,
tx: Transaction,
with_compression: bool,
) -> (BytecodeCompressionResult, VmExecutionResultAndLogs) {
) -> (BytecodeCompressionResult<'_>, VmExecutionResultAndLogs) {
self.push_transaction_with_compression(tx, with_compression);
let result = self.inspect_inner(tracer, VmExecutionMode::OneTx, None);
if self.has_unpublished_bytecodes() {
Expand All @@ -115,7 +115,10 @@ impl<S: WriteStorage, H: HistoryMode> VmInterface for Vm<S, H> {
)
} else {
(
Ok(self.bootloader_state.get_last_tx_compressed_bytecodes()),
Ok(self
.bootloader_state
.get_last_tx_compressed_bytecodes()
.into()),
result,
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,11 +191,11 @@ impl BootloaderState {
l2_block.first_tx_index + l2_block.txs.len()
}

pub(crate) fn get_last_tx_compressed_bytecodes(&self) -> Vec<CompressedBytecodeInfo> {
pub(crate) fn get_last_tx_compressed_bytecodes(&self) -> &[CompressedBytecodeInfo] {
if let Some(tx) = self.last_l2_block().txs.last() {
tx.compressed_bytecodes.clone()
&tx.compressed_bytecodes
} else {
vec![]
&[]
}
}

Expand Down
7 changes: 5 additions & 2 deletions core/lib/multivm/src/versions/vm_1_4_2/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ impl<S: WriteStorage, H: HistoryMode> VmInterface for Vm<S, H> {
tracer: Self::TracerDispatcher,
tx: Transaction,
with_compression: bool,
) -> (BytecodeCompressionResult, VmExecutionResultAndLogs) {
) -> (BytecodeCompressionResult<'_>, VmExecutionResultAndLogs) {
self.push_transaction_with_compression(tx, with_compression);
let result = self.inspect_inner(tracer, VmExecutionMode::OneTx, None);
if self.has_unpublished_bytecodes() {
Expand All @@ -115,7 +115,10 @@ impl<S: WriteStorage, H: HistoryMode> VmInterface for Vm<S, H> {
)
} else {
(
Ok(self.bootloader_state.get_last_tx_compressed_bytecodes()),
Ok(self
.bootloader_state
.get_last_tx_compressed_bytecodes()
.into()),
result,
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,11 +191,11 @@ impl BootloaderState {
l2_block.first_tx_index + l2_block.txs.len()
}

pub(crate) fn get_last_tx_compressed_bytecodes(&self) -> Vec<CompressedBytecodeInfo> {
pub(crate) fn get_last_tx_compressed_bytecodes(&self) -> &[CompressedBytecodeInfo] {
if let Some(tx) = self.last_l2_block().txs.last() {
tx.compressed_bytecodes.clone()
&tx.compressed_bytecodes
} else {
vec![]
&[]
}
}

Expand Down
7 changes: 5 additions & 2 deletions core/lib/multivm/src/versions/vm_boojum_integration/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ impl<S: WriteStorage, H: HistoryMode> VmInterface for Vm<S, H> {
tracer: Self::TracerDispatcher,
tx: Transaction,
with_compression: bool,
) -> (BytecodeCompressionResult, VmExecutionResultAndLogs) {
) -> (BytecodeCompressionResult<'_>, VmExecutionResultAndLogs) {
self.push_transaction_with_compression(tx, with_compression);
let result = self.inspect_inner(tracer, VmExecutionMode::OneTx);
if self.has_unpublished_bytecodes() {
Expand All @@ -116,7 +116,10 @@ impl<S: WriteStorage, H: HistoryMode> VmInterface for Vm<S, H> {
)
} else {
(
Ok(self.bootloader_state.get_last_tx_compressed_bytecodes()),
Ok(self
.bootloader_state
.get_last_tx_compressed_bytecodes()
.into()),
result,
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,11 +189,11 @@ impl BootloaderState {
l2_block.first_tx_index + l2_block.txs.len()
}

pub(crate) fn get_last_tx_compressed_bytecodes(&self) -> Vec<CompressedBytecodeInfo> {
pub(crate) fn get_last_tx_compressed_bytecodes(&self) -> &[CompressedBytecodeInfo] {
if let Some(tx) = self.last_l2_block().txs.last() {
tx.compressed_bytecodes.clone()
&tx.compressed_bytecodes
} else {
vec![]
&[]
}
}

Expand Down
12 changes: 6 additions & 6 deletions core/lib/multivm/src/versions/vm_fast/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use super::{
use crate::{
glue::GlueInto,
interface::{
storage::ReadStorage, BytecodeCompressionError, CompressedBytecodeInfo,
storage::ReadStorage, BytecodeCompressionError, BytecodeCompressionResult,
CurrentExecutionState, ExecutionResult, FinishedL1Batch, Halt, L1BatchEnv, L2BlockEnv,
Refunds, SystemEnv, TxRevertReason, VmEvent, VmExecutionLogs, VmExecutionMode,
VmExecutionResultAndLogs, VmExecutionStatistics, VmInterface, VmInterfaceHistoryEnabled,
Expand Down Expand Up @@ -585,17 +585,17 @@ impl<S: ReadStorage> VmInterface for Vm<S> {
(): Self::TracerDispatcher,
tx: zksync_types::Transaction,
with_compression: bool,
) -> (
Result<Vec<CompressedBytecodeInfo>, BytecodeCompressionError>,
VmExecutionResultAndLogs,
) {
) -> (BytecodeCompressionResult<'_>, VmExecutionResultAndLogs) {
self.push_transaction_inner(tx, 0, with_compression);
let result = self.inspect((), VmExecutionMode::OneTx);

let compression_result = if self.has_unpublished_bytecodes() {
Err(BytecodeCompressionError::BytecodeCompressionFailed)
} else {
Ok(self.bootloader_state.get_last_tx_compressed_bytecodes())
Ok(self
.bootloader_state
.get_last_tx_compressed_bytecodes()
.into())
};
(compression_result, result)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,11 +191,11 @@ impl BootloaderState {
l2_block.first_tx_index + l2_block.txs.len()
}

pub(crate) fn get_last_tx_compressed_bytecodes(&self) -> Vec<CompressedBytecodeInfo> {
pub(crate) fn get_last_tx_compressed_bytecodes(&self) -> &[CompressedBytecodeInfo] {
if let Some(tx) = self.last_l2_block().txs.last() {
tx.compressed_bytecodes.clone()
&tx.compressed_bytecodes
} else {
vec![]
&[]
}
}

Expand Down
7 changes: 5 additions & 2 deletions core/lib/multivm/src/versions/vm_latest/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ impl<S: WriteStorage, H: HistoryMode> VmInterface for Vm<S, H> {
tracer: Self::TracerDispatcher,
tx: Transaction,
with_compression: bool,
) -> (BytecodeCompressionResult, VmExecutionResultAndLogs) {
) -> (BytecodeCompressionResult<'_>, VmExecutionResultAndLogs) {
self.push_transaction_with_compression(tx, with_compression);
let result = self.inspect_inner(tracer, VmExecutionMode::OneTx, None);
if self.has_unpublished_bytecodes() {
Expand All @@ -151,7 +151,10 @@ impl<S: WriteStorage, H: HistoryMode> VmInterface for Vm<S, H> {
)
} else {
(
Ok(self.bootloader_state.get_last_tx_compressed_bytecodes()),
Ok(self
.bootloader_state
.get_last_tx_compressed_bytecodes()
.into()),
result,
)
}
Expand Down
4 changes: 2 additions & 2 deletions core/lib/multivm/src/versions/vm_m5/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,14 @@ impl<S: Storage, H: HistoryMode> VmInterface for Vm<S, H> {
_tracer: Self::TracerDispatcher,
tx: Transaction,
_with_compression: bool,
) -> (BytecodeCompressionResult, VmExecutionResultAndLogs) {
) -> (BytecodeCompressionResult<'_>, VmExecutionResultAndLogs) {
crate::vm_m5::vm_with_bootloader::push_transaction_to_bootloader_memory(
&mut self.vm,
&tx,
self.system_env.execution_mode.glue_into(),
);
// Bytecode compression isn't supported
(Ok(vec![]), self.inspect((), VmExecutionMode::OneTx))
(Ok(vec![].into()), self.inspect((), VmExecutionMode::OneTx))
}

fn record_vm_memory_metrics(&self) -> VmMemoryMetrics {
Expand Down
4 changes: 2 additions & 2 deletions core/lib/multivm/src/versions/vm_m6/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ impl<S: Storage, H: HistoryMode> VmInterface for Vm<S, H> {
tracer: Self::TracerDispatcher,
tx: Transaction,
with_compression: bool,
) -> (BytecodeCompressionResult, VmExecutionResultAndLogs) {
) -> (BytecodeCompressionResult<'_>, VmExecutionResultAndLogs) {
if let Some(storage_invocations) = tracer.storage_invocations {
self.vm
.execution_mode
Expand Down Expand Up @@ -182,7 +182,7 @@ impl<S: Storage, H: HistoryMode> VmInterface for Vm<S, H> {
result,
)
} else {
(Ok(compressed_bytecodes), result)
(Ok(compressed_bytecodes.into()), result)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,11 +167,11 @@ impl BootloaderState {
l2_block.first_tx_index + l2_block.txs.len()
}

pub(crate) fn get_last_tx_compressed_bytecodes(&self) -> Vec<CompressedBytecodeInfo> {
pub(crate) fn get_last_tx_compressed_bytecodes(&self) -> &[CompressedBytecodeInfo] {
if let Some(tx) = self.last_l2_block().txs.last() {
tx.compressed_bytecodes.clone()
&tx.compressed_bytecodes
} else {
vec![]
&[]
}
}

Expand Down
7 changes: 5 additions & 2 deletions core/lib/multivm/src/versions/vm_refunds_enhancement/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ impl<S: WriteStorage, H: HistoryMode> VmInterface for Vm<S, H> {
dispatcher: Self::TracerDispatcher,
tx: Transaction,
with_compression: bool,
) -> (BytecodeCompressionResult, VmExecutionResultAndLogs) {
) -> (BytecodeCompressionResult<'_>, VmExecutionResultAndLogs) {
self.push_transaction_with_compression(tx, with_compression);
let result = self.inspect(dispatcher, VmExecutionMode::OneTx);
if self.has_unpublished_bytecodes() {
Expand All @@ -109,7 +109,10 @@ impl<S: WriteStorage, H: HistoryMode> VmInterface for Vm<S, H> {
)
} else {
(
Ok(self.bootloader_state.get_last_tx_compressed_bytecodes()),
Ok(self
.bootloader_state
.get_last_tx_compressed_bytecodes()
.into()),
result,
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,11 +167,11 @@ impl BootloaderState {
l2_block.first_tx_index + l2_block.txs.len()
}

pub(crate) fn get_last_tx_compressed_bytecodes(&self) -> Vec<CompressedBytecodeInfo> {
pub(crate) fn get_last_tx_compressed_bytecodes(&self) -> &[CompressedBytecodeInfo] {
if let Some(tx) = self.last_l2_block().txs.last() {
tx.compressed_bytecodes.clone()
&tx.compressed_bytecodes
} else {
vec![]
&[]
}
}

Expand Down
7 changes: 5 additions & 2 deletions core/lib/multivm/src/versions/vm_virtual_blocks/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ impl<S: WriteStorage, H: HistoryMode> VmInterface for Vm<S, H> {
tracer: TracerDispatcher<S, H::VmVirtualBlocksMode>,
tx: Transaction,
with_compression: bool,
) -> (BytecodeCompressionResult, VmExecutionResultAndLogs) {
) -> (BytecodeCompressionResult<'_>, VmExecutionResultAndLogs) {
self.push_transaction_with_compression(tx, with_compression);
let result = self.inspect_inner(tracer, VmExecutionMode::OneTx);
if self.has_unpublished_bytecodes() {
Expand All @@ -109,7 +109,10 @@ impl<S: WriteStorage, H: HistoryMode> VmInterface for Vm<S, H> {
)
} else {
(
Ok(self.bootloader_state.get_last_tx_compressed_bytecodes()),
Ok(self
.bootloader_state
.get_last_tx_compressed_bytecodes()
.into()),
result,
)
}
Expand Down
2 changes: 1 addition & 1 deletion core/lib/multivm/src/vm_instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ impl<S: ReadStorage, H: HistoryMode> VmInterface for VmInstance<S, H> {
dispatcher: Self::TracerDispatcher,
tx: zksync_types::Transaction,
with_compression: bool,
) -> (BytecodeCompressionResult, VmExecutionResultAndLogs) {
) -> (BytecodeCompressionResult<'_>, VmExecutionResultAndLogs) {
dispatch_vm!(self.inspect_transaction_with_bytecode_compression(
dispatcher.into(),
tx,
Expand Down
9 changes: 5 additions & 4 deletions core/lib/vm_executor/src/batch/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ impl<S: ReadStorage + 'static> CommandReceiver<S> {
.unwrap_or_default();
return Ok(BatchTransactionExecutionResult {
tx_result: Box::new(tx_result),
compressed_bytecodes,
compressed_bytecodes: compressed_bytecodes.into_owned(),
call_traces,
});
}
Expand All @@ -269,8 +269,9 @@ impl<S: ReadStorage + 'static> CommandReceiver<S> {

let (compression_result, tx_result) =
vm.inspect_transaction_with_bytecode_compression(tracer.into(), tx.clone(), false);
let compressed_bytecodes =
compression_result.context("compression failed when it wasn't applied")?;
let compressed_bytecodes = compression_result
.context("compression failed when it wasn't applied")?
.into_owned();

// TODO implement tracer manager which will be responsible
// for collecting result from all tracers and save it to the database
Expand Down Expand Up @@ -308,7 +309,7 @@ impl<S: ReadStorage + 'static> CommandReceiver<S> {
.unwrap_or_default();
Ok(BatchTransactionExecutionResult {
tx_result: Box::new(tx_result),
compressed_bytecodes,
compressed_bytecodes: compressed_bytecodes.into_owned(),
call_traces,
})
} else {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::borrow::Cow;

use crate::CompressedBytecodeInfo;

/// Errors related to bytecode compression.
Expand All @@ -9,4 +11,5 @@ pub enum BytecodeCompressionError {
}

/// Result of compressing bytecodes used by a transaction.
pub type BytecodeCompressionResult = Result<Vec<CompressedBytecodeInfo>, BytecodeCompressionError>;
pub type BytecodeCompressionResult<'a> =
Result<Cow<'a, [CompressedBytecodeInfo]>, BytecodeCompressionError>;
Loading

0 comments on commit bc0d7d5

Please sign in to comment.