diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 1c33742e84..8d7b7cd6fb 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -346,18 +346,31 @@ struct CachedExecutorsEntry { executor: Arc, hit_count: AtomicU64, } + +impl Clone for CachedExecutorsEntry { + fn clone(&self) -> Self { + Self { + prev_epoch_count: self.prev_epoch_count, + epoch_count: AtomicU64::new(self.epoch_count.load(Relaxed)), + executor: self.executor.clone(), + hit_count: AtomicU64::new(self.hit_count.load(Relaxed)), + } + } +} + /// LFU Cache of executors with single-epoch memory of usage counts #[derive(Debug)] struct CachedExecutors { - max: usize, + capacity: usize, current_epoch: Epoch, pub(self) executors: HashMap, stats: executor_cache::Stats, } + impl Default for CachedExecutors { fn default() -> Self { Self { - max: MAX_CACHED_EXECUTORS, + capacity: MAX_CACHED_EXECUTORS, current_epoch: Epoch::default(), executors: HashMap::default(), stats: executor_cache::Stats::default(), @@ -375,59 +388,46 @@ impl AbiExample for CachedExecutors { } } -impl Clone for CachedExecutors { - fn clone(&self) -> Self { - let executors = self.executors.iter().map(|(&key, entry)| { - let entry = CachedExecutorsEntry { - prev_epoch_count: entry.prev_epoch_count, - epoch_count: AtomicU64::new(entry.epoch_count.load(Relaxed)), - executor: entry.executor.clone(), - hit_count: AtomicU64::new(entry.hit_count.load(Relaxed)), - }; - (key, entry) - }); - Self { - max: self.max, - current_epoch: self.current_epoch, - executors: executors.collect(), - stats: executor_cache::Stats::default(), - } - } -} - impl CachedExecutors { - fn clone_with_epoch(self: &Arc, epoch: Epoch) -> Arc { - if self.current_epoch == epoch { - return self.clone(); - } - let executors = self.executors.iter().map(|(&key, entry)| { - // The total_count = prev_epoch_count + epoch_count will be used for LFU eviction. - // If the epoch has changed, we store the prev_epoch_count and reset the epoch_count to 0. - let entry = CachedExecutorsEntry { - prev_epoch_count: entry.epoch_count.load(Relaxed), - epoch_count: AtomicU64::default(), - executor: entry.executor.clone(), - hit_count: AtomicU64::new(entry.hit_count.load(Relaxed)), - }; - (key, entry) - }); - Arc::new(Self { - max: self.max, - current_epoch: epoch, - executors: executors.collect(), - stats: executor_cache::Stats::default(), - }) - } - - fn new(max: usize, current_epoch: Epoch) -> Self { + fn new(max_capacity: usize, current_epoch: Epoch) -> Self { Self { - max, + capacity: max_capacity, current_epoch, executors: HashMap::new(), stats: executor_cache::Stats::default(), } } + fn new_from_parent_bank_executors( + parent_bank_executors: &CachedExecutors, + current_epoch: Epoch, + ) -> Self { + let executors = if parent_bank_executors.current_epoch == current_epoch { + parent_bank_executors.executors.clone() + } else { + parent_bank_executors + .executors + .iter() + .map(|(&key, entry)| { + let entry = CachedExecutorsEntry { + prev_epoch_count: entry.epoch_count.load(Relaxed), + epoch_count: AtomicU64::default(), + executor: entry.executor.clone(), + hit_count: AtomicU64::new(entry.hit_count.load(Relaxed)), + }; + (key, entry) + }) + .collect() + }; + + Self { + capacity: parent_bank_executors.capacity, + current_epoch, + executors, + stats: executor_cache::Stats::default(), + } + } + fn get(&self, pubkey: &Pubkey) -> Option> { if let Some(entry) = self.executors.get(pubkey) { self.stats.hits.fetch_add(1, Relaxed); @@ -469,7 +469,7 @@ impl CachedExecutors { let primer_counts = Self::get_primer_counts(counts.as_slice(), new_executors.len()); - if self.executors.len() >= self.max { + if self.executors.len() >= self.capacity { let mut least_keys = counts .iter() .take(new_executors.len()) @@ -1330,9 +1330,7 @@ pub struct Bank { pub rewards_pool_pubkeys: Arc>, /// Cached executors - // Inner Arc is meant to implement copy-on-write semantics as opposed to - // sharing mutations (hence RwLock> instead of Arc>). - cached_executors: RwLock>, + cached_executors: RwLock, transaction_debug_keys: Option>>, @@ -1517,7 +1515,7 @@ impl Bank { cluster_type: Option::::default(), lazy_rent_collection: AtomicBool::default(), rewards_pool_pubkeys: Arc::>::default(), - cached_executors: RwLock::>::default(), + cached_executors: RwLock::::default(), transaction_debug_keys: Option::>>::default(), transaction_log_collector_config: Arc::>::default( ), @@ -1777,8 +1775,11 @@ impl Bank { let (cached_executors, cached_executors_time) = Measure::this( |_| { - let cached_executors = parent.cached_executors.read().unwrap(); - RwLock::new(cached_executors.clone_with_epoch(epoch)) + let parent_bank_executors = parent.cached_executors.read().unwrap(); + RwLock::new(CachedExecutors::new_from_parent_bank_executors( + &parent_bank_executors, + epoch, + )) }, (), "cached_executors_creation", @@ -2205,10 +2206,7 @@ impl Bank { cluster_type: Some(genesis_config.cluster_type), lazy_rent_collection: new(), rewards_pool_pubkeys: new(), - cached_executors: RwLock::new(Arc::new(CachedExecutors::new( - MAX_CACHED_EXECUTORS, - fields.epoch, - ))), + cached_executors: RwLock::new(CachedExecutors::new(MAX_CACHED_EXECUTORS, fields.epoch)), transaction_debug_keys: debug_keys, transaction_log_collector_config: new(), transaction_log_collector: new(), @@ -4193,15 +4191,17 @@ impl Bank { return Rc::new(RefCell::new(Executors::default())); } - let cache = self.cached_executors.read().unwrap(); - let executors = executable_keys - .into_iter() - .filter_map(|key| { - cache - .get(key) - .map(|executor| (*key, TransactionExecutor::new_cached(executor))) - }) - .collect(); + let executors = { + let cache = self.cached_executors.read().unwrap(); + executable_keys + .into_iter() + .filter_map(|key| { + cache + .get(key) + .map(|executor| (*key, TransactionExecutor::new_cached(executor))) + }) + .collect() + }; Rc::new(RefCell::new(executors)) } @@ -4229,21 +4229,17 @@ impl Bank { .collect(); if !dirty_executors.is_empty() { - let mut cache = self.cached_executors.write().unwrap(); - let cache = Arc::make_mut(&mut cache); - cache.put(&dirty_executors); + self.cached_executors.write().unwrap().put(&dirty_executors); } } /// Remove an executor from the bank's cache fn remove_executor(&self, pubkey: &Pubkey) { - let mut cache = self.cached_executors.write().unwrap(); - let _ = Arc::make_mut(&mut cache).remove(pubkey); + let _ = self.cached_executors.write().unwrap().remove(pubkey); } pub fn clear_executors(&self) { - let mut cache = self.cached_executors.write().unwrap(); - Arc::make_mut(&mut cache).clear(); + self.cached_executors.write().unwrap().clear(); } /// Execute a transaction using the provided loaded accounts and update @@ -14664,13 +14660,13 @@ pub(crate) mod tests { assert!(cache.get(&key1).is_some()); assert!(cache.get(&key1).is_some()); - let mut cache = Arc::new(cache).clone_with_epoch(1); + let mut cache = CachedExecutors::new_from_parent_bank_executors(&cache, 1); assert!(cache.current_epoch == 1); assert!(cache.get(&key2).is_some()); assert!(cache.get(&key2).is_some()); assert!(cache.get(&key3).is_some()); - Arc::make_mut(&mut cache).put(&[(&key4, executor.clone())]); + cache.put(&[(&key4, executor.clone())]); assert!(cache.get(&key4).is_some()); let num_retained = [&key1, &key2, &key3] @@ -14679,8 +14675,8 @@ pub(crate) mod tests { .count(); assert_eq!(num_retained, 2); - Arc::make_mut(&mut cache).put(&[(&key1, executor.clone())]); - Arc::make_mut(&mut cache).put(&[(&key3, executor.clone())]); + cache.put(&[(&key1, executor.clone())]); + cache.put(&[(&key3, executor.clone())]); assert!(cache.get(&key1).is_some()); assert!(cache.get(&key3).is_some()); let num_retained = [&key2, &key4] @@ -14689,10 +14685,10 @@ pub(crate) mod tests { .count(); assert_eq!(num_retained, 1); - cache = cache.clone_with_epoch(2); + cache = CachedExecutors::new_from_parent_bank_executors(&cache, 2); assert!(cache.current_epoch == 2); - Arc::make_mut(&mut cache).put(&[(&key3, executor.clone())]); + cache.put(&[(&key3, executor.clone())]); assert!(cache.get(&key3).is_some()); } @@ -14758,6 +14754,94 @@ pub(crate) mod tests { assert_eq!(cache.stats.one_hit_wonders.load(Relaxed), 1); } + #[test] + fn test_cached_executors_stats() { + #[derive(Debug, Default, PartialEq)] + struct ComparableStats { + hits: u64, + misses: u64, + evictions: HashMap, + insertions: u64, + replacements: u64, + one_hit_wonders: u64, + } + impl From<&executor_cache::Stats> for ComparableStats { + fn from(stats: &executor_cache::Stats) -> Self { + let executor_cache::Stats { + hits, + misses, + evictions, + insertions, + replacements, + one_hit_wonders, + } = stats; + ComparableStats { + hits: hits.load(Relaxed), + misses: misses.load(Relaxed), + evictions: evictions.clone(), + insertions: insertions.load(Relaxed), + replacements: replacements.load(Relaxed), + one_hit_wonders: one_hit_wonders.load(Relaxed), + } + } + } + + const CURRENT_EPOCH: Epoch = 0; + let mut cache = CachedExecutors::new(2, CURRENT_EPOCH); + let mut expected_stats = ComparableStats::default(); + + let program_id1 = Pubkey::new_unique(); + let program_id2 = Pubkey::new_unique(); + let executor: Arc = Arc::new(TestExecutor {}); + + // make sure we're starting from where we think we are + assert_eq!(ComparableStats::from(&cache.stats), expected_stats,); + + // insert some executors + cache.put(&[(&program_id1, executor.clone())]); + cache.put(&[(&program_id2, executor.clone())]); + expected_stats.insertions += 2; + assert_eq!(ComparableStats::from(&cache.stats), expected_stats); + + // replace a one-hit-wonder executor + cache.put(&[(&program_id1, executor.clone())]); + expected_stats.replacements += 1; + expected_stats.one_hit_wonders += 1; + assert_eq!(ComparableStats::from(&cache.stats), expected_stats); + + // hit some executors + cache.get(&program_id1); + cache.get(&program_id1); + cache.get(&program_id2); + expected_stats.hits += 3; + assert_eq!(ComparableStats::from(&cache.stats), expected_stats); + + // miss an executor + cache.get(&Pubkey::new_unique()); + expected_stats.misses += 1; + assert_eq!(ComparableStats::from(&cache.stats), expected_stats); + + // evict an executor + cache.put(&[(&Pubkey::new_unique(), executor.clone())]); + expected_stats.insertions += 1; + expected_stats.evictions.insert(program_id2, 1); + assert_eq!(ComparableStats::from(&cache.stats), expected_stats); + + // make sure stats are cleared in new_from_parent + assert_eq!( + ComparableStats::from( + &CachedExecutors::new_from_parent_bank_executors(&cache, CURRENT_EPOCH).stats + ), + ComparableStats::default() + ); + assert_eq!( + ComparableStats::from( + &CachedExecutors::new_from_parent_bank_executors(&cache, CURRENT_EPOCH + 1).stats + ), + ComparableStats::default() + ); + } + #[test] fn test_bank_executor_cache() { solana_logger::setup();