-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Additional peformance improvements and metrics for parallel execution #3197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: create_exec_sequencer_skeleton
Are you sure you want to change the base?
Changes from 13 commits
f8fa7a5
7da091f
74bfc1b
7f67288
6c0faa0
aca9dea
11572c4
2543a82
f0d594b
193c19e
e76af37
7f76c9f
cde001e
5590612
b6d8dbf
394822e
3a035f3
3ee3d1c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,15 @@ | ||
| // Define arguments | ||
|
|
||
| use fuel_core::service::config::Trigger; | ||
| use fuel_core::service::config::{ | ||
| ExecutorMode, | ||
| Trigger, | ||
| }; | ||
| use fuel_core_chain_config::{ | ||
| ChainConfig, | ||
| CoinConfig, | ||
| SnapshotMetadata, | ||
| }; | ||
| use fuel_core_metrics::encode_metrics; | ||
| use fuel_core_storage::transactional::AtomicView; | ||
| use fuel_core_types::{ | ||
| blockchain::transaction::TransactionExt, | ||
|
|
@@ -54,7 +58,7 @@ use test_helpers::builder::{ | |
| TestSetupBuilder, | ||
| }; | ||
|
|
||
| const PATH_SNAPSHOT: &str = "/Users/green/fuel/fuel-core-2/benches/local-testnet"; | ||
| const PATH_SNAPSHOT: &str = "./local-testnet"; | ||
|
|
||
| fn checked_parameters() -> CheckPredicateParams { | ||
| let metadata = SnapshotMetadata::read(PATH_SNAPSHOT).unwrap(); | ||
|
|
@@ -70,6 +74,16 @@ struct Args { | |
| pub number_of_cores: usize, | ||
| #[clap(short = 't', long, default_value = "150000")] | ||
| pub number_of_transactions: u64, | ||
| #[clap(long, default_value = "0")] | ||
| pub txpool_verification_threads: usize, | ||
| #[clap(long, default_value = "0")] | ||
| pub txpool_verification_queue_size: usize, | ||
| #[clap(long, default_value = "0")] | ||
| pub txpool_p2p_sync_threads: usize, | ||
| #[clap(long, default_value = "0")] | ||
| pub txpool_p2p_sync_queue_size: usize, | ||
| #[clap(long, default_value = "0")] | ||
| pub executor_parallel_worker_count: usize, | ||
| } | ||
|
|
||
| fn generate_transactions(nb_txs: u64, rng: &mut StdRng) -> Vec<Transaction> { | ||
|
|
@@ -135,6 +149,9 @@ fn generate_transactions(nb_txs: u64, rng: &mut StdRng) -> Vec<Transaction> { | |
| } | ||
|
|
||
| fn main() { | ||
| let _ = tracing_subscriber::fmt() | ||
| .with_max_level(tracing::Level::WARN) | ||
| .try_init(); | ||
| let args = Args::parse(); | ||
| let mut rng = StdRng::seed_from_u64(2322u64); | ||
|
|
||
|
|
@@ -200,9 +217,29 @@ fn main() { | |
| Some(tx.max_gas(&chain_conf.consensus_parameters).unwrap()) | ||
| }) | ||
| .sum(); | ||
| test_builder.gas_limit = Some(gas_limit * 4); | ||
| test_builder.gas_limit = Some(gas_limit); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Benchmark gas limit reduced by 75 percentMedium Severity The block gas limit calculation was changed from |
||
| test_builder.block_size_limit = Some(u64::MAX); | ||
| test_builder.number_threads_pool_verif = args.number_of_cores; | ||
| if args.txpool_verification_threads != 0 { | ||
| test_builder.txpool_verification_threads = args.txpool_verification_threads; | ||
| } | ||
| if args.txpool_verification_queue_size != 0 { | ||
| test_builder.txpool_verification_queue_size = args.txpool_verification_queue_size; | ||
| } | ||
| if args.txpool_p2p_sync_threads != 0 { | ||
| test_builder.txpool_p2p_sync_threads = args.txpool_p2p_sync_threads; | ||
| } | ||
| if args.txpool_p2p_sync_queue_size != 0 { | ||
| test_builder.txpool_p2p_sync_queue_size = args.txpool_p2p_sync_queue_size; | ||
| } | ||
| if args.executor_parallel_worker_count != 0 { | ||
| test_builder.executor_parallel_worker_count = args.executor_parallel_worker_count; | ||
| } | ||
| #[cfg(feature = "parallel-executor")] | ||
| { | ||
| test_builder.executor_mode = ExecutorMode::Parallel; | ||
| test_builder.executor_metrics = true; | ||
| } | ||
| test_builder.max_txs = transactions.len(); | ||
| // spin up node | ||
| let rt = tokio::runtime::Builder::new_multi_thread() | ||
|
|
@@ -241,10 +278,19 @@ fn main() { | |
| .unwrap() | ||
| .unwrap(); | ||
| assert_eq!(block.entity.transactions().len(), transactions.len() + 1); | ||
| println!("transaction count: {}", block.entity.transactions().len()); | ||
| block | ||
| } | ||
| }); | ||
|
|
||
| if let Ok(metrics) = encode_metrics() { | ||
| for line in metrics.lines().filter(|line| { | ||
| line.starts_with("parallel_executor_") && !line.starts_with('#') | ||
| }) { | ||
| println!("metrics: {line}"); | ||
| } | ||
| } | ||
|
|
||
| // rt.block_on(async move { | ||
| // test_builder.set_chain_config(chain_conf.clone()); | ||
| // let TestContext { srv, .. } = test_builder.finalize().await; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| use clap::Args; | ||
| use fuel_core::service::config::ExecutorMode; | ||
| #[cfg(feature = "parallel-executor")] | ||
| use fuel_core::service::config::ParallelExecutorWorkerCountPolicy; | ||
| use fuel_core_types::blockchain::header::StateTransitionBytecodeVersion; | ||
|
|
||
| #[cfg(feature = "parallel-executor")] | ||
| use std::num::NonZeroUsize; | ||
|
|
||
| #[derive(Debug, Clone, Args)] | ||
| pub struct ExecutorArgs { | ||
| /// Overrides the version of the native executor. | ||
| #[arg(long = "native-executor-version", env)] | ||
| pub native_executor_version: Option<StateTransitionBytecodeVersion>, | ||
|
|
||
| /// Executor mode to use. | ||
| #[arg(long = "executor-mode", value_enum, default_value = "normal", env)] | ||
| pub executor_mode: ExecutorMode, | ||
|
|
||
| /// Number of cores to use for the parallel executor. | ||
| #[cfg(feature = "parallel-executor")] | ||
| #[arg( | ||
| long = "executor-number-of-cores", | ||
| env, | ||
| default_value = "1", | ||
| alias = "executor-worker-count" | ||
| )] | ||
| pub executor_number_of_cores: NonZeroUsize, | ||
|
|
||
| /// Enable metrics for the parallel executor. | ||
| #[cfg(feature = "parallel-executor")] | ||
| #[arg(long = "executor-metrics", env)] | ||
| pub executor_metrics: bool, | ||
|
|
||
| /// Worker count policy for tx selection in parallel scheduler. | ||
| #[cfg(feature = "parallel-executor")] | ||
| #[arg( | ||
| long = "executor-worker-count-policy", | ||
| value_enum, | ||
| default_value = "static-max", | ||
| env | ||
| )] | ||
| pub executor_worker_count_policy: ParallelExecutorWorkerCountPolicy, | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -162,6 +162,7 @@ where | |
| D: ports::worker::OffChainDatabase, | ||
| { | ||
| fn process_block(&mut self, result: SharedImportResult) -> anyhow::Result<()> { | ||
| let instant = tokio::time::Instant::now(); | ||
| let block = &result.sealed_block.entity; | ||
| let mut transaction = self.database.transaction(); | ||
| // save the status for every transaction using the finalized block id | ||
|
|
@@ -170,12 +171,21 @@ where | |
| self.asset_metadata_indexation_enabled, | ||
| &mut transaction, | ||
| )?; | ||
| tracing::warn!( | ||
| "persist_transaction_status elapsed time: {:?}", | ||
| instant.elapsed() | ||
| ); | ||
|
|
||
| // save the associated owner for each transaction in the block | ||
| index_tx_owners_for_block(block, &mut transaction, &self.chain_id)?; | ||
| tracing::warn!( | ||
| "index_tx_owners_for_block elapsed time: {:?}", | ||
| instant.elapsed() | ||
| ); | ||
|
|
||
| // save the transaction related information | ||
| process_transactions(block.transactions().iter(), &mut transaction)?; | ||
| tracing::warn!("process_transactions elapsed time: {:?}", instant.elapsed()); | ||
|
|
||
| let height = block.header().height(); | ||
| let block_id = block.id(); | ||
|
|
@@ -186,6 +196,7 @@ where | |
| let total_tx_count = transaction | ||
| .increase_tx_count(block.transactions().len() as u64) | ||
| .unwrap_or_default(); | ||
| tracing::warn!("increase_tx_count elapsed time: {:?}", instant.elapsed()); | ||
|
|
||
| process_executor_events( | ||
| result.events.iter().map(Cow::Borrowed), | ||
|
|
@@ -194,8 +205,13 @@ where | |
| self.coins_to_spend_indexation_enabled, | ||
| &self.base_asset_id, | ||
| )?; | ||
| tracing::warn!( | ||
| "process_executor_events elapsed time: {:?}", | ||
| instant.elapsed() | ||
| ); | ||
|
|
||
| transaction.commit()?; | ||
| tracing::warn!("transaction.commit elapsed time: {:?}", instant.elapsed()); | ||
|
|
||
| for status in result.tx_status.iter() { | ||
| let tx_id = status.id; | ||
|
|
@@ -205,6 +221,11 @@ where | |
| .send_complete(tx_id, height, status.into()); | ||
| } | ||
|
|
||
| tracing::warn!( | ||
| "process_block elapsed time: {:?} for block: {:?}", | ||
| instant.elapsed(), | ||
| height | ||
| ); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Debug warn-level logging in block processing hot pathHigh Severity Seven Additional Locations (1) |
||
| // Notify subscribers and update last seen block height | ||
| self.shared_state | ||
| .block_height_subscription_handler | ||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Benchmark snapshot path changed to incorrect relative path
High Severity
The
PATH_SNAPSHOTconstant was changed from an absolute path to"./local-testnet", which is a gitignored directory. The benchmark will fail when run from most working directories since it expects the snapshot in the current directory. The correct path should be"../bin/fuel-core/chainspec/local-testnet"as used in other parts of the codebase, or the absolute path should be removed in favor of a configurable parameter rather than hardcoded.