diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index e270175300be69..38e3387467ab41 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -281,51 +281,6 @@ impl fmt::Debug for Builtin { } } -/// Copy-on-write holder of CachedExecutors -#[derive(AbiExample, Debug, Default)] -struct CowCachedExecutors { - shared: bool, - executors: Arc>, -} -impl Clone for CowCachedExecutors { - fn clone(&self) -> Self { - Self { - shared: true, - executors: self.executors.clone(), - } - } -} -impl CowCachedExecutors { - fn clone_with_epoch(&self, epoch: u64) -> Self { - let executors_raw = self.read().unwrap(); - if executors_raw.current_epoch() == epoch { - self.clone() - } else { - Self { - shared: false, - executors: Arc::new(RwLock::new(executors_raw.clone_with_epoch(epoch))), - } - } - } - fn new(executors: Arc>) -> Self { - Self { - shared: true, - executors, - } - } - fn read(&self) -> LockResult> { - self.executors.read() - } - fn write(&mut self) -> LockResult> { - if self.shared { - self.shared = false; - let local_cache = (*self.executors.read().unwrap()).clone(); - self.executors = Arc::new(RwLock::new(local_cache)); - } - self.executors.write() - } -} - #[cfg(RUSTC_WITH_SPECIALIZATION)] impl AbiExample for Builtin { fn example() -> Self { @@ -364,8 +319,8 @@ impl Default for CachedExecutors { fn default() -> Self { Self { max: MAX_CACHED_EXECUTORS, - current_epoch: 0, - executors: HashMap::new(), + current_epoch: Epoch::default(), + executors: HashMap::default(), } } } @@ -382,44 +337,42 @@ impl AbiExample for CachedExecutors { impl Clone for CachedExecutors { fn clone(&self) -> Self { - self.clone_with_epoch(self.current_epoch) + 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(), + }; + (key, entry) + }); + Self { + max: self.max, + current_epoch: self.current_epoch, + executors: executors.collect(), + } } } -impl CachedExecutors { - fn current_epoch(&self) -> Epoch { - self.current_epoch - } - fn clone_with_epoch(&self, epoch: Epoch) -> Self { - let mut executors = HashMap::new(); - for (key, entry) in self.executors.iter() { +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. - if epoch > self.current_epoch { - executors.insert( - *key, - CachedExecutorsEntry { - prev_epoch_count: entry.epoch_count.load(Relaxed), - epoch_count: AtomicU64::new(0), - executor: entry.executor.clone(), - }, - ); - } else { - executors.insert( - *key, - CachedExecutorsEntry { - prev_epoch_count: entry.prev_epoch_count, - epoch_count: AtomicU64::new(entry.epoch_count.load(Relaxed)), - executor: entry.executor.clone(), - }, - ); - } - } - Self { + let entry = CachedExecutorsEntry { + prev_epoch_count: entry.epoch_count.load(Relaxed), + epoch_count: AtomicU64::default(), + executor: entry.executor.clone(), + }; + (key, entry) + }); + Arc::new(Self { max: self.max, current_epoch: epoch, - executors, - } + executors: executors.collect(), + }) } fn new(max: usize, current_epoch: Epoch) -> Self { @@ -1082,7 +1035,9 @@ pub struct Bank { pub rewards_pool_pubkeys: Arc>, /// Cached executors - cached_executors: RwLock, + // Inner Arc is meant to implement copy-on-write semantics as opposed to + // sharing mutations (hence RwLock> instead of Arc>). + cached_executors: RwLock>, transaction_debug_keys: Option>>, @@ -1355,9 +1310,10 @@ impl Bank { cluster_type: parent.cluster_type, lazy_rent_collection: AtomicBool::new(parent.lazy_rent_collection.load(Relaxed)), rewards_pool_pubkeys: parent.rewards_pool_pubkeys.clone(), - cached_executors: RwLock::new( - (*parent.cached_executors.read().unwrap()).clone_with_epoch(epoch), - ), + cached_executors: { + let cached_executors = parent.cached_executors.read().unwrap(); + RwLock::new(cached_executors.clone_with_epoch(epoch)) + }, transaction_debug_keys: parent.transaction_debug_keys.clone(), transaction_log_collector_config: parent.transaction_log_collector_config.clone(), transaction_log_collector: Arc::new(RwLock::new(TransactionLogCollector::default())), @@ -1529,9 +1485,10 @@ impl Bank { cluster_type: Some(genesis_config.cluster_type), lazy_rent_collection: new(), rewards_pool_pubkeys: new(), - cached_executors: RwLock::new(CowCachedExecutors::new(Arc::new(RwLock::new( - CachedExecutors::new(MAX_CACHED_EXECUTORS, fields.epoch), - )))), + cached_executors: RwLock::new(Arc::new(CachedExecutors::new( + MAX_CACHED_EXECUTORS, + fields.epoch, + ))), transaction_debug_keys: debug_keys, transaction_log_collector_config: new(), transaction_log_collector: new(), @@ -3331,8 +3288,7 @@ impl Bank { num_executors += instruction_loaders.len(); } let mut executors = HashMap::with_capacity(num_executors); - let cow_cache = self.cached_executors.read().unwrap(); - let cache = cow_cache.read().unwrap(); + let cache = self.cached_executors.read().unwrap(); for key in message.account_keys.iter() { if let Some(executor) = cache.get(key) { @@ -3365,8 +3321,8 @@ impl Bank { .collect(); if !dirty_executors.is_empty() { - let mut cow_cache = self.cached_executors.write().unwrap(); - let mut cache = cow_cache.write().unwrap(); + let mut cache = self.cached_executors.write().unwrap(); + let cache = Arc::make_mut(&mut cache); for (key, executor) in dirty_executors.into_iter() { cache.put(key, executor); } @@ -3374,10 +3330,9 @@ impl Bank { } /// Remove an executor from the bank's cache - pub fn remove_executor(&self, pubkey: &Pubkey) { - let mut cow_cache = self.cached_executors.write().unwrap(); - let mut cache = cow_cache.write().unwrap(); - cache.remove(pubkey); + fn remove_executor(&self, pubkey: &Pubkey) { + let mut cache = self.cached_executors.write().unwrap(); + Arc::make_mut(&mut cache).remove(pubkey); } #[allow(clippy::type_complexity)] @@ -11910,26 +11865,26 @@ pub(crate) mod tests { assert!(cache.get(&key1).is_some()); assert!(cache.get(&key1).is_some()); - cache = cache.clone_with_epoch(1); + let mut cache = Arc::new(cache).clone_with_epoch(1); assert!(cache.current_epoch == 1); assert!(cache.get(&key2).is_some()); assert!(cache.get(&key2).is_some()); assert!(cache.get(&key3).is_some()); - cache.put(&key4, executor.clone()); + Arc::make_mut(&mut cache).put(&key4, executor.clone()); assert!(cache.get(&key4).is_some()); assert!(cache.get(&key3).is_none()); - cache.put(&key1, executor.clone()); - cache.put(&key3, executor.clone()); + Arc::make_mut(&mut cache).put(&key1, executor.clone()); + Arc::make_mut(&mut cache).put(&key3, executor.clone()); assert!(cache.get(&key1).is_some()); assert!(cache.get(&key4).is_none()); cache = cache.clone_with_epoch(2); assert!(cache.current_epoch == 2); - cache.put(&key3, executor.clone()); + Arc::make_mut(&mut cache).put(&key3, executor.clone()); assert!(cache.get(&key3).is_some()); }