diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index cd0ec977d7..526cc3ba14 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -65,31 +65,56 @@ pub trait Executor: Debug + Send + Sync { pub type Executors = HashMap; -/// Tracks whether a given executor is "dirty" and needs to -/// updated in the executors cache +/// 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, + 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 cached(executor: Arc) -> Self { + /// 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_dirty: false, + is_miss: false, + is_updated: false, } } - /// Wraps an executor and tracks that it needs to be - /// updated in the executors cache. - pub fn dirty(executor: Arc) -> Self { + /// 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_dirty: true, + is_miss: true, + is_updated: false, } } + + /// 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; + } } /// Compute meter @@ -894,12 +919,18 @@ 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) { self.executors .borrow_mut() - .insert(*pubkey, TransactionExecutor::dirty(executor)); + .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)); } /// 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 118f71657f..c6cbcc120f 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 fe4c8aaf4e..d0efd6f4b2 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -3516,14 +3516,14 @@ impl Bank { for key in message.account_keys_iter() { if let Some(executor) = cache.get(key) { - executors.insert(*key, TransactionExecutor::cached(executor)); + executors.insert(*key, TransactionExecutor::new_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, TransactionExecutor::cached(executor)); + executors.insert(key, TransactionExecutor::new_cached(executor)); } } } @@ -3532,13 +3532,13 @@ 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(); let dirty_executors: Vec<_> = executors .iter() - .filter_map(|(key, TransactionExecutor { executor, is_dirty })| { - if *is_dirty { - Some((key, executor.clone())) + .filter_map(|(key, executor)| { + if executor.is_dirty(allow_updates) { + Some((key, executor.get())) } else { None } @@ -3630,6 +3630,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) @@ -3643,16 +3654,6 @@ impl Bank { loaded_transaction.accounts = transaction_context.deconstruct(); - 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, @@ -12826,24 +12827,28 @@ pub(crate) mod tests { // don't do any work if not dirty let mut executors = Executors::default(); - 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())); + 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().is_dirty = false; - bank.update_executors(executors); + executors + .borrow_mut() + .get_mut(&key1) + .unwrap() + .clear_miss_for_test(); + bank.update_executors(true, executors); let executors = bank.get_executors(&message, accounts, program_indices); assert_eq!(executors.borrow().len(), 0); // do work let mut executors = Executors::default(); - 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())); + 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())); 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().len(), 4); assert!(executors.borrow().contains_key(&key1)); @@ -12893,9 +12898,9 @@ pub(crate) mod tests { // add one to root bank let mut executors = Executors::default(); - executors.insert(key1, TransactionExecutor::dirty(executor.clone())); + executors.insert(key1, TransactionExecutor::new_miss(executor.clone())); 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().len(), 1); @@ -12908,9 +12913,9 @@ pub(crate) mod tests { assert_eq!(executors.borrow().len(), 1); let mut executors = Executors::default(); - executors.insert(key2, TransactionExecutor::dirty(executor.clone())); + executors.insert(key2, TransactionExecutor::new_miss(executor.clone())); 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().len(), 2);