Skip to content
This repository has been archived by the owner on Aug 21, 2024. It is now read-only.

Commit

Permalink
fix(concurrency): assert no concurrency mode in sequential run (#1987)
Browse files Browse the repository at this point in the history
  • Loading branch information
noaov1 authored Jun 26, 2024
1 parent 48942ed commit 12466f1
Show file tree
Hide file tree
Showing 8 changed files with 55 additions and 23 deletions.
3 changes: 1 addition & 2 deletions crates/blockifier/bench/blockifier_bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ use blockifier::test_utils::transfers_generator::TransfersGenerator;
use criterion::{criterion_group, criterion_main, Criterion};

pub fn transfers_benchmark(c: &mut Criterion) {
let concurrency_mode = false;
let mut transfers_generator = TransfersGenerator::new(concurrency_mode);
let mut transfers_generator = TransfersGenerator::new();
// Create a benchmark group called "transfers", which iterates over the accounts round-robin
// and performs transfers.
c.bench_function("transfers", |benchmark| {
Expand Down
20 changes: 15 additions & 5 deletions crates/blockifier/src/blockifier/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,28 @@
pub struct TransactionExecutorConfig {
pub concurrency_config: ConcurrencyConfig,
}
impl TransactionExecutorConfig {
pub fn create_for_testing() -> Self {
Self { concurrency_config: ConcurrencyConfig::create_for_testing() }
}
}

#[derive(Debug, Clone)]
#[cfg_attr(not(feature = "concurrency"), derive(Default))]
#[derive(Debug, Default, Clone)]
pub struct ConcurrencyConfig {
pub enabled: bool,
pub n_workers: usize,
pub chunk_size: usize,
}
#[cfg(all(feature = "testing", not(feature = "concurrency")))]
impl ConcurrencyConfig {
pub fn create_for_testing() -> Self {
Self { enabled: false, n_workers: 0, chunk_size: 0 }
}
}

#[cfg(feature = "concurrency")]
impl Default for ConcurrencyConfig {
fn default() -> Self {
#[cfg(all(feature = "testing", feature = "concurrency"))]
impl ConcurrencyConfig {
pub fn create_for_testing() -> Self {
Self { enabled: true, n_workers: 4, chunk_size: 64 }
}
}
12 changes: 12 additions & 0 deletions crates/blockifier/src/blockifier/transaction_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ impl<S: StateReader> TransactionExecutor<S> {
config: TransactionExecutorConfig,
) -> Self {
log::debug!("Initializing Transaction Executor...");
assert_eq!(
block_context.concurrency_mode, config.concurrency_config.enabled,
"The concurrency mode must be identical in the block context and in the config."
);
let bouncer_config = block_context.bouncer_config.clone();
// Note: the state might not be empty even at this point; it is the creator's
// responsibility to tune the bouncer according to pre and post block process.
Expand All @@ -85,6 +89,10 @@ impl<S: StateReader> TransactionExecutor<S> {
&mut self,
tx: &Transaction,
) -> TransactionExecutorResult<TransactionExecutionInfo> {
assert!(
!self.block_context.concurrency_mode,
"Executing a single transaction cannot be done in a concurrent mode."
);
let mut transactional_state = TransactionalState::create_transactional(
self.block_state.as_mut().expect(BLOCK_STATE_ACCESS_ERR),
);
Expand Down Expand Up @@ -117,6 +125,10 @@ impl<S: StateReader> TransactionExecutor<S> {
&mut self,
txs: &[Transaction],
) -> Vec<TransactionExecutorResult<TransactionExecutionInfo>> {
assert!(
!self.block_context.concurrency_mode,
"Executing transactions sequentially cannot be done in a concurrent mode."
);
let mut results = Vec::new();
for tx in txs {
match self.execute(tx) {
Expand Down
11 changes: 7 additions & 4 deletions crates/blockifier/src/blockifier/transaction_executor_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ fn test_l1_handler(block_context: BlockContext) {

fn test_bouncing(#[case] initial_bouncer_weights: BouncerWeights, #[case] n_events: usize) {
let max_n_events_in_block = 10;
let block_context = BlockContext::create_for_bouncer_testing(max_n_events_in_block);
let block_context = BlockContext::create_for_bouncer_testing(max_n_events_in_block, false);

let TestInitData { state, account_address, contract_address, mut nonce_manager } =
create_test_init_data(&block_context.chain_info, CairoVersion::Cairo1);
Expand All @@ -265,14 +265,17 @@ fn test_bouncing(#[case] initial_bouncer_weights: BouncerWeights, #[case] n_even

#[rstest]
fn test_execute_txs_bouncing() {
let config = TransactionExecutorConfig::create_for_testing();
let max_n_events_in_block = 10;
let block_context = BlockContext::create_for_bouncer_testing(max_n_events_in_block);
let block_context = BlockContext::create_for_bouncer_testing(
max_n_events_in_block,
config.concurrency_config.enabled,
);

let TestInitData { state, account_address, contract_address, .. } =
create_test_init_data(&block_context.chain_info, CairoVersion::Cairo1);

let mut tx_executor =
TransactionExecutor::new(state, block_context, TransactionExecutorConfig::default());
let mut tx_executor = TransactionExecutor::new(state, block_context, config);

let txs: Vec<Transaction> = [
emit_n_events_tx(1, account_address, contract_address, nonce!(0_u32)),
Expand Down
3 changes: 1 addition & 2 deletions crates/blockifier/src/blockifier/transfers_flow_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use crate::test_utils::transfers_generator::TransfersGenerator;

#[test]
pub fn transfers_flow_test() {
let concurrency_mode = true;
let mut transfers_generator = TransfersGenerator::new(concurrency_mode);
let mut transfers_generator = TransfersGenerator::new();
transfers_generator.execute_chunk_of_transfers();
}
7 changes: 5 additions & 2 deletions crates/blockifier/src/test_utils/struct_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,10 @@ impl BlockContext {
}
}

pub fn create_for_bouncer_testing(max_n_events_in_block: usize) -> Self {
pub fn create_for_bouncer_testing(
max_n_events_in_block: usize,
concurrency_mode: bool,
) -> Self {
Self {
bouncer_config: BouncerConfig {
block_max_capacity: BouncerWeights {
Expand All @@ -184,7 +187,7 @@ impl BlockContext {
},
..BouncerConfig::empty()
},
..Self::create_for_testing()
..Self::create_for_account_testing_with_concurrency_mode(concurrency_mode)
}
}

Expand Down
16 changes: 11 additions & 5 deletions crates/blockifier/src/test_utils/transfers_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,14 @@ pub struct TransfersGenerator {
}

impl TransfersGenerator {
pub fn new(concurrency_mode: bool) -> Self {
pub fn new() -> Self {
let account_contract = FeatureContract::AccountWithoutValidations(CairoVersion::Cairo0);
let block_context =
BlockContext::create_for_account_testing_with_concurrency_mode(concurrency_mode);
let executor_config = TransactionExecutorConfig::create_for_testing();
let block_context = BlockContext::create_for_account_testing_with_concurrency_mode(
executor_config.concurrency_config.enabled,
);
let chain_info = block_context.chain_info().clone();
let state = test_state(&chain_info, BALANCE * 1000, &[(account_contract, N_ACCOUNTS)]);
// TODO(Avi, 20/05/2024): Enable concurrency.
let executor_config = TransactionExecutorConfig::default();
let executor = TransactionExecutor::new(state, block_context, executor_config);
let account_addresses = (0..N_ACCOUNTS)
.map(|instance_id| account_contract.get_instance_address(instance_id))
Expand Down Expand Up @@ -115,3 +115,9 @@ impl TransfersGenerator {
AccountTransaction::Invoke(tx)
}
}

impl Default for TransfersGenerator {
fn default() -> Self {
Self::new()
}
}
6 changes: 3 additions & 3 deletions crates/native_blockifier/src/py_block_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,13 +168,13 @@ impl PyBlockExecutor {
) -> NativeBlockifierResult<Py<PyBytes>> {
let tx: Transaction = py_tx(tx, optional_py_class_info).expect(PY_TX_PARSING_ERR);
let tx_execution_info = self.tx_executor().execute(&tx)?;
let typed_tx_execution_info = ThinTransactionExecutionInfo::from_tx_execution_info(
let thin_tx_execution_info = ThinTransactionExecutionInfo::from_tx_execution_info(
&self.tx_executor().block_context,
tx_execution_info,
);

// Serialize and convert to PyBytes.
let serialized_tx_execution_info = typed_tx_execution_info.serialize();
let serialized_tx_execution_info = thin_tx_execution_info.serialize();
Ok(Python::with_gil(|py| PyBytes::new(py, &serialized_tx_execution_info).into()))
}

Expand Down Expand Up @@ -390,7 +390,7 @@ impl PyBlockExecutor {
use blockifier::state::global_cache::GLOBAL_CONTRACT_CACHE_SIZE_FOR_TEST;
Self {
bouncer_config: BouncerConfig::max(),
tx_executor_config: TransactionExecutorConfig::default(),
tx_executor_config: TransactionExecutorConfig::create_for_testing(),
storage: Box::new(storage),
chain_info: ChainInfo::default(),
versioned_constants: VersionedConstants::latest_constants().clone(),
Expand Down

0 comments on commit 12466f1

Please sign in to comment.