diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index 59715cdd6a..cd0ec977d7 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -63,18 +63,32 @@ pub trait Executor: Debug + Send + Sync { ) -> Result<(), InstructionError>; } -#[derive(Default)] -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 Executors { - pub fn insert(&mut self, key: Pubkey, executor: Arc) { - let _ = self.executors.insert(key, executor); - self.is_dirty = true; + +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 { + executor, + is_dirty: false, + } } - 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, + } } } @@ -883,12 +897,17 @@ impl<'a> InvokeContext<'a> { /// Loaders may need to do work in order to execute a program. Cache /// the work that can be re-used across executions pub fn add_executor(&self, pubkey: &Pubkey, executor: Arc) { - self.executors.borrow_mut().insert(*pubkey, executor); + self.executors + .borrow_mut() + .insert(*pubkey, TransactionExecutor::dirty(executor)); } /// Get the completed loader work that can be re-used across execution pub fn get_executor(&self, pubkey: &Pubkey) -> Option> { - self.executors.borrow().get(pubkey) + self.executors + .borrow() + .get(pubkey) + .map(|tx_executor| tx_executor.executor.clone()) } /// Get this invocation's compute budget diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index cbaac5b5ce..fe4c8aaf4e 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -74,7 +74,9 @@ use { solana_metrics::{inc_new_counter_debug, inc_new_counter_info}, solana_program_runtime::{ instruction_recorder::InstructionRecorder, - invoke_context::{BuiltinProgram, Executor, Executors, ProcessInstructionWithContext}, + invoke_context::{ + BuiltinProgram, Executor, Executors, ProcessInstructionWithContext, TransactionExecutor, + }, log_collector::LogCollector, timings::ExecuteDetailsTimings, }, @@ -3514,32 +3516,40 @@ 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 program_indices_of_instruction in program_indices.iter() { for account_index in program_indices_of_instruction.iter() { let key = accounts[*account_index].0; 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); } } } @@ -12816,50 +12826,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, accounts, program_indices); - 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, accounts, program_indices); - 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, accounts, program_indices); - 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, accounts, program_indices); - 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] @@ -12883,36 +12893,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, accounts, program_indices); - 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, accounts, program_indices); - assert_eq!(executors.borrow().executors.len(), 1); + assert_eq!(executors.borrow().len(), 1); let executors = fork2.get_executors(&message, accounts, program_indices); - 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, accounts, program_indices); - assert_eq!(executors.borrow().executors.len(), 2); + assert_eq!(executors.borrow().len(), 2); let executors = fork2.get_executors(&message, accounts, program_indices); - assert_eq!(executors.borrow().executors.len(), 1); + assert_eq!(executors.borrow().len(), 1); fork1.remove_executor(&key1); let executors = fork1.get_executors(&message, accounts, program_indices); - assert_eq!(executors.borrow().executors.len(), 1); + assert_eq!(executors.borrow().len(), 1); let executors = fork2.get_executors(&message, accounts, program_indices); - assert_eq!(executors.borrow().executors.len(), 1); + assert_eq!(executors.borrow().len(), 1); } #[test]