diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index a5598148816a91..3101149bb41185 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -71,6 +71,7 @@ pub trait Executor: Debug + Send + Sync { ) -> Result<(), InstructionError>; } +<<<<<<< HEAD #[derive(Default)] pub struct Executors { pub executors: HashMap>, @@ -83,6 +84,59 @@ impl Executors { } pub fn get(&self, key: &Pubkey) -> Option> { self.executors.get(key).cloned() +======= +pub type Executors = HashMap; + +/// Tracks whether a given executor is "dirty" and needs to updated in the +/// executors cache +pub struct TransactionExecutor { + executor: Arc, + is_miss: bool, + is_updated: bool, +} + +impl TransactionExecutor { + /// Wraps an executor and tracks that it doesn't need to be updated in the + /// executors cache. + pub fn new_cached(executor: Arc) -> Self { + Self { + executor, + is_miss: false, + is_updated: false, + } + } + + /// Wraps an executor and tracks that it needs to be updated in the + /// executors cache. + pub fn new_miss(executor: Arc) -> Self { + Self { + executor, + is_miss: true, + is_updated: false, + } +>>>>>>> 12e160269 (cache executors on failed transactions (#22308)) + } + + /// Wraps an executor and tracks that it needs to be updated in the + /// executors cache only if the transaction succeeded. + pub fn new_updated(executor: Arc) -> Self { + Self { + executor, + is_miss: false, + is_updated: true, + } + } + + pub fn is_dirty(&self, include_updates: bool) -> bool { + self.is_miss || (include_updates && self.is_updated) + } + + pub fn get(&self) -> Arc { + self.executor.clone() + } + + pub fn clear_miss_for_test(&mut self) { + self.is_miss = false; } } @@ -808,10 +862,22 @@ impl<'a> InvokeContext<'a> { &self.accounts_data_meter } - /// Loaders may need to do work in order to execute a program. Cache - /// the work that can be re-used across executions + /// Cache an executor that wasn't found in the cache pub fn add_executor(&self, pubkey: &Pubkey, executor: Arc) { +<<<<<<< HEAD self.executors.borrow_mut().insert(*pubkey, executor); +======= + self.executors + .borrow_mut() + .insert(*pubkey, TransactionExecutor::new_miss(executor)); + } + + /// Cache an executor that has changed + pub fn update_executor(&self, pubkey: &Pubkey, executor: Arc) { + self.executors + .borrow_mut() + .insert(*pubkey, TransactionExecutor::new_updated(executor)); +>>>>>>> 12e160269 (cache executors on failed transactions (#22308)) } /// Get the completed loader work that can be re-used across execution diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 5d4feb52f6b113..5bf484151cc0ef 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -510,7 +510,7 @@ fn process_loader_upgradeable_instruction( use_jit, true, )?; - invoke_context.add_executor(&new_program_id, executor); + invoke_context.update_executor(&new_program_id, executor); let keyed_accounts = invoke_context.get_keyed_accounts()?; let payer = keyed_account_at_index(keyed_accounts, first_instruction_account)?; @@ -658,7 +658,7 @@ fn process_loader_upgradeable_instruction( use_jit, true, )?; - invoke_context.add_executor(&new_program_id, executor); + invoke_context.update_executor(&new_program_id, executor); let keyed_accounts = invoke_context.get_keyed_accounts()?; let programdata = keyed_account_at_index(keyed_accounts, first_instruction_account)?; @@ -925,7 +925,7 @@ fn process_loader_instruction( create_executor(first_instruction_account, 0, invoke_context, use_jit, true)?; let keyed_accounts = invoke_context.get_keyed_accounts()?; let program = keyed_account_at_index(keyed_accounts, first_instruction_account)?; - invoke_context.add_executor(program.unsigned_key(), executor); + invoke_context.update_executor(program.unsigned_key(), executor); program.try_account_ref_mut()?.set_executable(true); ic_msg!( invoke_context, diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 375395c7213126..1b4b7be91b4966 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -3546,14 +3546,22 @@ impl Bank { for key in message.account_keys_iter() { if let Some(executor) = cache.get(key) { +<<<<<<< HEAD executors.insert(*key, executor); +======= + executors.insert(*key, TransactionExecutor::new_cached(executor)); +>>>>>>> 12e160269 (cache executors on failed transactions (#22308)) } } 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) { +<<<<<<< HEAD executors.insert(key, executor); +======= + executors.insert(key, TransactionExecutor::new_cached(executor)); +>>>>>>> 12e160269 (cache executors on failed transactions (#22308)) } } } @@ -3565,9 +3573,24 @@ impl Bank { } /// Add executors back to the bank's cache if modified - fn update_executors(&self, executors: Rc>) { + fn update_executors(&self, allow_updates: bool, executors: Rc>) { let executors = executors.borrow(); +<<<<<<< HEAD if executors.is_dirty { +======= + let dirty_executors: Vec<_> = executors + .iter() + .filter_map(|(key, executor)| { + if executor.is_dirty(allow_updates) { + Some((key, executor.get())) + } else { + None + } + }) + .collect(); + + if !dirty_executors.is_empty() { +>>>>>>> 12e160269 (cache executors on failed transactions (#22308)) let mut cow_cache = self.cached_executors.write().unwrap(); let mut cache = cow_cache.write().unwrap(); for (key, executor) in executors.executors.iter() { @@ -3648,6 +3671,17 @@ impl Bank { self.load_accounts_data_len(), ); + self.update_executors(process_result.is_ok(), executors); + + let status = process_result + .map(|info| { + self.store_accounts_data_len(info.accounts_data_len); + }) + .map_err(|err| { + error_counters.instruction_error += 1; + err + }); + let log_messages: Option = log_collector.and_then(|log_collector| { Rc::try_unwrap(log_collector) @@ -3670,16 +3704,6 @@ impl Bank { return TransactionExecutionResult::NotExecuted(e); } - let status = process_result - .map(|info| { - self.store_accounts_data_len(info.accounts_data_len); - self.update_executors(executors); - }) - .map_err(|err| { - error_counters.instruction_error += 1; - err - }); - TransactionExecutionResult::Executed(TransactionExecutionDetails { status, log_messages, @@ -12891,6 +12915,7 @@ pub(crate) mod tests { // don't do any work if not dirty let mut executors = Executors::default(); +<<<<<<< HEAD executors.insert(key1, executor.clone()); executors.insert(key2, executor.clone()); executors.insert(key3, executor.clone()); @@ -12898,17 +12923,37 @@ pub(crate) mod tests { let executors = Rc::new(RefCell::new(executors)); executors.borrow_mut().is_dirty = false; bank.update_executors(executors); +======= + executors.insert(key1, TransactionExecutor::new_cached(executor.clone())); + executors.insert(key2, TransactionExecutor::new_cached(executor.clone())); + executors.insert(key3, TransactionExecutor::new_cached(executor.clone())); + executors.insert(key4, TransactionExecutor::new_cached(executor.clone())); + let executors = Rc::new(RefCell::new(executors)); + executors + .borrow_mut() + .get_mut(&key1) + .unwrap() + .clear_miss_for_test(); + bank.update_executors(true, executors); +>>>>>>> 12e160269 (cache executors on failed transactions (#22308)) let executors = bank.get_executors(&message, accounts, program_indices); assert_eq!(executors.borrow().executors.len(), 0); // do work let mut executors = Executors::default(); +<<<<<<< HEAD executors.insert(key1, executor.clone()); executors.insert(key2, executor.clone()); executors.insert(key3, executor.clone()); executors.insert(key4, executor.clone()); +======= + executors.insert(key1, TransactionExecutor::new_miss(executor.clone())); + executors.insert(key2, TransactionExecutor::new_miss(executor.clone())); + executors.insert(key3, TransactionExecutor::new_miss(executor.clone())); + executors.insert(key4, TransactionExecutor::new_miss(executor.clone())); +>>>>>>> 12e160269 (cache executors on failed transactions (#22308)) let executors = Rc::new(RefCell::new(executors)); - bank.update_executors(executors); + bank.update_executors(true, executors); let executors = bank.get_executors(&message, accounts, program_indices); assert_eq!(executors.borrow().executors.len(), 4); assert!(executors.borrow().executors.contains_key(&key1)); @@ -12958,9 +13003,13 @@ pub(crate) mod tests { // add one to root bank let mut executors = Executors::default(); +<<<<<<< HEAD executors.insert(key1, executor.clone()); +======= + executors.insert(key1, TransactionExecutor::new_miss(executor.clone())); +>>>>>>> 12e160269 (cache executors on failed transactions (#22308)) let executors = Rc::new(RefCell::new(executors)); - root.update_executors(executors); + root.update_executors(true, executors); let executors = root.get_executors(&message, accounts, program_indices); assert_eq!(executors.borrow().executors.len(), 1); @@ -12973,9 +13022,13 @@ pub(crate) mod tests { assert_eq!(executors.borrow().executors.len(), 1); let mut executors = Executors::default(); +<<<<<<< HEAD executors.insert(key2, executor.clone()); +======= + executors.insert(key2, TransactionExecutor::new_miss(executor.clone())); +>>>>>>> 12e160269 (cache executors on failed transactions (#22308)) let executors = Rc::new(RefCell::new(executors)); - fork1.update_executors(executors); + fork1.update_executors(true, executors); let executors = fork1.get_executors(&message, accounts, program_indices); assert_eq!(executors.borrow().executors.len(), 2);