Optimize PER calculation - skip stale vote accounts (#5967)

* skip stale vote account in PER calc

* skip stale vote accounts in point calculation too

* add a flag to turn on stake_cache assert when needed and update a
comments in test to indicate the check is only for test

* pr: refactor vote account check

* pr: implement convert from vote_account to account shared data for ref

* fix merge conflicts

runtime/src/bank/metrics.rs

@@ -22,7 +22,6 @@ pub(crate) struct RewardsMetrics {
     pub(crate) redeem_rewards_us: u64,
     pub(crate) store_stake_accounts_us: AtomicU64,
     pub(crate) store_vote_accounts_us: AtomicU64,
-    pub(crate) vote_accounts_cache_miss_count: AtomicU64,
 }
 
 pub(crate) struct NewBankTimings {
@@ -92,11 +91,6 @@ pub(crate) fn report_new_epoch_metrics(
             metrics.store_vote_accounts_us.load(Relaxed),
             i64
         ),
-        (
-            "vote_accounts_cache_miss_count",
-            metrics.vote_accounts_cache_miss_count.load(Relaxed),
-            i64
-        ),
     );
 }
 

runtime/src/bank/partitioned_epoch_rewards/calculation.rs

@@ -8,7 +8,7 @@ use {
     crate::{
         bank::{
             PrevEpochInflationRewards, RewardCalcTracer, RewardCalculationEvent, RewardsMetrics,
-            VoteAccount, VoteReward, VoteRewards,
+            VoteReward, VoteRewards,
         },
         inflation_rewards::{
             points::{calculate_points, PointValue},
@@ -26,10 +26,12 @@ use {
     },
     solana_clock::{Epoch, Slot},
     solana_measure::measure_us,
-    solana_pubkey::Pubkey,
-    solana_reward_info::RewardInfo,
-    solana_stake_interface::state::Delegation,
-    solana_sysvar::epoch_rewards::EpochRewards,
+    solana_sdk::{
+        account::ReadableAccount, pubkey::Pubkey, reward_info::RewardInfo,
+        stake::state::Delegation, sysvar::epoch_rewards::EpochRewards,
+    },
+    solana_vote::vote_account::VoteAccount,
+    solana_vote_program::vote_state::VoteStateVersions,
     std::sync::{
         atomic::{AtomicU64, Ordering::Relaxed},
         Arc,
@@ -296,21 +298,6 @@ impl Bank {
             cached_vote_accounts,
         } = reward_calculate_params;
 
-        let solana_vote_program: Pubkey = solana_vote_program::id();
-
-        let get_vote_account = |vote_pubkey: &Pubkey| -> Option<VoteAccount> {
-            if let Some(vote_account) = cached_vote_accounts.get(vote_pubkey) {
-                return Some(vote_account.clone());
-            }
-            // If accounts-db contains a valid vote account, then it should
-            // already have been cached in cached_vote_accounts; so the code
-            // below is only for sanity checking, and can be removed once
-            // the cache is deemed to be reliable.
-            metrics.vote_accounts_cache_miss_count.fetch_add(1, Relaxed);
-            let account = self.get_account_with_fixed_root(vote_pubkey)?;
-            VoteAccount::try_from(account).ok()
-        };
-
         let new_warmup_cooldown_rate_epoch = self.new_warmup_cooldown_rate_epoch();
         let estimated_num_vote_accounts = cached_vote_accounts.len();
         let vote_account_rewards: VoteRewards = DashMap::with_capacity_and_hasher_and_shard_amount(
@@ -320,6 +307,7 @@ impl Bank {
         );
 
         let total_stake_rewards = AtomicU64::default();
+        const ASSERT_STAKE_CACHE: bool = false; // Turn this on to assert that all vote accounts are in the cache
         let (stake_rewards, measure_stake_rewards_us) = measure_us!(thread_pool.install(|| {
             stake_delegations
                 .par_iter()
@@ -334,10 +322,22 @@ impl Bank {
 
                     let stake_pubkey = **stake_pubkey;
                     let vote_pubkey = stake_account.delegation().voter_pubkey;
-                    let vote_account = get_vote_account(&vote_pubkey)?;
-                    if vote_account.owner() != &solana_vote_program {
-                        return None;
+                    let vote_account_from_cache = cached_vote_accounts.get(&vote_pubkey);
+                    if ASSERT_STAKE_CACHE && vote_account_from_cache.is_none() {
+                        let account_from_db = self.get_account_with_fixed_root(&vote_pubkey);
+                        if let Some(account_from_db) = account_from_db {
+                            if VoteStateVersions::is_correct_size_and_initialized(
+                                account_from_db.data(),
+                            ) && VoteAccount::try_from(account_from_db.clone()).is_ok()
+                            {
+                                panic!(
+                                    "Vote account {} not found in cache, but found in db: {:?}",
+                                    vote_pubkey, account_from_db
+                                );
+                            }
+                        }
                     }
+                    let vote_account = vote_account_from_cache?;
                     let vote_state_view = vote_account.vote_state_view();
                     let mut stake_state = *stake_account.stake_state();
 
@@ -419,19 +419,6 @@ impl Bank {
         } = reward_calculate_params;
 
         let solana_vote_program: Pubkey = solana_vote_program::id();
-
-        let get_vote_account = |vote_pubkey: &Pubkey| -> Option<VoteAccount> {
-            if let Some(vote_account) = cached_vote_accounts.get(vote_pubkey) {
-                return Some(vote_account.clone());
-            }
-            // If accounts-db contains a valid vote account, then it should
-            // already have been cached in cached_vote_accounts; so the code
-            // below is only for sanity checking, and can be removed once
-            // the cache is deemed to be reliable.
-            let account = self.get_account_with_fixed_root(vote_pubkey)?;
-            VoteAccount::try_from(account).ok()
-        };
-
         let new_warmup_cooldown_rate_epoch = self.new_warmup_cooldown_rate_epoch();
         let (points, measure_us) = measure_us!(thread_pool.install(|| {
             stake_delegations
@@ -439,7 +426,7 @@ impl Bank {
                 .map(|(_stake_pubkey, stake_account)| {
                     let vote_pubkey = stake_account.delegation().voter_pubkey;
 
-                    let Some(vote_account) = get_vote_account(&vote_pubkey) else {
+                    let Some(vote_account) = cached_vote_accounts.get(&vote_pubkey) else {
                         return 0;
                     };
                     if vote_account.owner() != &solana_vote_program {

runtime/src/bank/tests.rs

@@ -1954,8 +1954,8 @@ impl Bank {
             }
             // If accounts-db contains a valid vote account, then it should
             // already have been cached in cached_vote_accounts; so the code
-            // below is only for sanity check, and can be removed once
-            // vote_accounts_cache_miss_count is shown to be always zero.
+            // below is only for sanity check in tests, and should not be hit
+            // in practice.
             let account = self.get_account_with_fixed_root(vote_pubkey)?;
             if account.owner() == &solana_vote_program
                 && VoteState::deserialize(account.data()).is_ok()

vote/src/vote_account.rs

@@ -311,6 +311,12 @@ impl Serialize for VoteAccount {
     }
 }
 
+impl<'a> From<&'a VoteAccount> for AccountSharedData {
+    fn from(account: &'a VoteAccount) -> Self {
+        account.0.account.clone()
+    }
+}
+
 impl From<VoteAccount> for AccountSharedData {
     fn from(account: VoteAccount) -> Self {
         account.0.account.clone()