SVM: fix executor metrics accumulation in program loader (#3695)

(cherry picked from commit fa7d553)

runtime/src/bank.rs

@@ -7203,7 +7203,14 @@ impl Bank {
         let environments = self
             .transaction_processor
             .get_environments_for_epoch(effective_epoch)?;
-        load_program_with_pubkey(self, &environments, pubkey, self.slot(), reload)
+        load_program_with_pubkey(
+            self,
+            &environments,
+            pubkey,
+            self.slot(),
+            &mut ExecuteTimings::default(), // Called by ledger-tool, metrics not accumulated.
+            reload,
+        )
     }
 
     pub fn withdraw(&self, pubkey: &Pubkey, lamports: u64) -> Result<()> {

svm/src/program_loader.rs

@@ -15,7 +15,7 @@ use {
         pubkey::Pubkey,
         transaction::{self, TransactionError},
     },
-    solana_timings::ExecuteDetailsTimings,
+    solana_timings::ExecuteTimings,
     solana_type_overrides::sync::Arc,
 };
 
@@ -124,6 +124,7 @@ pub fn load_program_with_pubkey<CB: TransactionProcessingCallback>(
     environments: &ProgramRuntimeEnvironments,
     pubkey: &Pubkey,
     slot: Slot,
+    execute_timings: &mut ExecuteTimings,
     reload: bool,
 ) -> Option<Arc<ProgramCacheEntry>> {
     let mut load_program_metrics = LoadProgramMetrics {
@@ -210,8 +211,7 @@ pub fn load_program_with_pubkey<CB: TransactionProcessingCallback>(
         )
     });
 
-    let mut timings = ExecuteDetailsTimings::default();
-    load_program_metrics.submit_datapoint(&mut timings);
+    load_program_metrics.submit_datapoint(&mut execute_timings.details);
     loaded_program.update_access_slot(slot);
     Some(Arc::new(loaded_program))
 }
@@ -528,6 +528,7 @@ mod tests {
             &batch_processor.get_environments_for_epoch(50).unwrap(),
             &key,
             500,
+            &mut ExecuteTimings::default(),
             false,
         );
         assert!(result.is_none());
@@ -550,6 +551,7 @@ mod tests {
             &batch_processor.get_environments_for_epoch(20).unwrap(),
             &key,
             0, // Slot 0
+            &mut ExecuteTimings::default(),
             false,
         );
 
@@ -584,6 +586,7 @@ mod tests {
             &batch_processor.get_environments_for_epoch(20).unwrap(),
             &key,
             200,
+            &mut ExecuteTimings::default(),
             false,
         );
         let loaded_program = ProgramCacheEntry::new_tombstone(
@@ -611,6 +614,7 @@ mod tests {
             &batch_processor.get_environments_for_epoch(20).unwrap(),
             &key,
             200,
+            &mut ExecuteTimings::default(),
             false,
         );
 
@@ -664,6 +668,7 @@ mod tests {
             &batch_processor.get_environments_for_epoch(0).unwrap(),
             &key1,
             0,
+            &mut ExecuteTimings::default(),
             false,
         );
         let loaded_program = ProgramCacheEntry::new_tombstone(
@@ -701,6 +706,7 @@ mod tests {
             &batch_processor.get_environments_for_epoch(20).unwrap(),
             &key1,
             200,
+            &mut ExecuteTimings::default(),
             false,
         );
 
@@ -750,6 +756,7 @@ mod tests {
             &batch_processor.get_environments_for_epoch(0).unwrap(),
             &key,
             0,
+            &mut ExecuteTimings::default(),
             false,
         );
         let loaded_program = ProgramCacheEntry::new_tombstone(
@@ -783,6 +790,7 @@ mod tests {
             &batch_processor.get_environments_for_epoch(20).unwrap(),
             &key,
             200,
+            &mut ExecuteTimings::default(),
             false,
         );
 
@@ -833,6 +841,7 @@ mod tests {
                     .unwrap(),
                 &key,
                 200,
+                &mut ExecuteTimings::default(),
                 false,
             )
             .unwrap();

svm/src/transaction_processor.rs

@@ -297,6 +297,7 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
             let program_cache_for_tx_batch = self.replenish_program_cache(
                 callbacks,
                 &program_accounts_map,
+                &mut execute_timings,
                 config.check_program_modification_slot,
                 config.limit_to_load_programs,
             );
@@ -569,6 +570,7 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
         &self,
         callback: &CB,
         program_accounts_map: &HashMap<Pubkey, (&Pubkey, u64)>,
+        execute_timings: &mut ExecuteTimings,
         check_program_modification_slot: bool,
         limit_to_load_programs: bool,
     ) -> ProgramCacheForTxBatch {
@@ -616,6 +618,7 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
                         &program_cache.get_environments_for_epoch(self.epoch),
                         &key,
                         self.slot,
+                        execute_timings,
                         false,
                     )
                     .expect("called load_program_with_pubkey() with nonexistent account");
@@ -688,6 +691,7 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
                     &environments_for_epoch,
                     &key,
                     self.slot,
+                    &mut ExecuteTimings::default(),
                     false,
                 ) {
                     recompiled.tx_usage_counter.fetch_add(
@@ -1387,7 +1391,13 @@ mod tests {
         let mut account_maps: HashMap<Pubkey, (&Pubkey, u64)> = HashMap::new();
         account_maps.insert(key, (&owner, 4));
 
-        batch_processor.replenish_program_cache(&mock_bank, &account_maps, false, true);
+        batch_processor.replenish_program_cache(
+            &mock_bank,
+            &account_maps,
+            &mut ExecuteTimings::default(),
+            false,
+            true,
+        );
     }
 
     #[test]
@@ -1416,6 +1426,7 @@ mod tests {
             let result = batch_processor.replenish_program_cache(
                 &mock_bank,
                 &account_maps,
+                &mut ExecuteTimings::default(),
                 false,
                 limit_to_load_programs,
             );

svm/tests/concurrent_tests.rs

@@ -32,6 +32,7 @@ use {
             TransactionProcessingEnvironment,
         },
     },
+    solana_timings::ExecuteTimings,
     std::collections::HashMap,
 };
 
@@ -69,7 +70,13 @@ fn program_cache_execution(threads: usize) {
             let maps = account_maps.clone();
             let programs = programs.clone();
             thread::spawn(move || {
-                let result = processor.replenish_program_cache(&local_bank, &maps, false, true);
+                let result = processor.replenish_program_cache(
+                    &local_bank,
+                    &maps,
+                    &mut ExecuteTimings::default(),
+                    false,
+                    true,
+                );
                 for key in &programs {
                     let cache_entry = result.find(key);
                     assert!(matches!(

svm/tests/conformance.rs

@@ -433,6 +433,7 @@ fn execute_fixture_as_instr(
         &batch_processor.get_environments_for_epoch(2).unwrap(),
         &program_id,
         42,
+        &mut ExecuteTimings::default(),
         false,
     )
     .unwrap();

svm/tests/integration_test.rs

@@ -1300,3 +1300,40 @@ fn svm_inspect_account() {
         num_actual_inspected_accounts,
     );
 }
+
+// Tests for proper accumulation of metrics across loaded programs in a batch.
+#[test]
+fn svm_metrics_accumulation() {
+    for test_entry in program_medley() {
+        let env = SvmTestEnvironment::create(test_entry);
+
+        let (transactions, check_results) = env.test_entry.prepare_transactions();
+
+        let result = env.batch_processor.load_and_execute_sanitized_transactions(
+            &env.mock_bank,
+            &transactions,
+            check_results,
+            &env.processing_environment,
+            &env.processing_config,
+        );
+
+        assert_ne!(
+            result
+                .execute_timings
+                .details
+                .create_executor_jit_compile_us,
+            0
+        );
+        assert_ne!(
+            result.execute_timings.details.create_executor_load_elf_us,
+            0
+        );
+        assert_ne!(
+            result
+                .execute_timings
+                .details
+                .create_executor_verify_code_us,
+            0
+        );
+    }
+}