Refactor - Move `Executor` in program-runtime crate (#28782)

* Moves CreateMetrics into the program-runtime crate.

* Moves the Executor trait into executor.rs

* Removes the first_instruction_account parameter from Executor::execute().

program-runtime/src/executor.rs

@@ -0,0 +1,39 @@
+use {
+    crate::{invoke_context::InvokeContext, timings::ExecuteDetailsTimings},
+    solana_sdk::{instruction::InstructionError, saturating_add_assign},
+};
+
+/// Program executor
+pub trait Executor: std::fmt::Debug + Send + Sync {
+    /// Execute the program
+    fn execute(&self, invoke_context: &mut InvokeContext) -> Result<(), InstructionError>;
+}
+
+#[derive(Debug, Default)]
+pub struct CreateMetrics {
+    pub program_id: String,
+    pub register_syscalls_us: u64,
+    pub load_elf_us: u64,
+    pub verify_code_us: u64,
+    pub jit_compile_us: u64,
+}
+
+impl CreateMetrics {
+    pub fn submit_datapoint(&self, timings: &mut ExecuteDetailsTimings) {
+        saturating_add_assign!(
+            timings.create_executor_register_syscalls_us,
+            self.register_syscalls_us
+        );
+        saturating_add_assign!(timings.create_executor_load_elf_us, self.load_elf_us);
+        saturating_add_assign!(timings.create_executor_verify_code_us, self.verify_code_us);
+        saturating_add_assign!(timings.create_executor_jit_compile_us, self.jit_compile_us);
+        datapoint_trace!(
+            "create_executor_trace",
+            ("program_id", self.program_id, String),
+            ("register_syscalls_us", self.register_syscalls_us, i64),
+            ("load_elf_us", self.load_elf_us, i64),
+            ("verify_code_us", self.verify_code_us, i64),
+            ("jit_compile_us", self.jit_compile_us, i64),
+        );
+    }
+}

program-runtime/src/executor_cache.rs

@@ -1,11 +1,8 @@
 use {
-    crate::invoke_context::InvokeContext,
+    crate::executor::Executor,
     log::*,
     rand::Rng,
-    solana_sdk::{
-        instruction::InstructionError, pubkey::Pubkey, saturating_add_assign, slot_history::Slot,
-        stake_history::Epoch, transaction_context::IndexOfAccount,
-    },
+    solana_sdk::{pubkey::Pubkey, saturating_add_assign, slot_history::Slot, stake_history::Epoch},
     std::{
         collections::HashMap,
         fmt::Debug,
@@ -17,16 +14,6 @@ use {
     },
 };
 
-/// Program executor
-pub trait Executor: Debug + Send + Sync {
-    /// Execute the program
-    fn execute(
-        &self,
-        first_instruction_account: IndexOfAccount,
-        invoke_context: &mut InvokeContext,
-    ) -> Result<(), InstructionError>;
-}
-
 /// Relation between a TransactionExecutorCacheEntry and its matching BankExecutorCacheEntry
 #[repr(u8)]
 #[derive(Clone, Copy, PartialEq, Eq, Debug)]
@@ -386,7 +373,6 @@ mod tests {
     impl Executor for TestExecutor {
         fn execute(
             &self,
-            _first_instruction_account: IndexOfAccount,
             _invoke_context: &mut InvokeContext,
         ) -> std::result::Result<(), InstructionError> {
             Ok(())

program-runtime/src/lib.rs

@@ -11,6 +11,7 @@ extern crate solana_metrics;
 
 pub mod accounts_data_meter;
 pub mod compute_budget;
+pub mod executor;
 pub mod executor_cache;
 pub mod invoke_context;
 pub mod log_collector;

programs/bpf_loader/src/lib.rs

@@ -9,9 +9,6 @@ pub mod upgradeable;
 pub mod upgradeable_with_jit;
 pub mod with_jit;
 
-#[macro_use]
-extern crate solana_metrics;
-
 use {
     crate::{
         allocator_bump::BpfAllocator,
@@ -22,13 +19,13 @@ use {
     solana_measure::measure::Measure,
     solana_program_runtime::{
         compute_budget::ComputeBudget,
-        executor_cache::{Executor, TransactionExecutorCache},
+        executor::{CreateMetrics, Executor},
+        executor_cache::TransactionExecutorCache,
         ic_logger_msg, ic_msg,
         invoke_context::{ComputeMeter, InvokeContext},
         log_collector::LogCollector,
         stable_log,
         sysvar_cache::get_sysvar_with_account_check,
-        timings::ExecuteDetailsTimings,
     },
     solana_rbpf::{
         aligned_memory::AlignedMemory,
@@ -91,39 +88,6 @@ pub enum BpfError {
 }
 impl UserDefinedError for BpfError {}
 
-mod executor_metrics {
-    use super::*;
-
-    #[derive(Debug, Default)]
-    pub struct CreateMetrics {
-        pub program_id: String,
-        pub register_syscalls_us: u64,
-        pub load_elf_us: u64,
-        pub verify_code_us: u64,
-        pub jit_compile_us: u64,
-    }
-
-    impl CreateMetrics {
-        pub fn submit_datapoint(&self, timings: &mut ExecuteDetailsTimings) {
-            saturating_add_assign!(
-                timings.create_executor_register_syscalls_us,
-                self.register_syscalls_us
-            );
-            saturating_add_assign!(timings.create_executor_load_elf_us, self.load_elf_us);
-            saturating_add_assign!(timings.create_executor_verify_code_us, self.verify_code_us);
-            saturating_add_assign!(timings.create_executor_jit_compile_us, self.jit_compile_us);
-            datapoint_trace!(
-                "create_executor_trace",
-                ("program_id", self.program_id, String),
-                ("register_syscalls_us", self.register_syscalls_us, i64),
-                ("load_elf_us", self.load_elf_us, i64),
-                ("verify_code_us", self.verify_code_us, i64),
-                ("jit_compile_us", self.jit_compile_us, i64),
-            );
-        }
-    }
-}
-
 // The BPF loader is special in that it is the only place in the runtime and its built-in programs,
 // where data comes not only from instruction account but also program accounts.
 // Thus, these two helper methods have to distinguish the mixed sources via index_in_instruction.
@@ -162,7 +126,7 @@ fn create_executor_from_bytes(
     feature_set: &FeatureSet,
     compute_budget: &ComputeBudget,
     log_collector: Option<Rc<RefCell<LogCollector>>>,
-    create_executor_metrics: &mut executor_metrics::CreateMetrics,
+    create_executor_metrics: &mut CreateMetrics,
     programdata: &[u8],
     use_jit: bool,
     reject_deployment_of_broken_elfs: bool,
@@ -246,7 +210,7 @@ pub fn create_executor_from_account(
     program: &BorrowedAccount,
     programdata: &BorrowedAccount,
     use_jit: bool,
-) -> Result<(Arc<dyn Executor>, Option<executor_metrics::CreateMetrics>), InstructionError> {
+) -> Result<(Arc<dyn Executor>, Option<CreateMetrics>), InstructionError> {
     if !check_loader_id(program.get_owner()) {
         ic_logger_msg!(
             log_collector,
@@ -292,9 +256,9 @@ pub fn create_executor_from_account(
         }
     }
 
-    let mut create_executor_metrics = executor_metrics::CreateMetrics {
+    let mut create_executor_metrics = CreateMetrics {
         program_id: program.get_key().to_string(),
-        ..executor_metrics::CreateMetrics::default()
+        ..CreateMetrics::default()
     };
     let executor = create_executor_from_bytes(
         feature_set,
@@ -489,7 +453,7 @@ fn process_instruction_common(
             create_executor_metrics.submit_datapoint(&mut invoke_context.timings);
         }
 
-        executor.execute(program_account_index, invoke_context)
+        executor.execute(invoke_context)
     } else {
         drop(program);
         debug_assert_eq!(first_instruction_account, 1);
@@ -702,7 +666,7 @@ fn process_loader_upgradeable_instruction(
             let instruction_context = transaction_context.get_current_instruction_context()?;
             let buffer =
                 instruction_context.try_borrow_instruction_account(transaction_context, 3)?;
-            let mut create_executor_metrics = executor_metrics::CreateMetrics::default();
+            let mut create_executor_metrics = CreateMetrics::default();
             let executor = create_executor_from_bytes(
                 &invoke_context.feature_set,
                 invoke_context.get_compute_budget(),
@@ -881,7 +845,7 @@ fn process_loader_upgradeable_instruction(
             // Load and verify the program bits
             let buffer =
                 instruction_context.try_borrow_instruction_account(transaction_context, 2)?;
-            let mut create_executor_metrics = executor_metrics::CreateMetrics::default();
+            let mut create_executor_metrics = CreateMetrics::default();
             let executor = create_executor_from_bytes(
                 &invoke_context.feature_set,
                 invoke_context.get_compute_budget(),
@@ -1378,7 +1342,7 @@ fn process_loader_instruction(
                 ic_msg!(invoke_context, "key[0] did not sign the transaction");
                 return Err(InstructionError::MissingRequiredSignature);
             }
-            let mut create_executor_metrics = executor_metrics::CreateMetrics::default();
+            let mut create_executor_metrics = CreateMetrics::default();
             let executor = create_executor_from_bytes(
                 &invoke_context.feature_set,
                 invoke_context.get_compute_budget(),
@@ -1436,11 +1400,7 @@ impl Debug for BpfExecutor {
 }
 
 impl Executor for BpfExecutor {
-    fn execute(
-        &self,
-        _first_instruction_account: IndexOfAccount,
-        invoke_context: &mut InvokeContext,
-    ) -> Result<(), InstructionError> {
+    fn execute(&self, invoke_context: &mut InvokeContext) -> Result<(), InstructionError> {
         let log_collector = invoke_context.get_log_collector();
         let compute_meter = invoke_context.get_compute_meter();
         let stack_height = invoke_context.get_stack_height();

runtime/src/bank.rs

@@ -89,8 +89,9 @@ use {
     solana_program_runtime::{
         accounts_data_meter::MAX_ACCOUNTS_DATA_LEN,
         compute_budget::{self, ComputeBudget},
+        executor::Executor,
         executor_cache::{
-            BankExecutorCache, Executor, TransactionExecutorCache, TxBankExecutorCacheDiff,
+            BankExecutorCache, TransactionExecutorCache, TxBankExecutorCacheDiff,
             MAX_CACHED_EXECUTORS,
         },
         invoke_context::{BuiltinProgram, ProcessInstructionWithContext},
@@ -7950,7 +7951,6 @@ pub(crate) mod tests {
         rand::Rng,
         solana_program_runtime::{
             compute_budget::MAX_COMPUTE_UNIT_LIMIT,
-            executor_cache::Executor,
             invoke_context::{mock_process_instruction, InvokeContext},
             prioritization_fee::{PrioritizationFeeDetails, PrioritizationFeeType},
         },
@@ -15129,7 +15129,6 @@ pub(crate) mod tests {
     impl Executor for TestExecutor {
         fn execute(
             &self,
-            _first_instruction_account: IndexOfAccount,
             _invoke_context: &mut InvokeContext,
         ) -> std::result::Result<(), InstructionError> {
             Ok(())