Remove executors trait and replace with LoadedProgram

program-runtime/src/executor_cache.rs

@@ -1,5 +1,5 @@
 use {
-    crate::executor::Executor,
+    crate::loaded_programs::LoadedProgram,
     log::*,
     rand::Rng,
     solana_sdk::{pubkey::Pubkey, saturating_add_assign, slot_history::Slot, stake_history::Epoch},
@@ -21,36 +21,35 @@ use {
 #[derive(Default, Debug)]
 pub struct TransactionExecutorCache {
     /// Executors or tombstones which are visible during the transaction
-    pub visible: HashMap<Pubkey, Option<Arc<dyn Executor>>>,
+    pub visible: HashMap<Pubkey, Arc<LoadedProgram>>,
     /// Executors of programs which were re-/deploymed during the transaction
-    pub deployments: HashMap<Pubkey, Arc<dyn Executor>>,
+    pub deployments: HashMap<Pubkey, Arc<LoadedProgram>>,
     /// Executors which were missing in the cache and not re-/deploymed during the transaction
-    pub add_to_cache: HashMap<Pubkey, Arc<dyn Executor>>,
+    pub add_to_cache: HashMap<Pubkey, Arc<LoadedProgram>>,
 }
 
 impl TransactionExecutorCache {
-    pub fn new(executable_keys: impl Iterator<Item = (Pubkey, Arc<dyn Executor>)>) -> Self {
+    pub fn new(executable_keys: impl Iterator<Item = (Pubkey, Arc<LoadedProgram>)>) -> Self {
         Self {
-            visible: executable_keys
-                .map(|(id, executor)| (id, Some(executor)))
-                .collect(),
+            visible: executable_keys.collect(),
             deployments: HashMap::new(),
             add_to_cache: HashMap::new(),
         }
     }
 
-    pub fn get(&self, key: &Pubkey) -> Option<Option<Arc<dyn Executor>>> {
+    pub fn get(&self, key: &Pubkey) -> Option<Arc<LoadedProgram>> {
         self.visible.get(key).cloned()
     }
 
     pub fn set_tombstone(&mut self, key: Pubkey) {
-        self.visible.insert(key, None);
+        self.visible
+            .insert(key, Arc::new(LoadedProgram::new_tombstone()));
     }
 
     pub fn set(
         &mut self,
         key: Pubkey,
-        executor: Arc<dyn Executor>,
+        executor: Arc<LoadedProgram>,
         upgrade: bool,
         delay_visibility_of_program_deployment: bool,
     ) {
@@ -60,22 +59,22 @@ impl TransactionExecutorCache {
                 // we don't load the new version from the database as it should remain invisible
                 self.set_tombstone(key);
             } else {
-                self.visible.insert(key, Some(executor.clone()));
+                self.visible.insert(key, executor.clone());
             }
             self.deployments.insert(key, executor);
         } else {
-            self.visible.insert(key, Some(executor.clone()));
+            self.visible.insert(key, executor.clone());
             self.add_to_cache.insert(key, executor);
         }
     }
 
-    pub fn get_executors_added_to_the_cache(&mut self) -> HashMap<Pubkey, Arc<dyn Executor>> {
+    pub fn get_executors_added_to_the_cache(&mut self) -> HashMap<Pubkey, Arc<LoadedProgram>> {
         let mut executors = HashMap::new();
         std::mem::swap(&mut executors, &mut self.add_to_cache);
         executors
     }
 
-    pub fn get_executors_which_were_deployed(&mut self) -> HashMap<Pubkey, Arc<dyn Executor>> {
+    pub fn get_executors_which_were_deployed(&mut self) -> HashMap<Pubkey, Arc<LoadedProgram>> {
         let mut executors = HashMap::new();
         std::mem::swap(&mut executors, &mut self.deployments);
         executors
@@ -90,7 +89,7 @@ pub const MAX_CACHED_EXECUTORS: usize = 256;
 pub struct BankExecutorCacheEntry {
     prev_epoch_count: u64,
     epoch_count: AtomicU64,
-    executor: Arc<dyn Executor>,
+    executor: Arc<LoadedProgram>,
     pub hit_count: AtomicU64,
 }
 
@@ -175,7 +174,7 @@ impl BankExecutorCache {
         }
     }
 
-    pub fn get(&self, pubkey: &Pubkey) -> Option<Arc<dyn Executor>> {
+    pub fn get(&self, pubkey: &Pubkey) -> Option<Arc<LoadedProgram>> {
         if let Some(entry) = self.executors.get(pubkey) {
             self.stats.hits.fetch_add(1, Relaxed);
             entry.epoch_count.fetch_add(1, Relaxed);
@@ -187,12 +186,12 @@ impl BankExecutorCache {
         }
     }
 
-    pub fn put(&mut self, executors: impl Iterator<Item = (Pubkey, Arc<dyn Executor>)>) {
+    pub fn put(&mut self, executors: impl Iterator<Item = (Pubkey, Arc<LoadedProgram>)>) {
         let mut new_executors: Vec<_> = executors
             .filter_map(|(key, executor)| {
                 if let Some(mut entry) = self.remove(&key) {
                     self.stats.replacements.fetch_add(1, Relaxed);
-                    entry.executor = executor.clone();
+                    entry.executor = executor;
                     let _ = self.executors.insert(key, entry);
                     None
                 } else {
@@ -353,28 +352,15 @@ impl Stats {
 #[allow(clippy::indexing_slicing)]
 #[cfg(test)]
 mod tests {
-    use {
-        super::*, crate::invoke_context::InvokeContext, solana_sdk::instruction::InstructionError,
-    };
-
-    #[derive(Debug)]
-    struct TestExecutor {}
-    impl Executor for TestExecutor {
-        fn execute(
-            &self,
-            _invoke_context: &mut InvokeContext,
-        ) -> std::result::Result<(), InstructionError> {
-            Ok(())
-        }
-    }
+    use super::*;
 
     #[test]
     fn test_executor_cache() {
         let key1 = solana_sdk::pubkey::new_rand();
         let key2 = solana_sdk::pubkey::new_rand();
         let key3 = solana_sdk::pubkey::new_rand();
         let key4 = solana_sdk::pubkey::new_rand();
-        let executor: Arc<dyn Executor> = Arc::new(TestExecutor {});
+        let executor = Arc::new(LoadedProgram::default());
         let mut cache = BankExecutorCache::new(3, 0);
 
         cache.put([(key1, executor.clone())].into_iter());
@@ -398,7 +384,7 @@ mod tests {
         assert!(cache.get(&key4).is_some());
         assert!(cache.get(&key4).is_some());
         assert!(cache.get(&key4).is_some());
-        cache.put([(key3, executor.clone())].into_iter());
+        cache.put([(key3, executor)].into_iter());
         assert!(cache.get(&key3).is_some());
         let num_retained = [&key1, &key2, &key4]
             .iter()
@@ -413,7 +399,7 @@ mod tests {
         let key2 = solana_sdk::pubkey::new_rand();
         let key3 = solana_sdk::pubkey::new_rand();
         let key4 = solana_sdk::pubkey::new_rand();
-        let executor: Arc<dyn Executor> = Arc::new(TestExecutor {});
+        let executor = Arc::new(LoadedProgram::default());
         let mut cache = BankExecutorCache::new(3, 0);
         assert!(cache.current_epoch == 0);
 
@@ -452,7 +438,7 @@ mod tests {
         cache = BankExecutorCache::new_from_parent_bank_executors(&cache, 2);
         assert!(cache.current_epoch == 2);
 
-        cache.put([(key3, executor.clone())].into_iter());
+        cache.put([(key3, executor)].into_iter());
         assert!(cache.get(&key3).is_some());
     }
 
@@ -461,7 +447,7 @@ mod tests {
         let key1 = solana_sdk::pubkey::new_rand();
         let key2 = solana_sdk::pubkey::new_rand();
         let key3 = solana_sdk::pubkey::new_rand();
-        let executor: Arc<dyn Executor> = Arc::new(TestExecutor {});
+        let executor = Arc::new(LoadedProgram::default());
         let mut cache = BankExecutorCache::new(2, 0);
 
         cache.put([(key1, executor.clone())].into_iter());
@@ -480,7 +466,7 @@ mod tests {
         entries.sort_by_key(|(_, v)| *v);
         assert!(entries[0].1 < entries[1].1);
 
-        cache.put([(key3, executor.clone())].into_iter());
+        cache.put([(key3, executor)].into_iter());
         assert!(cache.get(&entries[0].0).is_none());
         assert!(cache.get(&entries[1].0).is_some());
     }
@@ -491,7 +477,7 @@ mod tests {
 
         let one_hit_wonder = Pubkey::new_unique();
         let popular = Pubkey::new_unique();
-        let executor: Arc<dyn Executor> = Arc::new(TestExecutor {});
+        let executor = Arc::new(LoadedProgram::default());
 
         // make sure we're starting from where we think we are
         assert_eq!(cache.stats.one_hit_wonders.load(Relaxed), 0);
@@ -511,7 +497,7 @@ mod tests {
         assert_eq!(cache.executors[&popular].hit_count.load(Relaxed), 2);
 
         // evict "popular program"
-        cache.put([(one_hit_wonder, executor.clone())].into_iter());
+        cache.put([(one_hit_wonder, executor)].into_iter());
         assert_eq!(cache.executors[&one_hit_wonder].hit_count.load(Relaxed), 1);
 
         // one-hit-wonder counter not incremented
@@ -576,7 +562,7 @@ mod tests {
 
         let program_id1 = Pubkey::new_unique();
         let program_id2 = Pubkey::new_unique();
-        let executor: Arc<dyn Executor> = Arc::new(TestExecutor {});
+        let executor = Arc::new(LoadedProgram::default());
 
         // make sure we're starting from where we think we are
         assert_eq!(ComparableStats::from(&cache.stats), expected_stats,);
@@ -606,7 +592,7 @@ mod tests {
         assert_eq!(ComparableStats::from(&cache.stats), expected_stats);
 
         // evict an executor
-        cache.put([(Pubkey::new_unique(), executor.clone())].into_iter());
+        cache.put([(Pubkey::new_unique(), executor)].into_iter());
         expected_stats.insertions += 1;
         expected_stats.evictions.insert(program_id2, 1);
         assert_eq!(ComparableStats::from(&cache.stats), expected_stats);

program-runtime/src/loaded_programs.rs

@@ -1,5 +1,6 @@
 use {
     crate::invoke_context::InvokeContext,
+    solana_measure::measure::Measure,
     solana_rbpf::{
         elf::Executable,
         error::EbpfError,
@@ -66,7 +67,13 @@ impl Debug for LoadedProgramType {
     }
 }
 
-#[derive(Debug)]
+impl Default for LoadedProgramType {
+    fn default() -> Self {
+        LoadedProgramType::Invalid
+    }
+}
+
+#[derive(Debug, Default)]
 pub struct LoadedProgram {
     /// The program of this entry
     pub program: LoadedProgramType,
@@ -80,6 +87,12 @@ pub struct LoadedProgram {
     pub usage_counter: AtomicU64,
 }
 
+#[derive(Debug, Default)]
+pub struct LoadProgramMetrics {
+    pub load_elf_us: u64,
+    pub verify_code_us: u64,
+}
+
 impl LoadedProgram {
     /// Creates a new user program
     pub fn new(
@@ -88,13 +101,32 @@ impl LoadedProgram {
         deployment_slot: Slot,
         elf_bytes: &[u8],
         account_size: usize,
+        metrics: &mut LoadProgramMetrics,
     ) -> Result<Self, EbpfError> {
         let program = if bpf_loader_deprecated::check_id(loader_key) {
+            let mut load_elf_time = Measure::start("load_elf_time");
             let executable = Executable::load(elf_bytes, loader.clone())?;
-            LoadedProgramType::LegacyV0(VerifiedExecutable::from_executable(executable)?)
+            load_elf_time.stop();
+            metrics.load_elf_us = load_elf_time.as_us();
+
+            let mut verify_code_time = Measure::start("verify_code_time");
+            let program_type =
+                LoadedProgramType::LegacyV0(VerifiedExecutable::from_executable(executable)?);
+            verify_code_time.stop();
+            metrics.verify_code_us = verify_code_time.as_us();
+            program_type
         } else if bpf_loader::check_id(loader_key) || bpf_loader_upgradeable::check_id(loader_key) {
+            let mut load_elf_time = Measure::start("load_elf_time");
             let executable = Executable::load(elf_bytes, loader.clone())?;
-            LoadedProgramType::LegacyV1(VerifiedExecutable::from_executable(executable)?)
+            load_elf_time.stop();
+            metrics.load_elf_us = load_elf_time.as_us();
+
+            let mut verify_code_time = Measure::start("verify_code_time");
+            let program_type =
+                LoadedProgramType::LegacyV1(VerifiedExecutable::from_executable(executable)?);
+            verify_code_time.stop();
+            metrics.verify_code_us = verify_code_time.as_us();
+            program_type
         } else {
             panic!();
         };
@@ -120,6 +152,16 @@ impl LoadedProgram {
             program: LoadedProgramType::BuiltIn(program),
         }
     }
+
+    pub fn new_tombstone() -> Self {
+        Self {
+            program: LoadedProgramType::Invalid,
+            account_size: 0,
+            deployment_slot: 0,
+            effective_slot: 0,
+            usage_counter: AtomicU64::default(),
+        }
+    }
 }
 
 #[derive(Debug, Default)]
@@ -245,6 +287,12 @@ mod tests {
         },
     };
 
+    #[test]
+    fn test_tombstone() {
+        let tombstone = LoadedProgram::new_tombstone();
+        assert!(matches!(tombstone.program, LoadedProgramType::Invalid));
+    }
+
     struct TestForkGraph {
         relation: BlockRelation,
     }