From 0e2a5707ff463edfc001fc0ca16a3a290576c746 Mon Sep 17 00:00:00 2001 From: meship-starkware Date: Mon, 20 Jan 2025 17:33:33 +0200 Subject: [PATCH] fix(cairo_native): change stack size to max using threads --- Cargo.lock | 23 ----- Cargo.toml | 1 - crates/blockifier/Cargo.toml | 3 +- .../src/blockifier/transaction_executor.rs | 85 +++++++++++++---- .../execution/native/entry_point_execution.rs | 50 ++-------- .../syscalls/syscall_tests/out_of_gas.rs | 95 ++++++++++--------- 6 files changed, 124 insertions(+), 133 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6a9e220fcb..9291f88fdd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1664,7 +1664,6 @@ dependencies = [ "serde", "serde_json", "sha2", - "stacker", "starknet-types-core", "starknet_api", "starknet_infra_utils", @@ -8789,15 +8788,6 @@ version = "2.0.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "33cb294fe86a74cbcf50d4445b37da762029549ebeea341421c7c70370f86cac" -[[package]] -name = "psm" -version = "0.1.24" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "200b9ff220857e53e184257720a14553b2f4aa02577d2ed9842d45d4b9654810" -dependencies = [ - "cc", -] - [[package]] name = "publicsuffix" version = "2.3.0" @@ -10445,19 +10435,6 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3" -[[package]] -name = "stacker" -version = "0.1.17" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "799c883d55abdb5e98af1a7b3f23b9b6de8ecada0ecac058672d7635eb48ca7b" -dependencies = [ - "cc", - "cfg-if", - "libc", - "psm", - "windows-sys 0.52.0", -] - [[package]] name = "starknet-core" version = "0.6.1" diff --git a/Cargo.toml b/Cargo.toml index e66a0fb2a1..b410a4b9d7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -212,7 +212,6 @@ serde_yaml = "0.9.16" sha2 = "0.10.8" sha3 = "0.10.8" simple_logger = "4.0.0" -stacker = "0.1.17" starknet-core = "0.6.0" starknet-crypto = "0.7.1" starknet-types-core = "0.1.6" diff --git a/crates/blockifier/Cargo.toml b/crates/blockifier/Cargo.toml index 4f52c90ce0..458f07ef9b 100644 --- a/crates/blockifier/Cargo.toml +++ b/crates/blockifier/Cargo.toml @@ -10,7 +10,7 @@ description = "The transaction-executing component in the Starknet sequencer." workspace = true [features] -cairo_native = ["dep:cairo-native", "dep:stacker", "starknet_sierra_multicompile/cairo_native"] +cairo_native = ["dep:cairo-native", "starknet_sierra_multicompile/cairo_native"] jemalloc = ["dep:tikv-jemallocator"] native_blockifier = [] reexecution = ["transaction_serde"] @@ -50,7 +50,6 @@ semver.workspace = true serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true, features = ["arbitrary_precision"] } sha2.workspace = true -stacker = { workspace = true, optional = true } starknet-types-core.workspace = true starknet_api.workspace = true starknet_infra_utils.workspace = true diff --git a/crates/blockifier/src/blockifier/transaction_executor.rs b/crates/blockifier/src/blockifier/transaction_executor.rs index 01ad8c2ce8..5ee21ac558 100644 --- a/crates/blockifier/src/blockifier/transaction_executor.rs +++ b/crates/blockifier/src/blockifier/transaction_executor.rs @@ -132,7 +132,7 @@ impl TransactionExecutor { } } - pub fn execute_txs_sequentially( + pub fn execute_txs_sequentially_inner( &mut self, txs: &[Transaction], ) -> Vec> { @@ -228,6 +228,39 @@ impl TransactionExecutor { } } + pub fn execute_txs_sequentially( + &mut self, + txs: &[Transaction], + ) -> Vec> { + #[cfg(not(feature = "cairo_native"))] + return self.execute_txs_sequentially_inner(txs); + #[cfg(feature = "cairo_native")] + { + // TODO meshi: find a way to access the contract class manager config from transaction + // executor. + let txs = txs.to_vec(); + std::thread::scope(|s| { + std::thread::Builder::new() + // when running Cairo natively, the real stack is used and could get overflowed + // (unlike the VM where the stack is simulated in the heap as a memory segment). + // + // We pre-allocate the stack here, and not during Native execution (not trivial), so it + // needs to be big enough ahead. + // However, making it very big is wasteful (especially with multi-threading). + // So, the stack size should support calls with a reasonable gas limit, for extremely deep + // recursions to reach out-of-gas before hitting the bottom of the recursion. + // + // The gas upper bound is MAX_POSSIBLE_SIERRA_GAS, and sequencers must not raise it without + // adjusting the stack size. + .stack_size(self.config.stack_size) + .spawn_scoped(s, || self.execute_txs_sequentially_inner(&txs)) + .expect("Failed to spawn thread") + .join() + .expect("Failed to join thread.") + }) + } + } + pub fn execute_chunk( &mut self, chunk: &[Transaction], @@ -251,26 +284,40 @@ impl TransactionExecutor { std::thread::scope(|s| { for _ in 0..self.config.concurrency_config.n_workers { let worker_executor = Arc::clone(&worker_executor); - s.spawn(move || { - // Making sure that the program will abort if a panic accured while halting the - // scheduler. - let abort_guard = AbortIfPanic; - // If a panic is not handled or the handling logic itself panics, then we abort - // the program. - if let Err(err) = catch_unwind(AssertUnwindSafe(|| { - worker_executor.run(); - })) { - // If the program panics here, the abort guard will exit the program. - // In this case, no panic message will be logged. Add the cargo flag - // --nocapture to log the panic message. + let _handle = std::thread::Builder::new() + // when running Cairo natively, the real stack is used and could get overflowed + // (unlike the VM where the stack is simulated in the heap as a memory segment). + // + // We pre-allocate the stack here, and not during Native execution (not trivial), so it + // needs to be big enough ahead. + // However, making it very big is wasteful (especially with multi-threading). + // So, the stack size should support calls with a reasonable gas limit, for extremely deep + // recursions to reach out-of-gas before hitting the bottom of the recursion. + // + // The gas upper bound is MAX_POSSIBLE_SIERRA_GAS, and sequencers must not raise it without + // adjusting the stack size. + .stack_size(self.config.stack_size) + .spawn_scoped(s, move || { + // Making sure that the program will abort if a panic accured while halting + // the scheduler. + let abort_guard = AbortIfPanic; + // If a panic is not handled or the handling logic itself panics, then we + // abort the program. + if let Err(err) = catch_unwind(AssertUnwindSafe(|| { + worker_executor.run(); + })) { + // If the program panics here, the abort guard will exit the program. + // In this case, no panic message will be logged. Add the cargo flag + // --nocapture to log the panic message. - worker_executor.scheduler.halt(); - abort_guard.release(); - panic::resume_unwind(err); - } + worker_executor.scheduler.halt(); + abort_guard.release(); + panic::resume_unwind(err); + } - abort_guard.release(); - }); + abort_guard.release(); + }) + .expect("Failed to spawn thread."); } }); diff --git a/crates/blockifier/src/execution/native/entry_point_execution.rs b/crates/blockifier/src/execution/native/entry_point_execution.rs index d0ca87b173..564d188d67 100644 --- a/crates/blockifier/src/execution/native/entry_point_execution.rs +++ b/crates/blockifier/src/execution/native/entry_point_execution.rs @@ -1,7 +1,5 @@ use cairo_native::execution_result::ContractExecutionResult; use cairo_native::utils::BuiltinCosts; -use num_rational::Ratio; -use stacker; use crate::execution::call_info::{CallExecution, CallInfo, Retdata}; use crate::execution::contract_class::TrackedResource; @@ -14,7 +12,6 @@ use crate::execution::errors::{EntryPointExecutionError, PostExecutionError, Pre use crate::execution::native::contract_class::NativeCompiledClassV1; use crate::execution::native::syscall_handler::NativeSyscallHandler; use crate::state::state_api::State; -use crate::versioned_constants::CairoNativeStackConfig; // todo(rodrigo): add an `entry point not found` test for Native pub fn execute_entry_point_call( @@ -51,45 +48,14 @@ pub fn execute_entry_point_call( .checked_sub(initial_budget) .ok_or(PreExecutionError::InsufficientEntryPointGas)?; - // Grow the stack (if it's below the red zone) to handle deep Cairo recursions - - // when running Cairo natively, the real stack is used and could get overflowed - // (unlike the VM where the stack is simulated in the heap as a memory segment). - // - // We pre-allocate the stack here, and not during Native execution (not trivial), so it - // needs to be big enough ahead. - // However, making it very big is wasteful (especially with multi-threading). - // So, the stack size should support calls with a reasonable gas limit, for extremely deep - // recursions to reach out-of-gas before hitting the bottom of the recursion. - // - // The gas upper bound is MAX_POSSIBLE_SIERRA_GAS, and sequencers must not raise it without - // adjusting the stack size. - // This also limits multi-threading, since each thread has its own stack. - // If the the free stack size is in the red zone, We will grow the stack to the - // target size, relative to reaming gas. - let stack_config = CairoNativeStackConfig { - // TODO(Aviv): Take it from VC. - gas_to_stack_ratio: Ratio::new(1, 20), - max_stack_size: 200 * 1024 * 1024, - min_stack_red_zone: 2 * 1024 * 1024, - buffer_size: 5 * 1024 * 1024, - }; - let stack_size_red_zone = stack_config.get_stack_size_red_zone(call_initial_gas); - let target_stack_size = - usize::try_from(stack_config.get_target_stack_size(stack_size_red_zone)) - .unwrap_or_else(|e| panic!("Failed to convert target stack size to usize: {}", e)); - let stack_size_red_zone = usize::try_from(stack_size_red_zone) - .unwrap_or_else(|e| panic!("Failed to convert stack size red zone to usize: {}", e)); - // Use `maybe_grow` and not `grow` for performance, since in happy flows, only the main call - // should trigger the growth. - let execution_result = stacker::maybe_grow(stack_size_red_zone, target_stack_size, || { - compiled_class.executor.run( - entry_point.selector.0, - &syscall_handler.base.call.calldata.0.clone(), - call_initial_gas, - Some(builtin_costs), - &mut syscall_handler, - ) - }); + let execution_result = compiled_class.executor.run( + entry_point.selector.0, + &syscall_handler.base.call.calldata.0.clone(), + call_initial_gas, + Some(builtin_costs), + &mut syscall_handler, + ); + syscall_handler.finalize(); let call_result = execution_result.map_err(EntryPointExecutionError::NativeUnexpectedError)?; diff --git a/crates/blockifier/src/execution/syscalls/syscall_tests/out_of_gas.rs b/crates/blockifier/src/execution/syscalls/syscall_tests/out_of_gas.rs index c30bc19a62..1b822c0c9f 100644 --- a/crates/blockifier/src/execution/syscalls/syscall_tests/out_of_gas.rs +++ b/crates/blockifier/src/execution/syscalls/syscall_tests/out_of_gas.rs @@ -3,8 +3,6 @@ use starknet_api::{calldata, felt}; use test_case::test_case; use crate::abi::constants::MAX_POSSIBLE_SIERRA_GAS; -#[cfg(feature = "cairo_native")] -use crate::context::ChainInfo; use crate::execution::call_info::CallExecution; use crate::execution::entry_point::CallEntryPoint; use crate::execution::syscalls::syscall_tests::constants; @@ -12,10 +10,6 @@ use crate::execution::syscalls::syscall_tests::get_block_hash::initialize_state; use crate::execution::syscalls::SyscallSelector; use crate::retdata; use crate::test_utils::contracts::FeatureContract; -#[cfg(feature = "cairo_native")] -use crate::test_utils::initial_test_state::test_state; -#[cfg(feature = "cairo_native")] -use crate::test_utils::BALANCE; use crate::test_utils::{trivial_external_entry_point_new, CairoVersion, RunnableCairo1}; use crate::versioned_constants::VersionedConstants; @@ -64,44 +58,53 @@ fn test_total_tx_limits_less_than_max_sierra_gas() { ); } -#[cfg(feature = "cairo_native")] -#[rstest::rstest] -#[case(MAX_POSSIBLE_SIERRA_GAS, MAX_POSSIBLE_SIERRA_GAS - 2681170910)] -#[case(MAX_POSSIBLE_SIERRA_GAS / 10, 81886490)] -#[case(MAX_POSSIBLE_SIERRA_GAS / 100, 8190940)] -#[case(MAX_POSSIBLE_SIERRA_GAS / 1000, 822890)] -#[case(MAX_POSSIBLE_SIERRA_GAS / 10000, 85440)] -#[case(MAX_POSSIBLE_SIERRA_GAS / 100000, 12340)] -#[case(MAX_POSSIBLE_SIERRA_GAS / 1000000, 0)] -#[case(350, 0)] -#[case(35, 0)] -#[case(0, 0)] -/// Tests that Native can handle deep recursion calls without overflowing the stack. -/// Note that the recursive function must be complicated, since the compiler might transform -/// simple recursions into loops. The tested function was manually tested with higher gas and -/// reached stack overflow. -/// -/// Also, there is no need to test the VM here since it doesn't use the stack. -fn test_stack_overflow(#[case] initial_gas: u64, #[case] gas_consumed: u64) { - let test_contract = FeatureContract::TestContract(CairoVersion::Cairo1(RunnableCairo1::Native)); - let mut state = test_state(&ChainInfo::create_for_testing(), BALANCE, &[(test_contract, 1)]); +// TODO (AvivG) move this test to Transaction executor - let depth = felt!(1000000_u128); - let entry_point_call = CallEntryPoint { - calldata: calldata![depth], - entry_point_selector: selector_from_name("test_stack_overflow"), - initial_gas, - ..trivial_external_entry_point_new(test_contract) - }; - let call_info = entry_point_call.execute_directly(&mut state).unwrap(); - assert_eq!( - call_info.execution, - CallExecution { - // 'Out of gas' - retdata: retdata![felt!["0x4f7574206f6620676173"]], - gas_consumed, - failed: true, - ..Default::default() - } - ); -} +// #[cfg(feature = "cairo_native")] +// #[cfg(feature = "cairo_native")] +// use crate::context::ChainInfo; +// #[cfg(feature = "cairo_native")] +// use crate::test_utils::initial_test_state::test_state; +// #[cfg(feature = "cairo_native")] +// use crate::test_utils::BALANCE; +// #[rstest::rstest] +// #[case(MAX_POSSIBLE_SIERRA_GAS, MAX_POSSIBLE_SIERRA_GAS - 2681170910)] +// #[case(MAX_POSSIBLE_SIERRA_GAS / 10, 81886490)] +// #[case(MAX_POSSIBLE_SIERRA_GAS / 100, 8190940)] +// #[case(MAX_POSSIBLE_SIERRA_GAS / 1000, 822890)] +// #[case(MAX_POSSIBLE_SIERRA_GAS / 10000, 85440)] +// #[case(MAX_POSSIBLE_SIERRA_GAS / 100000, 12340)] +// #[case(MAX_POSSIBLE_SIERRA_GAS / 1000000, 0)] +// #[case(350, 0)] +// #[case(35, 0)] +// #[case(0, 0)] +// /// Tests that Native can handle deep recursion calls without overflowing the stack. +// /// Note that the recursive function must be complicated, since the compiler might transform +// /// simple recursions into loops. The tested function was manually tested with higher gas and +// /// reached stack overflow. +// /// +// /// Also, there is no need to test the VM here since it doesn't use the stack. +// fn test_stack_overflow(#[case] initial_gas: u64, #[case] gas_consumed: u64) { +// let test_contract = +// FeatureContract::TestContract(CairoVersion::Cairo1(RunnableCairo1::Native)); let mut state = +// test_state(&ChainInfo::create_for_testing(), BALANCE, &[(test_contract, 1)]); + +// let depth = felt!(1000000_u128); +// let entry_point_call = CallEntryPoint { +// calldata: calldata![depth], +// entry_point_selector: selector_from_name("test_stack_overflow"), +// initial_gas, +// ..trivial_external_entry_point_new(test_contract) +// }; +// let call_info = entry_point_call.execute_directly(&mut state).unwrap(); +// assert_eq!( +// call_info.execution, +// CallExecution { +// // 'Out of gas' +// retdata: retdata![felt!["0x4f7574206f6620676173"]], +// gas_consumed, +// failed: true, +// ..Default::default() +// } +// ); +// }