From 7ac0d542b6c4dc738f7a6d65f69ce0c681746c54 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Sat, 7 Feb 2026 11:13:16 -0800 Subject: [PATCH] refactor(engine): wrap ExecutionCache internals in single Arc (#21950) Co-authored-by: Amp --- crates/engine/tree/src/tree/cached_state.rs | 120 ++++++++++---------- 1 file changed, 59 insertions(+), 61 deletions(-) diff --git a/crates/engine/tree/src/tree/cached_state.rs b/crates/engine/tree/src/tree/cached_state.rs index a6956c93f3..02bd2b4e0f 100644 --- a/crates/engine/tree/src/tree/cached_state.rs +++ b/crates/engine/tree/src/tree/cached_state.rs @@ -315,7 +315,7 @@ impl AccountReader for CachedStateProvider { })? { CachedStatus::NotCached(value) | CachedStatus::Cached(value) => Ok(value), } - } else if let Some(account) = self.caches.account_cache.get(address) { + } else if let Some(account) = self.caches.0.account_cache.get(address) { self.metrics.account_cache_hits.increment(1); Ok(account) } else { @@ -350,7 +350,7 @@ impl StateProvider for CachedStateProvider { Ok(Some(value).filter(|v| !v.is_zero())) } } - } else if let Some(value) = self.caches.storage_cache.get(&(account, storage_key)) { + } else if let Some(value) = self.caches.0.storage_cache.get(&(account, storage_key)) { self.metrics.storage_cache_hits.increment(1); Ok(Some(value).filter(|v| !v.is_zero())) } else { @@ -368,7 +368,7 @@ impl BytecodeReader for CachedStateProvider { })? { CachedStatus::NotCached(code) | CachedStatus::Cached(code) => Ok(code), } - } else if let Some(code) = self.caches.code_cache.get(code_hash) { + } else if let Some(code) = self.caches.0.code_cache.get(code_hash) { self.metrics.code_cache_hits.increment(1); Ok(code) } else { @@ -488,27 +488,31 @@ impl HashedPostStateProvider for CachedStateProvider /// Since EIP-6780, SELFDESTRUCT only works within the same transaction where the /// contract was created, so we don't need to handle clearing the storage. #[derive(Debug, Clone)] -pub struct ExecutionCache { +pub struct ExecutionCache(Arc); + +/// Inner state of the [`ExecutionCache`], wrapped in a single [`Arc`]. +#[derive(Debug)] +struct ExecutionCacheInner { /// Cache for contract bytecode, keyed by code hash. - code_cache: Arc, FbBuildHasher<32>>>, + code_cache: FixedCache, FbBuildHasher<32>>, /// Flat storage cache: maps `(Address, StorageKey)` to storage value. - storage_cache: Arc>, + storage_cache: FixedCache<(Address, StorageKey), StorageValue>, /// Cache for basic account information (nonce, balance, code hash). - account_cache: Arc, FbBuildHasher<20>>>, + account_cache: FixedCache, FbBuildHasher<20>>, - /// Stats handler for the code cache. + /// Stats handler for the code cache (shared with the cache via [`Stats`]). code_stats: Arc, - /// Stats handler for the storage cache. + /// Stats handler for the storage cache (shared with the cache via [`Stats`]). storage_stats: Arc, - /// Stats handler for the account cache. + /// Stats handler for the account cache (shared with the cache via [`Stats`]). account_stats: Arc, /// One-time notification when SELFDESTRUCT is encountered - selfdestruct_encountered: Arc, + selfdestruct_encountered: Once, } impl ExecutionCache { @@ -546,24 +550,18 @@ impl ExecutionCache { let storage_stats = Arc::new(CacheStatsHandler::new(storage_capacity)); let account_stats = Arc::new(CacheStatsHandler::new(account_capacity)); - Self { - code_cache: Arc::new( - FixedCache::new(code_capacity, FbBuildHasher::<32>::default()) - .with_stats(Some(Stats::new(code_stats.clone()))), - ), - storage_cache: Arc::new( - FixedCache::new(storage_capacity, DefaultHashBuilder::default()) - .with_stats(Some(Stats::new(storage_stats.clone()))), - ), - account_cache: Arc::new( - FixedCache::new(account_capacity, FbBuildHasher::<20>::default()) - .with_stats(Some(Stats::new(account_stats.clone()))), - ), + Self(Arc::new(ExecutionCacheInner { + code_cache: FixedCache::new(code_capacity, FbBuildHasher::<32>::default()) + .with_stats(Some(Stats::new(code_stats.clone()))), + storage_cache: FixedCache::new(storage_capacity, DefaultHashBuilder::default()) + .with_stats(Some(Stats::new(storage_stats.clone()))), + account_cache: FixedCache::new(account_capacity, FbBuildHasher::<20>::default()) + .with_stats(Some(Stats::new(account_stats.clone()))), code_stats, storage_stats, account_stats, - selfdestruct_encountered: Arc::default(), - } + selfdestruct_encountered: Once::new(), + })) } /// Gets code from cache, or inserts using the provided function. @@ -573,7 +571,7 @@ impl ExecutionCache { f: impl FnOnce() -> Result, E>, ) -> Result>, E> { let mut miss = false; - let result = self.code_cache.get_or_try_insert_with(hash, |_| { + let result = self.0.code_cache.get_or_try_insert_with(hash, |_| { miss = true; f() })?; @@ -593,7 +591,7 @@ impl ExecutionCache { f: impl FnOnce() -> Result, ) -> Result, E> { let mut miss = false; - let result = self.storage_cache.get_or_try_insert_with((address, key), |_| { + let result = self.0.storage_cache.get_or_try_insert_with((address, key), |_| { miss = true; f() })?; @@ -612,7 +610,7 @@ impl ExecutionCache { f: impl FnOnce() -> Result, E>, ) -> Result>, E> { let mut miss = false; - let result = self.account_cache.get_or_try_insert_with(address, |_| { + let result = self.0.account_cache.get_or_try_insert_with(address, |_| { miss = true; f() })?; @@ -626,17 +624,17 @@ impl ExecutionCache { /// Insert storage value into cache. pub fn insert_storage(&self, address: Address, key: StorageKey, value: Option) { - self.storage_cache.insert((address, key), value.unwrap_or_default()); + self.0.storage_cache.insert((address, key), value.unwrap_or_default()); } /// Insert code into cache. fn insert_code(&self, hash: B256, code: Option) { - self.code_cache.insert(hash, code); + self.0.code_cache.insert(hash, code); } /// Insert account into cache. fn insert_account(&self, address: Address, account: Option) { - self.account_cache.insert(address, account); + self.0.account_cache.insert(address, account); } /// Inserts the post-execution state changes into the cache. @@ -694,7 +692,7 @@ impl ExecutionCache { let had_code = account.original_info.as_ref().is_some_and(|info| !info.is_empty_code_hash()); if had_code { - self.selfdestruct_encountered.call_once(|| { + self.0.selfdestruct_encountered.call_once(|| { warn!( target: "engine::caching", address = ?addr, @@ -707,7 +705,7 @@ impl ExecutionCache { return Ok(()) } - self.account_cache.remove(addr); + self.0.account_cache.remove(addr); continue } @@ -737,30 +735,30 @@ impl ExecutionCache { /// We do not clear the bytecodes cache, because its mapping can never change, as it's /// `keccak256(bytecode) => bytecode`. pub(crate) fn clear(&self) { - self.storage_cache.clear(); - self.account_cache.clear(); + self.0.storage_cache.clear(); + self.0.account_cache.clear(); - self.storage_stats.reset_size(); - self.account_stats.reset_size(); + self.0.storage_stats.reset_size(); + self.0.account_stats.reset_size(); } /// Updates the provided metrics with the current stats from the cache's stats handlers, /// and resets the hit/miss/collision counters. pub(crate) fn update_metrics(&self, metrics: &CachedStateMetrics) { - metrics.code_cache_size.set(self.code_stats.size() as f64); - metrics.code_cache_capacity.set(self.code_stats.capacity() as f64); - metrics.code_cache_collisions.set(self.code_stats.collisions() as f64); - self.code_stats.reset_stats(); + metrics.code_cache_size.set(self.0.code_stats.size() as f64); + metrics.code_cache_capacity.set(self.0.code_stats.capacity() as f64); + metrics.code_cache_collisions.set(self.0.code_stats.collisions() as f64); + self.0.code_stats.reset_stats(); - metrics.storage_cache_size.set(self.storage_stats.size() as f64); - metrics.storage_cache_capacity.set(self.storage_stats.capacity() as f64); - metrics.storage_cache_collisions.set(self.storage_stats.collisions() as f64); - self.storage_stats.reset_stats(); + metrics.storage_cache_size.set(self.0.storage_stats.size() as f64); + metrics.storage_cache_capacity.set(self.0.storage_stats.capacity() as f64); + metrics.storage_cache_collisions.set(self.0.storage_stats.collisions() as f64); + self.0.storage_stats.reset_stats(); - metrics.account_cache_size.set(self.account_stats.size() as f64); - metrics.account_cache_capacity.set(self.account_stats.capacity() as f64); - metrics.account_cache_collisions.set(self.account_stats.collisions() as f64); - self.account_stats.reset_stats(); + metrics.account_cache_size.set(self.0.account_stats.size() as f64); + metrics.account_cache_capacity.set(self.0.account_stats.capacity() as f64); + metrics.account_cache_collisions.set(self.0.account_stats.collisions() as f64); + self.0.account_stats.reset_stats(); } } @@ -971,9 +969,9 @@ mod tests { caches.insert_storage(addr1, storage_key, Some(U256::from(42))); // Verify caches are populated - assert!(caches.account_cache.get(&addr1).is_some()); - assert!(caches.account_cache.get(&addr2).is_some()); - assert!(caches.storage_cache.get(&(addr1, storage_key)).is_some()); + assert!(caches.0.account_cache.get(&addr1).is_some()); + assert!(caches.0.account_cache.get(&addr2).is_some()); + assert!(caches.0.storage_cache.get(&(addr1, storage_key)).is_some()); let bundle = BundleState { // BundleState with a destroyed contract (had code) @@ -1003,9 +1001,9 @@ mod tests { assert!(result.is_ok()); // Verify all caches were cleared - assert!(caches.account_cache.get(&addr1).is_none()); - assert!(caches.account_cache.get(&addr2).is_none()); - assert!(caches.storage_cache.get(&(addr1, storage_key)).is_none()); + assert!(caches.0.account_cache.get(&addr1).is_none()); + assert!(caches.0.account_cache.get(&addr2).is_none()); + assert!(caches.0.storage_cache.get(&(addr1, storage_key)).is_none()); } #[test] @@ -1047,9 +1045,9 @@ mod tests { assert!(caches.insert_state(&bundle).is_ok()); // Verify only addr1 was removed, other data is still present - assert!(caches.account_cache.get(&addr1).is_none()); - assert!(caches.account_cache.get(&addr2).is_some()); - assert!(caches.storage_cache.get(&(addr1, storage_key)).is_some()); + assert!(caches.0.account_cache.get(&addr1).is_none()); + assert!(caches.0.account_cache.get(&addr2).is_some()); + assert!(caches.0.storage_cache.get(&(addr1, storage_key)).is_some()); } #[test] @@ -1083,7 +1081,7 @@ mod tests { assert!(caches.insert_state(&bundle).is_ok()); // Verify only addr1 was removed - assert!(caches.account_cache.get(&addr1).is_none()); - assert!(caches.account_cache.get(&addr2).is_some()); + assert!(caches.0.account_cache.get(&addr1).is_none()); + assert!(caches.0.account_cache.get(&addr2).is_some()); } }