diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 7a8fc4d4ce..75c7dddf1b 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -51,7 +51,9 @@ use { inline_spl_token, instruction_recorder::InstructionRecorder, log_collector::LogCollector, - message_processor::{ExecuteDetailsTimings, Executors, MessageProcessor}, + message_processor::{ + ExecuteDetailsTimings, Executors, MessageProcessor, TransactionExecutor, + }, rent_collector::RentCollector, stake_weighted_timestamp::{ calculate_stake_weighted_timestamp, MaxAllowableDrift, MAX_ALLOWABLE_DRIFT_PERCENTAGE, @@ -3328,31 +3330,39 @@ impl Bank { for key in message.account_keys.iter() { if let Some(executor) = cache.get(key) { - executors.insert(*key, executor); + executors.insert(*key, TransactionExecutor::cached(executor)); } } for instruction_loaders in loaders.iter() { for (key, _) in instruction_loaders.iter() { if let Some(executor) = cache.get(key) { - executors.insert(*key, executor); + executors.insert(*key, TransactionExecutor::cached(executor)); } } } - Rc::new(RefCell::new(Executors { - executors, - is_dirty: false, - })) + Rc::new(RefCell::new(executors)) } /// Add executors back to the bank's cache if modified fn update_executors(&self, executors: Rc>) { let executors = executors.borrow(); - if executors.is_dirty { + let dirty_executors: Vec<_> = executors + .iter() + .filter_map(|(key, TransactionExecutor { executor, is_dirty })| { + if *is_dirty { + Some((key, executor.clone())) + } else { + None + } + }) + .collect(); + + if !dirty_executors.is_empty() { let mut cow_cache = self.cached_executors.write().unwrap(); let mut cache = cow_cache.write().unwrap(); - for (key, executor) in executors.executors.iter() { - cache.put(key, (*executor).clone()); + for (key, executor) in dirty_executors.into_iter() { + cache.put(key, executor); } } } @@ -11948,50 +11958,50 @@ pub(crate) mod tests { // don't do any work if not dirty let mut executors = Executors::default(); - executors.insert(key1, executor.clone()); - executors.insert(key2, executor.clone()); - executors.insert(key3, executor.clone()); - executors.insert(key4, executor.clone()); + executors.insert(key1, TransactionExecutor::cached(executor.clone())); + executors.insert(key2, TransactionExecutor::cached(executor.clone())); + executors.insert(key3, TransactionExecutor::cached(executor.clone())); + executors.insert(key4, TransactionExecutor::cached(executor.clone())); let executors = Rc::new(RefCell::new(executors)); - executors.borrow_mut().is_dirty = false; + executors.borrow_mut().get_mut(&key1).unwrap().is_dirty = false; bank.update_executors(executors); let executors = bank.get_executors(&message, loaders); - assert_eq!(executors.borrow().executors.len(), 0); + assert_eq!(executors.borrow().len(), 0); // do work let mut executors = Executors::default(); - executors.insert(key1, executor.clone()); - executors.insert(key2, executor.clone()); - executors.insert(key3, executor.clone()); - executors.insert(key4, executor.clone()); + executors.insert(key1, TransactionExecutor::dirty(executor.clone())); + executors.insert(key2, TransactionExecutor::dirty(executor.clone())); + executors.insert(key3, TransactionExecutor::dirty(executor.clone())); + executors.insert(key4, TransactionExecutor::dirty(executor.clone())); let executors = Rc::new(RefCell::new(executors)); bank.update_executors(executors); let executors = bank.get_executors(&message, loaders); - assert_eq!(executors.borrow().executors.len(), 4); - assert!(executors.borrow().executors.contains_key(&key1)); - assert!(executors.borrow().executors.contains_key(&key2)); - assert!(executors.borrow().executors.contains_key(&key3)); - assert!(executors.borrow().executors.contains_key(&key4)); + assert_eq!(executors.borrow().len(), 4); + assert!(executors.borrow().contains_key(&key1)); + assert!(executors.borrow().contains_key(&key2)); + assert!(executors.borrow().contains_key(&key3)); + assert!(executors.borrow().contains_key(&key4)); // Check inheritance let bank = Bank::new_from_parent(&Arc::new(bank), &solana_sdk::pubkey::new_rand(), 1); let executors = bank.get_executors(&message, loaders); - assert_eq!(executors.borrow().executors.len(), 4); - assert!(executors.borrow().executors.contains_key(&key1)); - assert!(executors.borrow().executors.contains_key(&key2)); - assert!(executors.borrow().executors.contains_key(&key3)); - assert!(executors.borrow().executors.contains_key(&key4)); + assert_eq!(executors.borrow().len(), 4); + assert!(executors.borrow().contains_key(&key1)); + assert!(executors.borrow().contains_key(&key2)); + assert!(executors.borrow().contains_key(&key3)); + assert!(executors.borrow().contains_key(&key4)); bank.remove_executor(&key1); bank.remove_executor(&key2); bank.remove_executor(&key3); bank.remove_executor(&key4); let executors = bank.get_executors(&message, loaders); - assert_eq!(executors.borrow().executors.len(), 0); - assert!(!executors.borrow().executors.contains_key(&key1)); - assert!(!executors.borrow().executors.contains_key(&key2)); - assert!(!executors.borrow().executors.contains_key(&key3)); - assert!(!executors.borrow().executors.contains_key(&key4)); + assert_eq!(executors.borrow().len(), 0); + assert!(!executors.borrow().contains_key(&key1)); + assert!(!executors.borrow().contains_key(&key2)); + assert!(!executors.borrow().contains_key(&key3)); + assert!(!executors.borrow().contains_key(&key4)); } #[test] @@ -12012,36 +12022,36 @@ pub(crate) mod tests { // add one to root bank let mut executors = Executors::default(); - executors.insert(key1, executor.clone()); + executors.insert(key1, TransactionExecutor::dirty(executor.clone())); let executors = Rc::new(RefCell::new(executors)); root.update_executors(executors); let executors = root.get_executors(&Message::default(), loaders); - assert_eq!(executors.borrow().executors.len(), 1); + assert_eq!(executors.borrow().len(), 1); let fork1 = Bank::new_from_parent(&root, &Pubkey::default(), 1); let fork2 = Bank::new_from_parent(&root, &Pubkey::default(), 1); let executors = fork1.get_executors(&Message::default(), loaders); - assert_eq!(executors.borrow().executors.len(), 1); + assert_eq!(executors.borrow().len(), 1); let executors = fork2.get_executors(&Message::default(), loaders); - assert_eq!(executors.borrow().executors.len(), 1); + assert_eq!(executors.borrow().len(), 1); let mut executors = Executors::default(); - executors.insert(key2, executor.clone()); + executors.insert(key2, TransactionExecutor::dirty(executor.clone())); let executors = Rc::new(RefCell::new(executors)); fork1.update_executors(executors); let executors = fork1.get_executors(&Message::default(), loaders); - assert_eq!(executors.borrow().executors.len(), 2); + assert_eq!(executors.borrow().len(), 2); let executors = fork2.get_executors(&Message::default(), loaders); - assert_eq!(executors.borrow().executors.len(), 1); + assert_eq!(executors.borrow().len(), 1); fork1.remove_executor(&key1); let executors = fork1.get_executors(&Message::default(), loaders); - assert_eq!(executors.borrow().executors.len(), 1); + assert_eq!(executors.borrow().len(), 1); let executors = fork2.get_executors(&Message::default(), loaders); - assert_eq!(executors.borrow().executors.len(), 1); + assert_eq!(executors.borrow().len(), 1); } #[test] diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index 51a21597ff..fec303d7c1 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -39,25 +39,32 @@ use { }, }; -pub struct Executors { - pub executors: HashMap>, +pub type Executors = HashMap; + +/// Tracks whether a given executor is "dirty" and needs to +/// updated in the executors cache +pub struct TransactionExecutor { + pub executor: Arc, pub is_dirty: bool, } -impl Default for Executors { - fn default() -> Self { + +impl TransactionExecutor { + /// Wraps an executor and tracks that it doesn't need + /// to be updated in the executors cache. + pub fn cached(executor: Arc) -> Self { Self { - executors: HashMap::default(), + executor, is_dirty: false, } } -} -impl Executors { - pub fn insert(&mut self, key: Pubkey, executor: Arc) { - let _ = self.executors.insert(key, executor); - self.is_dirty = true; - } - pub fn get(&self, key: &Pubkey) -> Option> { - self.executors.get(key).cloned() + + /// Wraps an executor and tracks that it needs to be + /// updated in the executors cache. + pub fn dirty(executor: Arc) -> Self { + Self { + executor, + is_dirty: true, + } } } @@ -531,10 +538,15 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> { self.compute_meter.clone() } fn add_executor(&self, pubkey: &Pubkey, executor: Arc) { - self.executors.borrow_mut().insert(*pubkey, executor); + self.executors + .borrow_mut() + .insert(*pubkey, TransactionExecutor::dirty(executor)); } fn get_executor(&self, pubkey: &Pubkey) -> Option> { - self.executors.borrow().get(pubkey) + self.executors + .borrow() + .get(pubkey) + .map(|tx_executor| tx_executor.executor.clone()) } fn record_instruction(&self, instruction: &Instruction) { if let Some(recorder) = &self.instruction_recorder {