From 18cb982d7c36783e3416a051c53dce361aa74077 Mon Sep 17 00:00:00 2001 From: rymnc <43716372+rymnc@users.noreply.github.com> Date: Thu, 2 Jan 2025 19:28:35 +0530 Subject: [PATCH] test: seqlock for gas price service? --- CHANGELOG.md | 1 + Cargo.lock | 4 +- benches/Cargo.toml | 8 ++ benches/benches/shared_gas_price_algorithm.rs | 78 +++++++++++++++++++ crates/fuel-core/src/graphql_api/ports.rs | 2 +- crates/fuel-core/src/schema/gas_price.rs | 1 - .../adapters/fuel_gas_price_provider.rs | 4 +- .../graph_ql_gas_price_estimate_tests.rs | 3 +- .../tests/producer_gas_price_tests.rs | 2 +- .../tests/tx_pool_gas_price_tests.rs | 2 +- .../src/service/adapters/graphql_api.rs | 2 +- crates/fuel-gas-price-algorithm/src/v0.rs | 2 +- crates/fuel-gas-price-algorithm/src/v1.rs | 2 +- crates/services/gas_price_service/Cargo.toml | 1 - .../src/common/gas_price_algorithm.rs | 16 ++-- .../gas_price_service/src/static_updater.rs | 2 +- .../gas_price_service/src/v0/service.rs | 6 +- .../gas_price_service/src/v1/service.rs | 8 +- crates/services/src/seqlock.rs | 6 ++ 19 files changed, 122 insertions(+), 28 deletions(-) create mode 100644 benches/benches/shared_gas_price_algorithm.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 38c9baed1e4..5a42d633e7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - [2033](https://github.com/FuelLabs/fuel-core/pull/2033): Remove `Option` in favor of `BlockHeightQuery` where applicable. - [2472](https://github.com/FuelLabs/fuel-core/pull/2472): Added the `amountU128` field to the `Balance` GraphQL schema, providing the total balance as a `U128`. The existing `amount` field clamps any balance exceeding `U64` to `u64::MAX`. - [2524](https://github.com/FuelLabs/fuel-core/pull/2524): Adds a new lock type which is optimized for certain workloads to the txpool and p2p services. +- [2528](https://github.com/FuelLabs/fuel-core/pull/2524): SeqLock benchmarks for `SharedGasPriceAlgo` and implementation. ### Fixed - [2365](https://github.com/FuelLabs/fuel-core/pull/2365): Fixed the error during dry run in the case of race condition. diff --git a/Cargo.lock b/Cargo.lock index 6fa3ea715a0..b57e741184c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3237,10 +3237,12 @@ dependencies = [ "fuel-core", "fuel-core-chain-config", "fuel-core-database", + "fuel-core-gas-price-service", "fuel-core-services", "fuel-core-storage", "fuel-core-sync", "fuel-core-types 0.40.2", + "fuel-gas-price-algorithm", "futures", "itertools 0.12.1", "num_enum", @@ -3249,6 +3251,7 @@ dependencies = [ "primitive-types", "quanta", "rand", + "rayon", "serde", "serde_json", "serde_yaml", @@ -3454,7 +3457,6 @@ dependencies = [ "fuel-gas-price-algorithm", "futures", "num_enum", - "parking_lot", "reqwest 0.12.10", "serde", "strum 0.25.0", diff --git a/benches/Cargo.toml b/benches/Cargo.toml index e4ee7db927b..dcfc82b84e1 100644 --- a/benches/Cargo.toml +++ b/benches/Cargo.toml @@ -13,6 +13,7 @@ criterion = { version = "0.5", features = [ "html_reports", "async", "async_tokio", + "async_futures", ] } ctrlc = "3.2.3" ed25519-dalek = { version = "2.0", features = ["rand_core"] } @@ -24,12 +25,14 @@ fuel-core = { path = "../crates/fuel-core", default-features = false, features = ] } fuel-core-chain-config = { workspace = true } fuel-core-database = { path = "./../crates/database" } +fuel-core-gas-price-service = { path = "./../crates/services/gas_price_service" } fuel-core-services = { path = "./../crates/services" } fuel-core-storage = { path = "./../crates/storage", features = ["smt"] } fuel-core-sync = { path = "./../crates/services/sync", features = [ "benchmarking", ] } fuel-core-types = { path = "./../crates/types", features = ["test-helpers"] } +fuel-gas-price-algorithm = { path = "./../crates/fuel-gas-price-algorithm" } futures = { workspace = true } itertools = { workspace = true } num_enum = { workspace = true } @@ -41,6 +44,7 @@ postcard = { workspace = true } primitive-types = { workspace = true, default-features = false } quanta = "0.12" rand = { workspace = true } +rayon = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } serde_yaml = "0.9.13" @@ -62,6 +66,10 @@ name = "state" harness = false name = "vm" +[[bench]] +harness = false +name = "shared_gas_price_algorithm" + [features] default = ["fuel-core/rocksdb"] diff --git a/benches/benches/shared_gas_price_algorithm.rs b/benches/benches/shared_gas_price_algorithm.rs new file mode 100644 index 00000000000..b0110aff077 --- /dev/null +++ b/benches/benches/shared_gas_price_algorithm.rs @@ -0,0 +1,78 @@ +use criterion::{ + criterion_group, + criterion_main, + Criterion, +}; +use fuel_core_gas_price_service::v1::algorithm::SharedV1Algorithm; +use fuel_gas_price_algorithm::v1::AlgorithmV1; + +#[inline] +fn dummy_algorithm() -> AlgorithmV1 { + AlgorithmV1::default() +} + +fn bench_shared_v1_algorithm(c: &mut Criterion) { + // bench initialization of SharedV1Algorithm + c.bench_function("SharedV1Algorithm::new_with_algorithm", |b| { + b.iter(|| { + let _ = SharedV1Algorithm::new_with_algorithm(dummy_algorithm()); + }) + }); + + // bench writes to SharedV1Algorithm + c.bench_function("SharedV1Algorithm::update", |b| { + let mut shared_v1_algorithm = + SharedV1Algorithm::new_with_algorithm(dummy_algorithm()); + + b.iter(|| { + shared_v1_algorithm.update(dummy_algorithm()); + }) + }); + + // bench reads from SharedV1Algorithm + c.bench_function("SharedV1Algorithm::next_gas_price", |b| { + let shared_v1_algorithm = + SharedV1Algorithm::new_with_algorithm(dummy_algorithm()); + + b.iter(|| { + let _ = shared_v1_algorithm.next_gas_price(); + }) + }); + + // bench concurrent reads and writes to SharedV1Algorithm + const READER_THREADS: usize = 4; + c.bench_function("SharedV1Algorithm::concurrent_rw", |b| { + let shared_v1_algorithm = + SharedV1Algorithm::new_with_algorithm(dummy_algorithm()); + b.iter_custom(|iters| { + let read_lock = shared_v1_algorithm.clone(); + let mut write_lock = shared_v1_algorithm.clone(); + let start = std::time::Instant::now(); + + // Simulate parallel reads and writes + rayon::scope(|s| { + // Writer thread + s.spawn(|_| { + for _ in 0..iters { + write_lock.update(dummy_algorithm()); + } + }); + + // Reader threads + for _ in 0..READER_THREADS { + let read_lock = read_lock.clone(); + s.spawn(move |_| { + for _ in 0..(iters / READER_THREADS as u64) { + let _ = read_lock.next_gas_price(); + } + }); + } + }); + + start.elapsed() + }); + }); +} + +criterion_group!(benches, bench_shared_v1_algorithm); +criterion_main!(benches); diff --git a/crates/fuel-core/src/graphql_api/ports.rs b/crates/fuel-core/src/graphql_api/ports.rs index 0463ee3992e..015a54bbe5d 100644 --- a/crates/fuel-core/src/graphql_api/ports.rs +++ b/crates/fuel-core/src/graphql_api/ports.rs @@ -265,7 +265,7 @@ pub trait P2pPort: Send + Sync { #[async_trait::async_trait] pub trait GasPriceEstimate: Send + Sync { /// The worst case scenario for gas price at a given horizon - async fn worst_case_gas_price(&self, height: BlockHeight) -> Option; + fn worst_case_gas_price(&self, height: BlockHeight) -> Option; } /// Trait for getting VM memory. diff --git a/crates/fuel-core/src/schema/gas_price.rs b/crates/fuel-core/src/schema/gas_price.rs index 7f7c7555e6d..c321855c94d 100644 --- a/crates/fuel-core/src/schema/gas_price.rs +++ b/crates/fuel-core/src/schema/gas_price.rs @@ -101,7 +101,6 @@ impl EstimateGasPriceQuery { let gas_price_provider = ctx.data_unchecked::(); let gas_price = gas_price_provider .worst_case_gas_price(target_block.into()) - .await .ok_or(async_graphql::Error::new(format!( "Failed to estimate gas price for block, algorithm not yet set: {target_block:?}" )))?; diff --git a/crates/fuel-core/src/service/adapters/fuel_gas_price_provider.rs b/crates/fuel-core/src/service/adapters/fuel_gas_price_provider.rs index aeb17627ccf..9174b4cf8b6 100644 --- a/crates/fuel-core/src/service/adapters/fuel_gas_price_provider.rs +++ b/crates/fuel-core/src/service/adapters/fuel_gas_price_provider.rs @@ -80,7 +80,7 @@ impl GraphqlGasPriceEstimate for FuelGasPriceProvider where A: GasPriceAlgorithm + Send + Sync, { - async fn worst_case_gas_price(&self, height: BlockHeight) -> Option { - Some(self.algorithm.worst_case_gas_price(height).await) + fn worst_case_gas_price(&self, height: BlockHeight) -> Option { + Some(self.algorithm.worst_case_gas_price(height)) } } diff --git a/crates/fuel-core/src/service/adapters/fuel_gas_price_provider/tests/graph_ql_gas_price_estimate_tests.rs b/crates/fuel-core/src/service/adapters/fuel_gas_price_provider/tests/graph_ql_gas_price_estimate_tests.rs index 18aa966e321..fee9703b736 100644 --- a/crates/fuel-core/src/service/adapters/fuel_gas_price_provider/tests/graph_ql_gas_price_estimate_tests.rs +++ b/crates/fuel-core/src/service/adapters/fuel_gas_price_provider/tests/graph_ql_gas_price_estimate_tests.rs @@ -7,13 +7,12 @@ async fn estimate_gas_price__happy_path() { let next_height = 432.into(); let price = 33; let algo = StaticAlgorithm::new(price); - let gas_price_provider = build_provider(algo.clone()); + let gas_price_provider = build_provider(algo); // when let expected_price = algo.worst_case_gas_price(next_height); let actual_price = gas_price_provider .worst_case_gas_price(next_height) - .await .unwrap(); // then diff --git a/crates/fuel-core/src/service/adapters/fuel_gas_price_provider/tests/producer_gas_price_tests.rs b/crates/fuel-core/src/service/adapters/fuel_gas_price_provider/tests/producer_gas_price_tests.rs index d8929cb4fdf..abcb6e2054f 100644 --- a/crates/fuel-core/src/service/adapters/fuel_gas_price_provider/tests/producer_gas_price_tests.rs +++ b/crates/fuel-core/src/service/adapters/fuel_gas_price_provider/tests/producer_gas_price_tests.rs @@ -9,7 +9,7 @@ async fn gas_price__if_requested_block_height_is_latest_return_gas_price() { // given let price = 33; let algo = StaticAlgorithm::new(price); - let gas_price_provider = build_provider(algo.clone()); + let gas_price_provider = build_provider(algo); // when let expected_price = algo.next_gas_price(); diff --git a/crates/fuel-core/src/service/adapters/fuel_gas_price_provider/tests/tx_pool_gas_price_tests.rs b/crates/fuel-core/src/service/adapters/fuel_gas_price_provider/tests/tx_pool_gas_price_tests.rs index d8929cb4fdf..abcb6e2054f 100644 --- a/crates/fuel-core/src/service/adapters/fuel_gas_price_provider/tests/tx_pool_gas_price_tests.rs +++ b/crates/fuel-core/src/service/adapters/fuel_gas_price_provider/tests/tx_pool_gas_price_tests.rs @@ -9,7 +9,7 @@ async fn gas_price__if_requested_block_height_is_latest_return_gas_price() { // given let price = 33; let algo = StaticAlgorithm::new(price); - let gas_price_provider = build_provider(algo.clone()); + let gas_price_provider = build_provider(algo); // when let expected_price = algo.next_gas_price(); diff --git a/crates/fuel-core/src/service/adapters/graphql_api.rs b/crates/fuel-core/src/service/adapters/graphql_api.rs index 43cd0471794..8223e086f9f 100644 --- a/crates/fuel-core/src/service/adapters/graphql_api.rs +++ b/crates/fuel-core/src/service/adapters/graphql_api.rs @@ -176,7 +176,7 @@ impl worker::TxPool for TxPoolAdapter { #[async_trait::async_trait] impl GasPriceEstimate for StaticGasPrice { - async fn worst_case_gas_price(&self, _height: BlockHeight) -> Option { + fn worst_case_gas_price(&self, _height: BlockHeight) -> Option { Some(self.gas_price) } } diff --git a/crates/fuel-gas-price-algorithm/src/v0.rs b/crates/fuel-gas-price-algorithm/src/v0.rs index 126d70a3669..7ee0902063d 100644 --- a/crates/fuel-gas-price-algorithm/src/v0.rs +++ b/crates/fuel-gas-price-algorithm/src/v0.rs @@ -14,7 +14,7 @@ pub enum Error { SkippedL2Block { expected: u32, got: u32 }, } -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq, Copy)] pub struct AlgorithmV0 { /// The gas price for to cover the execution of the next block new_exec_price: u64, diff --git a/crates/fuel-gas-price-algorithm/src/v1.rs b/crates/fuel-gas-price-algorithm/src/v1.rs index ac670d7b38f..5456b8f612f 100644 --- a/crates/fuel-gas-price-algorithm/src/v1.rs +++ b/crates/fuel-gas-price-algorithm/src/v1.rs @@ -27,7 +27,7 @@ pub enum Error { // TODO: separate exec gas price and DA gas price into newtypes for clarity // https://github.com/FuelLabs/fuel-core/issues/2382 -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq, Default, Copy)] pub struct AlgorithmV1 { /// The gas price for to cover the execution of the next block new_exec_price: u64, diff --git a/crates/services/gas_price_service/Cargo.toml b/crates/services/gas_price_service/Cargo.toml index 9903bba7d03..05296b1e70b 100644 --- a/crates/services/gas_price_service/Cargo.toml +++ b/crates/services/gas_price_service/Cargo.toml @@ -18,7 +18,6 @@ fuel-core-types = { workspace = true, features = ["std"] } fuel-gas-price-algorithm = { workspace = true } futures = { workspace = true } num_enum = { workspace = true } -parking_lot = { workspace = true } reqwest = { workspace = true, features = ["json"] } serde = { workspace = true } strum = { workspace = true, features = ["derive"] } diff --git a/crates/services/gas_price_service/src/common/gas_price_algorithm.rs b/crates/services/gas_price_service/src/common/gas_price_algorithm.rs index 1520a09740b..b3f691f7e63 100644 --- a/crates/services/gas_price_service/src/common/gas_price_algorithm.rs +++ b/crates/services/gas_price_service/src/common/gas_price_algorithm.rs @@ -1,13 +1,14 @@ +use fuel_core_services::seqlock::SeqLock; use fuel_core_types::fuel_types::BlockHeight; use std::sync::Arc; -pub trait GasPriceAlgorithm { +pub trait GasPriceAlgorithm: Copy { fn next_gas_price(&self) -> u64; fn worst_case_gas_price(&self, block_height: BlockHeight) -> u64; } #[derive(Debug, Default)] -pub struct SharedGasPriceAlgo(Arc>); +pub struct SharedGasPriceAlgo(Arc>); impl Clone for SharedGasPriceAlgo { fn clone(&self) -> Self { @@ -20,12 +21,13 @@ where A: Send + Sync, { pub fn new_with_algorithm(algorithm: A) -> Self { - Self(Arc::new(parking_lot::RwLock::new(algorithm))) + Self(Arc::new(SeqLock::new(algorithm))) } - pub async fn update(&mut self, new_algo: A) { - let mut write_lock = self.0.write(); - *write_lock = new_algo; + pub fn update(&mut self, new_algo: A) { + self.0.write(|data| { + *data = new_algo; + }); } } @@ -37,7 +39,7 @@ where self.0.read().next_gas_price() } - pub async fn worst_case_gas_price(&self, block_height: BlockHeight) -> u64 { + pub fn worst_case_gas_price(&self, block_height: BlockHeight) -> u64 { self.0.read().worst_case_gas_price(block_height) } } diff --git a/crates/services/gas_price_service/src/static_updater.rs b/crates/services/gas_price_service/src/static_updater.rs index 8185264c86c..d61fee346d6 100644 --- a/crates/services/gas_price_service/src/static_updater.rs +++ b/crates/services/gas_price_service/src/static_updater.rs @@ -11,7 +11,7 @@ impl StaticAlgorithmUpdater { } } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Copy)] pub struct StaticAlgorithm { price: u64, } diff --git a/crates/services/gas_price_service/src/v0/service.rs b/crates/services/gas_price_service/src/v0/service.rs index c4237b1141e..70481234ac0 100644 --- a/crates/services/gas_price_service/src/v0/service.rs +++ b/crates/services/gas_price_service/src/v0/service.rs @@ -62,8 +62,8 @@ where self.shared_algo.clone() } - async fn update(&mut self, new_algorithm: AlgorithmV0) { - self.shared_algo.update(new_algorithm).await; + fn update(&mut self, new_algorithm: AlgorithmV0) { + self.shared_algo.update(new_algorithm); } fn validate_block_gas_capacity( @@ -115,7 +115,7 @@ where } } - self.update(self.algorithm_updater.algorithm()).await; + self.update(self.algorithm_updater.algorithm()); Ok(()) } } diff --git a/crates/services/gas_price_service/src/v1/service.rs b/crates/services/gas_price_service/src/v1/service.rs index df0c91a5c93..55756703916 100644 --- a/crates/services/gas_price_service/src/v1/service.rs +++ b/crates/services/gas_price_service/src/v1/service.rs @@ -135,8 +135,8 @@ where &self.storage_tx_provider } - async fn update(&mut self, new_algorithm: AlgorithmV1) { - self.shared_algo.update(new_algorithm).await; + fn update(&mut self, new_algorithm: AlgorithmV1) { + self.shared_algo.update(new_algorithm); } fn validate_block_gas_capacity( @@ -193,7 +193,7 @@ where .map_err(|err| anyhow!(err))?; AtomicStorage::commit_transaction(storage_tx)?; let new_algo = self.algorithm_updater.algorithm(); - self.shared_algo.update(new_algo).await; + self.shared_algo.update(new_algo); // Clear the buffer after committing changes self.da_block_costs_buffer.clear(); Ok(()) @@ -210,7 +210,7 @@ where tx.set_metadata(&metadata).map_err(|err| anyhow!(err))?; AtomicStorage::commit_transaction(tx)?; let new_algo = self.algorithm_updater.algorithm(); - self.shared_algo.update(new_algo).await; + self.shared_algo.update(new_algo); } BlockInfo::Block { height, diff --git a/crates/services/src/seqlock.rs b/crates/services/src/seqlock.rs index d0f2b7ab920..f97129571f4 100644 --- a/crates/services/src/seqlock.rs +++ b/crates/services/src/seqlock.rs @@ -20,6 +20,12 @@ pub struct SeqLock { unsafe impl Sync for SeqLock {} +impl Default for SeqLock { + fn default() -> Self { + Self::new(T::default()) + } +} + impl SeqLock { /// Create a new SeqLock with the given data pub fn new(data: T) -> Self {