Replace executor cache usage with LoadedPrograms cache

ledger-tool/src/run.rs

@@ -15,7 +15,9 @@ use {
     },
     solana_program_runtime::{
         invoke_context::InvokeContext,
-        loaded_programs::{LoadProgramMetrics, LoadedProgram, LoadedProgramType},
+        loaded_programs::{
+            LoadProgramMetrics, LoadedProgram, LoadedProgramType, DELAY_VISIBILITY_SLOT_OFFSET,
+        },
         with_mock_invoke_context,
     },
     solana_rbpf::{
@@ -416,6 +418,9 @@ pub fn run(ledger_path: &Path, matches: &ArgMatches<'_>) {
         bank.get_builtin_programs()
     );
 
+    // Adding `DELAY_VISIBILITY_SLOT_OFFSET` to slots to accommodate for delay visibility of the program
+    let mut loaded_programs =
+        LoadedProgramsForTxBatch::new(bank.slot() + DELAY_VISIBILITY_SLOT_OFFSET);
     for key in cached_account_keys {
         let program = bank.load_program(&key, true).unwrap_or_else(|err| {
             // Create a tombstone for the program in the cache
@@ -426,11 +431,9 @@ pub fn run(ledger_path: &Path, matches: &ArgMatches<'_>) {
             ))
         });
         debug!("Loaded program {}", key);
-        invoke_context
-            .tx_executor_cache
-            .borrow_mut()
-            .set(key, program, false, false, 0);
+        loaded_programs.replenish(key, program);
     }
+    invoke_context.programs_loaded_for_tx_batch = Rc::new(RefCell::new(loaded_programs));
 
     invoke_context
         .transaction_context

program-runtime/src/invoke_context.rs

@@ -3,9 +3,8 @@ use {
         accounts_data_meter::AccountsDataMeter,
         builtin_program::{BuiltinPrograms, ProcessInstructionWithContext},
         compute_budget::ComputeBudget,
-        executor_cache::TransactionExecutorCache,
         ic_logger_msg, ic_msg,
-        loaded_programs::LoadedProgramType,
+        loaded_programs::{LoadedProgramType, LoadedProgramsForTxBatch},
         log_collector::LogCollector,
         pre_account::PreAccount,
         stable_log,
@@ -160,7 +159,9 @@ pub struct InvokeContext<'a> {
     current_compute_budget: ComputeBudget,
     compute_meter: RefCell<u64>,
     accounts_data_meter: AccountsDataMeter,
-    pub tx_executor_cache: Rc<RefCell<TransactionExecutorCache>>,
+    pub programs_loaded_for_tx_batch: Rc<RefCell<LoadedProgramsForTxBatch>>,
+    pub programs_modified_by_tx: Rc<RefCell<LoadedProgramsForTxBatch>>,
+    pub programs_updated_only_for_global_cache: Rc<RefCell<LoadedProgramsForTxBatch>>,
     pub feature_set: Arc<FeatureSet>,
     pub timings: ExecuteDetailsTimings,
     pub blockhash: Hash,
@@ -178,7 +179,9 @@ impl<'a> InvokeContext<'a> {
         sysvar_cache: &'a SysvarCache,
         log_collector: Option<Rc<RefCell<LogCollector>>>,
         compute_budget: ComputeBudget,
-        tx_executor_cache: Rc<RefCell<TransactionExecutorCache>>,
+        programs_loaded_for_tx_batch: Rc<RefCell<LoadedProgramsForTxBatch>>,
+        programs_modified_by_tx: Rc<RefCell<LoadedProgramsForTxBatch>>,
+        programs_updated_only_for_global_cache: Rc<RefCell<LoadedProgramsForTxBatch>>,
         feature_set: Arc<FeatureSet>,
         blockhash: Hash,
         lamports_per_signature: u64,
@@ -195,7 +198,9 @@ impl<'a> InvokeContext<'a> {
             compute_budget,
             compute_meter: RefCell::new(compute_budget.compute_unit_limit),
             accounts_data_meter: AccountsDataMeter::new(prev_accounts_data_len),
-            tx_executor_cache,
+            programs_loaded_for_tx_batch,
+            programs_modified_by_tx,
+            programs_updated_only_for_global_cache,
             feature_set,
             timings: ExecuteDetailsTimings::default(),
             blockhash,
@@ -902,8 +907,8 @@ macro_rules! with_mock_invoke_context_and_builtin_programs {
             },
             std::{cell::RefCell, rc::Rc, sync::Arc},
             $crate::{
-                compute_budget::ComputeBudget, executor_cache::TransactionExecutorCache,
-                invoke_context::InvokeContext, log_collector::LogCollector,
+                compute_budget::ComputeBudget, invoke_context::InvokeContext,
+                loaded_programs::LoadedProgramsForTxBatch, log_collector::LogCollector,
                 sysvar_cache::SysvarCache,
             },
         };
@@ -940,7 +945,9 @@ macro_rules! with_mock_invoke_context_and_builtin_programs {
             &sysvar_cache,
             Some(LogCollector::new_ref()),
             compute_budget,
-            Rc::new(RefCell::new(TransactionExecutorCache::default())),
+            Rc::new(RefCell::new(LoadedProgramsForTxBatch::default())),
+            Rc::new(RefCell::new(LoadedProgramsForTxBatch::default())),
+            Rc::new(RefCell::new(LoadedProgramsForTxBatch::default())),
             Arc::new(FeatureSet::all_enabled()),
             Hash::default(),
             0,

program-runtime/src/loaded_programs.rs

@@ -281,17 +281,23 @@ pub struct LoadedProgramsForTxBatch {
 }
 
 impl LoadedProgramsForTxBatch {
+    pub fn new(slot: Slot) -> Self {
+        Self {
+            entries: HashMap::new(),
+            slot,
+        }
+    }
+
     /// Refill the cache with a single entry. It's typically called during transaction loading, and
     /// transaction processing (for program management instructions).
-    /// The replaces the existing entry (if any) with the provided entry. The return value contains
+    /// It replaces the existing entry (if any) with the provided entry. The return value contains
     /// `true` if an entry existed.
     /// The function also returns the newly inserted value.
     pub fn replenish(
         &mut self,
         key: Pubkey,
         entry: Arc<LoadedProgram>,
     ) -> (bool, Arc<LoadedProgram>) {
-        debug_assert!(entry.effective_slot <= self.slot);
         (self.entries.insert(key, entry.clone()).is_some(), entry)
     }
 
@@ -314,6 +320,16 @@ impl LoadedProgramsForTxBatch {
     pub fn slot(&self) -> Slot {
         self.slot
     }
+
+    pub fn set_slot_for_tests(&mut self, slot: Slot) {
+        self.slot = slot;
+    }
+
+    pub fn merge(&mut self, other: &Self) {
+        other.entries.iter().for_each(|(key, entry)| {
+            self.replenish(*key, entry.clone());
+        })
+    }
 }
 
 pub enum LoadedProgramMatchCriteria {
@@ -453,6 +469,12 @@ impl LoadedPrograms {
                                 return None;
                             }
 
+                            if matches!(entry.program, LoadedProgramType::Unloaded) {
+                                // The program was unloaded. Consider it as missing, so it can be reloaded.
+                                missing.push(key);
+                                return None;
+                            }
+
                             if current_slot >= entry.effective_slot {
                                 return Some((key, entry.clone()));
                             } else if entry.is_implicit_delay_visibility_tombstone(current_slot) {
@@ -483,6 +505,12 @@ impl LoadedPrograms {
         )
     }
 
+    pub fn merge(&mut self, tx_batch_cache: &LoadedProgramsForTxBatch) {
+        tx_batch_cache.entries.iter().for_each(|(key, entry)| {
+            self.replenish(*key, entry.clone());
+        })
+    }
+
     /// Unloads programs which were used infrequently
     pub fn sort_and_unload(&mut self, shrink_to: PercentageInteger) {
         let sorted_candidates: Vec<(Pubkey, Arc<LoadedProgram>)> = self

programs/bpf_loader/src/lib.rs

@@ -10,7 +10,9 @@ use {
         compute_budget::ComputeBudget,
         ic_logger_msg, ic_msg,
         invoke_context::{BpfAllocator, InvokeContext, SyscallContext},
-        loaded_programs::{LoadProgramMetrics, LoadedProgram, LoadedProgramType},
+        loaded_programs::{
+            LoadProgramMetrics, LoadedProgram, LoadedProgramType, DELAY_VISIBILITY_SLOT_OFFSET,
+        },
         log_collector::LogCollector,
         stable_log,
         sysvar_cache::get_sysvar_with_account_check,
@@ -91,7 +93,7 @@ pub fn load_program_from_bytes(
     register_syscalls_time.stop();
     load_program_metrics.register_syscalls_us = register_syscalls_time.as_us();
     let effective_slot = if feature_set.is_active(&delay_visibility_of_program_deployment::id()) {
-        deployment_slot.saturating_add(1)
+        deployment_slot.saturating_add(DELAY_VISIBILITY_SLOT_OFFSET)
     } else {
         deployment_slot
     };
@@ -190,12 +192,27 @@ pub fn load_program_from_account(
     Ok((loaded_program, Some(load_program_metrics)))
 }
 
+fn find_program_in_cache(
+    invoke_context: &InvokeContext,
+    pubkey: &Pubkey,
+) -> Option<Arc<LoadedProgram>> {
+    // First lookup the cache of the programs modified by the current transaction. If not found, lookup
+    // the cache of the cache of the programs that are loaded for the transaction batch.
+    invoke_context
+        .programs_modified_by_tx
+        .borrow()
+        .find(pubkey)
+        .or_else(|| {
+            invoke_context
+                .programs_loaded_for_tx_batch
+                .borrow()
+                .find(pubkey)
+        })
+}
+
 macro_rules! deploy_program {
     ($invoke_context:expr, $program_id:expr, $loader_key:expr,
      $account_size:expr, $slot:expr, $drop:expr, $new_programdata:expr $(,)?) => {{
-        let delay_visibility_of_program_deployment = $invoke_context
-            .feature_set
-            .is_active(&delay_visibility_of_program_deployment::id());
         let mut load_program_metrics = LoadProgramMetrics::default();
         let executor = load_program_from_bytes(
             &$invoke_context.feature_set,
@@ -209,20 +226,14 @@ macro_rules! deploy_program {
             true, /* reject_deployment_of_broken_elfs */
             false, /* debugging_features */
         )?;
-        if let Some(old_entry) = $invoke_context.tx_executor_cache.borrow().get(&$program_id) {
+        if let Some(old_entry) = find_program_in_cache($invoke_context, &$program_id) {
             let usage_counter = old_entry.usage_counter.load(Ordering::Relaxed);
             executor.usage_counter.store(usage_counter, Ordering::Relaxed);
         }
         $drop
         load_program_metrics.program_id = $program_id.to_string();
         load_program_metrics.submit_datapoint(&mut $invoke_context.timings);
-        $invoke_context.tx_executor_cache.borrow_mut().set(
-            $program_id,
-            Arc::new(executor),
-            true,
-            delay_visibility_of_program_deployment,
-            $slot,
-        );
+        $invoke_context.programs_modified_by_tx.borrow_mut().replenish($program_id, Arc::new(executor));
     }};
 }
 
@@ -562,10 +573,7 @@ fn process_instruction_inner(
     }
 
     let mut get_or_create_executor_time = Measure::start("get_or_create_executor_time");
-    let executor = invoke_context
-        .tx_executor_cache
-        .borrow()
-        .get(program_account.get_key())
+    let executor = find_program_in_cache(invoke_context, program_account.get_key())
         .ok_or(InstructionError::InvalidAccountData)?;
 
     if executor.is_tombstone() {
@@ -1265,15 +1273,32 @@ fn process_loader_upgradeable_instruction(
                                 instruction_context,
                                 &log_collector,
                             )?;
+                            let clock = invoke_context.get_sysvar_cache().get_clock()?;
                             if invoke_context
                                 .feature_set
                                 .is_active(&delay_visibility_of_program_deployment::id())
                             {
-                                let clock = invoke_context.get_sysvar_cache().get_clock()?;
                                 invoke_context
-                                    .tx_executor_cache
+                                    .programs_modified_by_tx
                                     .borrow_mut()
-                                    .set_tombstone(program_key, clock.slot);
+                                    .replenish(
+                                        program_key,
+                                        Arc::new(LoadedProgram::new_tombstone(
+                                            clock.slot,
+                                            LoadedProgramType::Closed,
+                                        )),
+                                    );
+                            } else {
+                                invoke_context
+                                    .programs_updated_only_for_global_cache
+                                    .borrow_mut()
+                                    .replenish(
+                                        program_key,
+                                        Arc::new(LoadedProgram::new_tombstone(
+                                            clock.slot,
+                                            LoadedProgramType::Closed,
+                                        )),
+                                    );
                             }
                         }
                         _ => {
@@ -1661,7 +1686,10 @@ fn execute<'a, 'b: 'a>(
 }
 
 pub mod test_utils {
-    use {super::*, solana_sdk::account::ReadableAccount};
+    use {
+        super::*, solana_program_runtime::loaded_programs::DELAY_VISIBILITY_SLOT_OFFSET,
+        solana_sdk::account::ReadableAccount,
+    };
 
     pub fn load_all_invoked_programs(invoke_context: &mut InvokeContext) {
         let num_accounts = invoke_context.transaction_context.get_number_of_accounts();
@@ -1693,8 +1721,9 @@ pub mod test_utils {
                     true,
                     false,
                 ) {
-                    let mut cache = invoke_context.tx_executor_cache.borrow_mut();
-                    cache.set(*pubkey, Arc::new(loaded_program), true, false, 0)
+                    let mut cache = invoke_context.programs_modified_by_tx.borrow_mut();
+                    cache.set_slot_for_tests(DELAY_VISIBILITY_SLOT_OFFSET);
+                    cache.replenish(*pubkey, Arc::new(loaded_program));
                 }
             }
         }
@@ -4096,23 +4125,20 @@ mod tests {
             maybe_expiration_slot: None,
             usage_counter: AtomicU64::new(100),
         };
-        invoke_context.tx_executor_cache.borrow_mut().set(
-            program_id,
-            Arc::new(program),
-            false,
-            false,
-            0,
-        );
+        invoke_context
+            .programs_modified_by_tx
+            .borrow_mut()
+            .replenish(program_id, Arc::new(program));
 
         assert!(matches!(
             deploy_test_program(&mut invoke_context, program_id,),
             Ok(())
         ));
 
         let updated_program = invoke_context
-            .tx_executor_cache
+            .programs_modified_by_tx
             .borrow()
-            .get(&program_id)
+            .find(&program_id)
             .expect("Didn't find upgraded program in the cache");
 
         assert_eq!(updated_program.deployment_slot, 2);
@@ -4132,13 +4158,10 @@ mod tests {
             maybe_expiration_slot: None,
             usage_counter: AtomicU64::new(100),
         };
-        invoke_context.tx_executor_cache.borrow_mut().set(
-            program_id,
-            Arc::new(program),
-            false,
-            false,
-            0,
-        );
+        invoke_context
+            .programs_modified_by_tx
+            .borrow_mut()
+            .replenish(program_id, Arc::new(program));
 
         let program_id2 = Pubkey::new_unique();
         assert!(matches!(
@@ -4147,9 +4170,9 @@ mod tests {
         ));
 
         let program2 = invoke_context
-            .tx_executor_cache
+            .programs_modified_by_tx
             .borrow()
-            .get(&program_id2)
+            .find(&program_id2)
             .expect("Didn't find upgraded program in the cache");
 
         assert_eq!(program2.deployment_slot, 2);