perf: skip updating already cached executors if unmodified (backport #22300) (#22315)

* Skip updating already cached executors if unmodified

(cherry picked from commit f2ed6f09ee)

# Conflicts:
#	program-runtime/src/invoke_context.rs
#	runtime/src/bank.rs

* resolve conflicts

Co-authored-by: Justin Starry <justin@solana.com>

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 {
+        Rc::new(RefCell::new(executors))
-            executors,
-            is_dirty: false,
-        }))
     }
 
     /// Add executors back to the bank's cache if modified
     fn update_executors(&self, executors: Rc<RefCell<Executors>>) {
         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() {
+            for (key, executor) in dirty_executors.into_iter() {
-                cache.put(key, (*executor).clone());
+                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(key1, TransactionExecutor::cached(executor.clone()));
-        executors.insert(key2, executor.clone());
+        executors.insert(key2, TransactionExecutor::cached(executor.clone()));
-        executors.insert(key3, executor.clone());
+        executors.insert(key3, TransactionExecutor::cached(executor.clone()));
-        executors.insert(key4, 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(key1, TransactionExecutor::dirty(executor.clone()));
-        executors.insert(key2, executor.clone());
+        executors.insert(key2, TransactionExecutor::dirty(executor.clone()));
-        executors.insert(key3, executor.clone());
+        executors.insert(key3, TransactionExecutor::dirty(executor.clone()));
-        executors.insert(key4, 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_eq!(executors.borrow().len(), 4);
-        assert!(executors.borrow().executors.contains_key(&key1));
+        assert!(executors.borrow().contains_key(&key1));
-        assert!(executors.borrow().executors.contains_key(&key2));
+        assert!(executors.borrow().contains_key(&key2));
-        assert!(executors.borrow().executors.contains_key(&key3));
+        assert!(executors.borrow().contains_key(&key3));
-        assert!(executors.borrow().executors.contains_key(&key4));
+        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_eq!(executors.borrow().len(), 4);
-        assert!(executors.borrow().executors.contains_key(&key1));
+        assert!(executors.borrow().contains_key(&key1));
-        assert!(executors.borrow().executors.contains_key(&key2));
+        assert!(executors.borrow().contains_key(&key2));
-        assert!(executors.borrow().executors.contains_key(&key3));
+        assert!(executors.borrow().contains_key(&key3));
-        assert!(executors.borrow().executors.contains_key(&key4));
+        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_eq!(executors.borrow().len(), 0);
-        assert!(!executors.borrow().executors.contains_key(&key1));
+        assert!(!executors.borrow().contains_key(&key1));
-        assert!(!executors.borrow().executors.contains_key(&key2));
+        assert!(!executors.borrow().contains_key(&key2));
-        assert!(!executors.borrow().executors.contains_key(&key3));
+        assert!(!executors.borrow().contains_key(&key3));
-        assert!(!executors.borrow().executors.contains_key(&key4));
+        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]

runtime/src/message_processor.rs

@@ -39,25 +39,32 @@ use {
     },
 };
 
-pub struct Executors {
+pub type Executors = HashMap<Pubkey, TransactionExecutor>;
-    pub executors: HashMap<Pubkey, Arc<dyn Executor>>,
+
+/// Tracks whether a given executor is "dirty" and needs to
+/// updated in the executors cache
+pub struct TransactionExecutor {
+    pub executor: Arc<dyn Executor>,
     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<dyn Executor>) -> Self {
         Self {
-            executors: HashMap::default(),
+            executor,
             is_dirty: false,
         }
     }
-}
+
-impl Executors {
+    /// Wraps an executor and tracks that it needs to be
-    pub fn insert(&mut self, key: Pubkey, executor: Arc<dyn Executor>) {
+    /// updated in the executors cache.
-        let _ = self.executors.insert(key, executor);
+    pub fn dirty(executor: Arc<dyn Executor>) -> Self {
-        self.is_dirty = true;
+        Self {
+            executor,
+            is_dirty: true,
         }
-    pub fn get(&self, key: &Pubkey) -> Option<Arc<dyn Executor>> {
-        self.executors.get(key).cloned()
     }
 }
 
@@ -531,10 +538,15 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> {
         self.compute_meter.clone()
     }
     fn add_executor(&self, pubkey: &Pubkey, executor: Arc<dyn Executor>) {
-        self.executors.borrow_mut().insert(*pubkey, executor);
+        self.executors
+            .borrow_mut()
+            .insert(*pubkey, TransactionExecutor::dirty(executor));
     }
     fn get_executor(&self, pubkey: &Pubkey) -> Option<Arc<dyn Executor>> {
-        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 {