From f8ad40902e15fbebd67d8b0cfb50e605ce4f7732 Mon Sep 17 00:00:00 2001 From: AvivYossef-starkware Date: Sun, 23 Feb 2025 16:59:06 +0200 Subject: [PATCH] feat(blockifier): add cairo native cache rate metric --- Cargo.lock | 2 + .../src/state/contract_class_manager.rs | 6 ++ .../src/state/native_class_manager.rs | 60 ++++++++++++++++++- .../src/state/native_class_manager_test.rs | 2 + crates/starknet_batcher/Cargo.toml | 2 + crates/starknet_batcher/src/batcher.rs | 8 +++ crates/starknet_batcher/src/batcher_test.rs | 7 ++- crates/starknet_batcher/src/block_builder.rs | 7 +++ crates/starknet_batcher/src/metrics.rs | 4 ++ .../src/dashboard_definitions.rs | 8 +++ .../src/metric_definitions.rs | 3 +- 11 files changed, 105 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ee903e300fb..e0268aa116b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10871,6 +10871,8 @@ dependencies = [ "metrics 0.24.1", "metrics-exporter-prometheus", "mockall", + "num-rational 0.4.2", + "num-traits", "papyrus_config", "papyrus_state_reader", "papyrus_storage", diff --git a/crates/blockifier/src/state/contract_class_manager.rs b/crates/blockifier/src/state/contract_class_manager.rs index f47fc2b2183..644557ebfe1 100644 --- a/crates/blockifier/src/state/contract_class_manager.rs +++ b/crates/blockifier/src/state/contract_class_manager.rs @@ -7,6 +7,7 @@ pub use crate::state::native_class_manager::NativeClassManager as ContractClassM pub mod trivial_class_manager { #[cfg(any(feature = "testing", test))] use cached::Cached; + use num_rational::Ratio; use starknet_api::core::ClassHash; use crate::blockifier::config::ContractClassManagerConfig; @@ -44,6 +45,11 @@ pub mod trivial_class_manager { pub fn get_cache_size(&self) -> usize { self.cache.lock().cache_size() } + + pub fn get_cache_miss_rate(&self) -> Ratio { + // TODO(Aviv): consider support cache miss metrics. + Ratio::ZERO + } } } diff --git a/crates/blockifier/src/state/native_class_manager.rs b/crates/blockifier/src/state/native_class_manager.rs index 9f666af8c8c..8f027e62e4a 100644 --- a/crates/blockifier/src/state/native_class_manager.rs +++ b/crates/blockifier/src/state/native_class_manager.rs @@ -1,9 +1,11 @@ +use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::mpsc::{sync_channel, Receiver, SyncSender, TrySendError}; use std::sync::Arc; #[cfg(any(feature = "testing", test))] use cached::Cached; use log; +use num_rational::Ratio; use starknet_api::core::ClassHash; use starknet_api::state::SierraContractClass; use starknet_sierra_multicompile::command_line_compiler::CommandLineCompiler; @@ -49,6 +51,39 @@ pub struct NativeClassManager { sender: Option>, /// The sierra-to-native compiler. compiler: Option>, + /// cache_miss_rate + cache_metrics: CacheMetrics, +} + +#[derive(Default, Clone)] + +pub struct CacheMetrics { + cache_misses: Arc, + total_cache_calls: Arc, +} + +impl CacheMetrics { + pub fn new() -> Self { + Self { + cache_misses: Arc::new(AtomicUsize::new(0)), + total_cache_calls: Arc::new(AtomicUsize::new(0)), + } + } + + pub fn record_miss(&self) { + self.cache_misses.fetch_add(1, Ordering::Relaxed); + self.total_cache_calls.fetch_add(1, Ordering::Relaxed); + } + + pub fn record_hit(&self) { + self.total_cache_calls.fetch_add(1, Ordering::Relaxed); + } + + pub fn miss_rate(&self) -> Ratio { + let misses = self.cache_misses.load(Ordering::Relaxed); + let total = self.total_cache_calls.load(Ordering::Relaxed); + if total == 0 { Ratio::ZERO } else { Ratio::new(misses, total) } + } } impl NativeClassManager { @@ -70,6 +105,7 @@ impl NativeClassManager { cache, sender: None, compiler: None, + cache_metrics: CacheMetrics::new(), }; } @@ -82,6 +118,7 @@ impl NativeClassManager { cache, sender: None, compiler: Some(compiler), + cache_metrics: CacheMetrics::new(), }; } @@ -92,12 +129,27 @@ impl NativeClassManager { move || run_compilation_worker(cache, receiver, compiler) }); - NativeClassManager { cairo_native_run_config, cache, sender: Some(sender), compiler: None } + NativeClassManager { + cairo_native_run_config, + cache, + sender: Some(sender), + compiler: None, + cache_metrics: CacheMetrics::new(), + } } /// Returns the runnable compiled class for the given class hash, if it exists in cache. pub fn get_runnable(&self, class_hash: &ClassHash) -> Option { - let cached_class = self.cache.get(class_hash)?; + let cached_class = match self.cache.get(class_hash) { + Some(class) => { + self.cache_metrics.record_hit(); + class + } + None => { + self.cache_metrics.record_miss(); + return None; + } + }; if let CachedClass::V1(_, _) = cached_class { // TODO(Yoni): make sure `wait_on_native_compilation` cannot be set to true while // `run_cairo_native` is false. @@ -195,6 +247,10 @@ impl NativeClassManager { pub fn get_cache_size(&self) -> usize { self.cache.lock().cache_size() } + + pub fn get_cache_miss_rate(&self) -> Ratio { + self.cache_metrics.miss_rate() + } } /// Handles compilation requests from the channel, holding the receiver end of the channel. diff --git a/crates/blockifier/src/state/native_class_manager_test.rs b/crates/blockifier/src/state/native_class_manager_test.rs index f4a99a1568c..363e2713dee 100644 --- a/crates/blockifier/src/state/native_class_manager_test.rs +++ b/crates/blockifier/src/state/native_class_manager_test.rs @@ -19,6 +19,7 @@ use crate::state::global_cache::{ }; use crate::state::native_class_manager::{ process_compilation_request, + CacheMetrics, CompilationRequest, ContractClassManagerError, NativeClassManager, @@ -157,6 +158,7 @@ fn test_send_compilation_request_channel_disconnected() { cache: RawClassCache::new(GLOBAL_CONTRACT_CACHE_SIZE_FOR_TEST), sender: Some(sender), compiler: None, + cache_metrics: CacheMetrics::new(), }; // Disconnect the channel by dropping the receiver. drop(receiver); diff --git a/crates/starknet_batcher/Cargo.toml b/crates/starknet_batcher/Cargo.toml index f077e85bff9..3c271e04777 100644 --- a/crates/starknet_batcher/Cargo.toml +++ b/crates/starknet_batcher/Cargo.toml @@ -14,6 +14,8 @@ async-trait.workspace = true blockifier.workspace = true chrono.workspace = true indexmap.workspace = true +num-rational.workspace = true +num-traits.workspace = true papyrus_config.workspace = true papyrus_state_reader.workspace = true papyrus_storage.workspace = true diff --git a/crates/starknet_batcher/src/batcher.rs b/crates/starknet_batcher/src/batcher.rs index f16df46b280..d912b871812 100644 --- a/crates/starknet_batcher/src/batcher.rs +++ b/crates/starknet_batcher/src/batcher.rs @@ -6,6 +6,7 @@ use async_trait::async_trait; use blockifier::state::contract_class_manager::ContractClassManager; #[cfg(test)] use mockall::automock; +use num_traits::cast::ToPrimitive; use papyrus_storage::state::{StateStorageReader, StateStorageWriter}; use starknet_api::block::{BlockHeaderWithoutHash, BlockNumber}; use starknet_api::consensus_transaction::InternalConsensusTransaction; @@ -44,6 +45,7 @@ use starknet_sequencer_infra::component_definitions::{ }; use starknet_sequencer_metrics::metric_definitions::{ BATCHED_TRANSACTIONS, + CAIRO_NATIVE_CACHE_MISS_RATIO, REJECTED_TRANSACTIONS, STORAGE_HEIGHT, }; @@ -489,6 +491,12 @@ impl Batcher { let execution_infos: Vec<_> = block_execution_artifacts.execution_infos.into_iter().map(|(_, info)| info).collect(); + CAIRO_NATIVE_CACHE_MISS_RATIO.set( + self.block_builder_factory + .get_contract_class_manager_cache_miss_rate() + .to_f64() + .unwrap_or(0.0), + ); BATCHED_TRANSACTIONS.increment(n_txs); REJECTED_TRANSACTIONS.increment(n_rejected_txs); diff --git a/crates/starknet_batcher/src/batcher_test.rs b/crates/starknet_batcher/src/batcher_test.rs index 4f6649c6248..b3ab0f75e26 100644 --- a/crates/starknet_batcher/src/batcher_test.rs +++ b/crates/starknet_batcher/src/batcher_test.rs @@ -7,6 +7,7 @@ use blockifier::transaction::objects::TransactionExecutionInfo; use indexmap::indexmap; use metrics_exporter_prometheus::PrometheusBuilder; use mockall::predicate::eq; +use num_rational::Ratio; use rstest::rstest; use starknet_api::block::{BlockHeaderWithoutHash, BlockInfo, BlockNumber}; use starknet_api::consensus_transaction::InternalConsensusTransaction; @@ -117,12 +118,16 @@ impl Default for MockDependencies { let expected_gas_price = propose_block_input(PROPOSAL_ID).block_info.gas_prices.strk_gas_prices.l2_gas_price; mempool_client.expect_update_gas_price().with(eq(expected_gas_price)).returning(|_| Ok(())); + let mut block_builder_factory = MockBlockBuilderFactoryTrait::new(); + block_builder_factory + .expect_get_contract_class_manager_cache_miss_rate() + .return_const(Ratio::new(1, 2)); Self { storage_reader, storage_writer: MockBatcherStorageWriterTrait::new(), l1_provider_client: MockL1ProviderClient::new(), mempool_client, - block_builder_factory: MockBlockBuilderFactoryTrait::new(), + block_builder_factory, // TODO(noamsp): use MockClassManagerClient class_manager_client: Arc::new(EmptyClassManagerClient), } diff --git a/crates/starknet_batcher/src/block_builder.rs b/crates/starknet_batcher/src/block_builder.rs index 90d0f228a01..d2079ff7cf0 100644 --- a/crates/starknet_batcher/src/block_builder.rs +++ b/crates/starknet_batcher/src/block_builder.rs @@ -20,6 +20,7 @@ use blockifier::transaction::transaction_execution::Transaction as BlockifierTra use indexmap::{IndexMap, IndexSet}; #[cfg(test)] use mockall::automock; +use num_rational::Ratio; use papyrus_config::dumping::{append_sub_config_name, ser_param, SerializeConfig}; use papyrus_config::{ParamPath, ParamPrivacyInput, SerializedParam}; use papyrus_state_reader::papyrus_state::{ClassReader, PapyrusReader}; @@ -349,6 +350,8 @@ pub trait BlockBuilderFactoryTrait: Send + Sync { >, runtime: tokio::runtime::Handle, ) -> BlockBuilderResult<(Box, AbortSignalSender)>; + + fn get_contract_class_manager_cache_miss_rate(&self) -> Ratio; } #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] @@ -466,6 +469,10 @@ impl BlockBuilderFactoryTrait for BlockBuilderFactory { )); Ok((block_builder, abort_signal_sender)) } + + fn get_contract_class_manager_cache_miss_rate(&self) -> Ratio { + self.contract_class_manager.get_cache_miss_rate() + } } /// Supplementary information for use by downstream services. diff --git a/crates/starknet_batcher/src/metrics.rs b/crates/starknet_batcher/src/metrics.rs index 68e5935523d..5e9ffe1b2b4 100644 --- a/crates/starknet_batcher/src/metrics.rs +++ b/crates/starknet_batcher/src/metrics.rs @@ -1,6 +1,7 @@ use starknet_api::block::BlockNumber; use starknet_sequencer_metrics::metric_definitions::{ BATCHED_TRANSACTIONS, + CAIRO_NATIVE_CACHE_MISS_RATIO, PROPOSAL_ABORTED, PROPOSAL_FAILED, PROPOSAL_STARTED, @@ -14,6 +15,9 @@ pub fn register_metrics(storage_height: BlockNumber) { #[allow(clippy::as_conversions)] STORAGE_HEIGHT.set(storage_height.0 as f64); + CAIRO_NATIVE_CACHE_MISS_RATIO.register(); + CAIRO_NATIVE_CACHE_MISS_RATIO.set(0.0); + PROPOSAL_STARTED.register(); PROPOSAL_SUCCEEDED.register(); PROPOSAL_FAILED.register(); diff --git a/crates/starknet_sequencer_metrics/src/dashboard_definitions.rs b/crates/starknet_sequencer_metrics/src/dashboard_definitions.rs index b6f31c2246d..17241bdafb5 100644 --- a/crates/starknet_sequencer_metrics/src/dashboard_definitions.rs +++ b/crates/starknet_sequencer_metrics/src/dashboard_definitions.rs @@ -54,6 +54,13 @@ const PANEL_BATCHED_TRANSACTIONS: Panel = Panel::new( PanelType::Stat, ); +const PANEL_CAIRO_NATIVE_CACHE_MISS_RATIO: Panel = Panel::new( + "cairo_native_cache_miss_ratio", + "The ratio of cache misses in the Cairo native cache", + "cairo_native_cache_miss_ratio", + PanelType::Graph, +); + const PANEL_MEMPOOL_P2P_NUM_CONNECTED_PEERS: Panel = Panel::new( MEMPOOL_P2P_NUM_CONNECTED_PEERS.get_name(), MEMPOOL_P2P_NUM_CONNECTED_PEERS.get_description(), @@ -155,6 +162,7 @@ const BATCHER_ROW: Row<'_> = Row::new( PANEL_PROPOSAL_SUCCEEDED, PANEL_PROPOSAL_FAILED, PANEL_BATCHED_TRANSACTIONS, + PANEL_CAIRO_NATIVE_CACHE_MISS_RATIO, ], ); const HTTP_SERVER_ROW: Row<'_> = Row::new( diff --git a/crates/starknet_sequencer_metrics/src/metric_definitions.rs b/crates/starknet_sequencer_metrics/src/metric_definitions.rs index cb15f67a543..6c0bc4d0263 100644 --- a/crates/starknet_sequencer_metrics/src/metric_definitions.rs +++ b/crates/starknet_sequencer_metrics/src/metric_definitions.rs @@ -111,7 +111,8 @@ macro_rules! define_gauge_metrics { define_gauge_metrics!( MetricScope::Batcher => { - { STORAGE_HEIGHT, "batcher_storage_height", "The height of the batcher's storage" } + { STORAGE_HEIGHT, "batcher_storage_height", "The height of the batcher's storage" }, + { CAIRO_NATIVE_CACHE_MISS_RATIO, "cairo_native_cache_miss_ratio","The cache miss of cairo native"} }, MetricScope::Infra => { { BATCHER_LOCAL_QUEUE_DEPTH, "batcher_local_queue_depth", "The depth of the batcher's local message queue" },