SamplePerformanceService: Refactor stats snapshot logic (#30297)

Snapshot construction and interaction code was a bit more manual than necessary, even causing a bug to slip past a review.  Separated snapshot construction from the diffing of two snapshots.
This should make the logic clearer.

core/src/sample_performance_service.rs

@@ -14,12 +14,6 @@ use {
 const SAMPLE_INTERVAL: u64 = 60;
 const SLEEP_INTERVAL: u64 = 500;
 
-pub struct SamplePerformanceSnapshot {
-    pub num_transactions: u64,
-    pub num_non_vote_transactions: u64,
-    pub highest_slot: u64,
-}
-
 pub struct SamplePerformanceService {
     thread_hdl: JoinHandle<()>,
 }
@@ -39,7 +33,7 @@ impl SamplePerformanceService {
         let thread_hdl = Builder::new()
             .name("sample-performance".to_string())
             .spawn(move || {
-                Self::run(bank_forks, &blockstore, exit);
+                Self::run(bank_forks, blockstore, exit);
             })
             .unwrap();
 
@@ -48,23 +42,10 @@ impl SamplePerformanceService {
 
     pub fn run(
         bank_forks: Arc<RwLock<BankForks>>,
-        blockstore: &Arc<Blockstore>,
+        blockstore: Arc<Blockstore>,
         exit: Arc<AtomicBool>,
     ) {
-        let mut snapshot = {
+        let mut snapshot = StatsSnapshot::from_forks(&bank_forks);
-            let forks = bank_forks.read().unwrap();
-            let bank = forks.root_bank();
-            let highest_slot = forks.highest_slot();
-
-            // Store the absolute transaction counts to that we can compute the
-            // difference between these values at points in time to figure out
-            // how many transactions occurred in that timespan.
-            SamplePerformanceSnapshot {
-                num_transactions: bank.transaction_count(),
-                num_non_vote_transactions: bank.non_vote_transaction_count_since_restart(),
-                highest_slot,
-            }
-        };
 
         let mut now = Instant::now();
         loop {
@@ -76,18 +57,13 @@ impl SamplePerformanceService {
 
             if elapsed.as_secs() >= SAMPLE_INTERVAL {
                 now = Instant::now();
-                let (bank, highest_slot) = {
+                let new_snapshot = StatsSnapshot::from_forks(&bank_forks);
-                    let bank_forks = bank_forks.read().unwrap();
-                    (bank_forks.root_bank(), bank_forks.highest_slot())
-                };
 
-                let num_slots = highest_slot.saturating_sub(snapshot.highest_slot);
+                let (num_transactions, num_non_vote_transactions, num_slots) =
-                let num_transactions = bank
+                    new_snapshot.diff_since(&snapshot);
-                    .transaction_count()
+
-                    .saturating_sub(snapshot.num_transactions);
+                // Store the new snapshot to compare against in the next iteration of the loop.
-                let num_non_vote_transactions = bank
+                snapshot = new_snapshot;
-                    .non_vote_transaction_count_since_restart()
-                    .saturating_sub(snapshot.num_non_vote_transactions);
 
                 let perf_sample = PerfSampleV2 {
                     num_slots,
@@ -96,17 +72,10 @@ impl SamplePerformanceService {
                     sample_period_secs: elapsed.as_secs() as u16,
                 };
 
+                let highest_slot = snapshot.highest_slot;
                 if let Err(e) = blockstore.write_perf_sample(highest_slot, &perf_sample) {
                     error!("write_perf_sample failed: slot {:?} {:?}", highest_slot, e);
                 }
-
-                // Same as above, store the absolute transaction counts to use
-                // as comparison for the next iteration of this loop.
-                snapshot = SamplePerformanceSnapshot {
-                    num_transactions: bank.transaction_count(),
-                    num_non_vote_transactions: bank.non_vote_transaction_count_since_restart(),
-                    highest_slot,
-                };
             }
 
             sleep(Duration::from_millis(SLEEP_INTERVAL));
@@ -117,3 +86,31 @@ impl SamplePerformanceService {
         self.thread_hdl.join()
     }
 }
+
+struct StatsSnapshot {
+    pub num_transactions: u64,
+    pub num_non_vote_transactions: u64,
+    pub highest_slot: u64,
+}
+
+impl StatsSnapshot {
+    fn from_forks(forks: &RwLock<BankForks>) -> Self {
+        let forks = forks.read().unwrap();
+        let bank = forks.root_bank();
+        Self {
+            num_transactions: bank.transaction_count(),
+            num_non_vote_transactions: bank.non_vote_transaction_count_since_restart(),
+            highest_slot: forks.highest_slot(),
+        }
+    }
+
+    fn diff_since(&self, predecessor: &Self) -> (u64, u64, u64) {
+        (
+            self.num_transactions
+                .saturating_sub(predecessor.num_transactions),
+            self.num_non_vote_transactions
+                .saturating_sub(predecessor.num_non_vote_transactions),
+            self.highest_slot.saturating_sub(predecessor.highest_slot),
+        )
+    }
+}