uses OnceCell instead of RwLock+Once to cache vote-state in vote-account (#26257)

RwLock seems excessive since only the very 1st call to VoteAccount::vote_state
will write-lock the inner field. Future calls would also incur overhead of an
RwLockReadGuard.
once_cell::sync::OnceCell provides a matching api to the desired functionality.

Cargo.lock

@@ -2820,9 +2820,9 @@ checksum = "830b246a0e5f20af87141b25c173cd1b609bd7779a4617d6ec582abaf90870f3"
 
 [[package]]
 name = "once_cell"
-version = "1.8.0"
+version = "1.12.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "692fcb63b64b1758029e0a96ee63e049ce8c5948587f2f7208df04625e5f6b56"
+checksum = "7709cef83f0c1f58f666e746a08b21e0085f7440fa6a29cc194d68aac97a4225"
 
 [[package]]
 name = "opaque-debug"
@@ -5113,6 +5113,7 @@ dependencies = [
  "lazy_static",
  "log",
  "memmap2",
+ "once_cell",
  "rustc_version 0.4.0",
  "serde",
  "serde_bytes",
@@ -5876,6 +5877,7 @@ dependencies = [
  "num-derive",
  "num-traits",
  "num_cpus",
+ "once_cell",
  "ouroboros",
  "rand 0.7.3",
  "rand_chacha 0.2.2",

frozen-abi/Cargo.toml

@@ -14,6 +14,7 @@ bs58 = "0.4.0"
 bv = { version = "0.11.1", features = ["serde"] }
 lazy_static = "1.4.0"
 log = "0.4.17"
+once_cell = "1.12.0"
 serde = "1.0.137"
 serde_bytes = "0.11"
 serde_derive = "1.0.103"

frozen-abi/src/abi_example.rs

@@ -555,3 +555,9 @@ impl<O: AbiEnumVisitor, E: AbiEnumVisitor> AbiEnumVisitor for Result<O, E> {
         digester.create_child()
     }
 }
+
+impl<T: AbiExample> AbiExample for once_cell::sync::OnceCell<T> {
+    fn example() -> Self {
+        Self::with_value(T::example())
+    }
+}

programs/bpf/Cargo.lock

@@ -2548,9 +2548,9 @@ checksum = "830b246a0e5f20af87141b25c173cd1b609bd7779a4617d6ec582abaf90870f3"
 
 [[package]]
 name = "once_cell"
-version = "1.8.0"
+version = "1.12.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "692fcb63b64b1758029e0a96ee63e049ce8c5948587f2f7208df04625e5f6b56"
+checksum = "7709cef83f0c1f58f666e746a08b21e0085f7440fa6a29cc194d68aac97a4225"
 
 [[package]]
 name = "opaque-debug"
@@ -4703,6 +4703,7 @@ dependencies = [
  "lazy_static",
  "log",
  "memmap2",
+ "once_cell",
  "rustc_version",
  "serde",
  "serde_bytes",
@@ -5210,6 +5211,7 @@ dependencies = [
  "num-derive",
  "num-traits",
  "num_cpus",
+ "once_cell",
  "ouroboros",
  "rand 0.7.3",
  "rayon",

runtime/Cargo.toml

@@ -32,6 +32,7 @@ memmap2 = "0.5.3"
 num-derive = { version = "0.3" }
 num-traits = { version = "0.2" }
 num_cpus = "1.13.1"
+once_cell = "1.12.0"
 ouroboros = "0.15.0"
 rand = "0.7.0"
 rayon = "1.5.3"

runtime/src/bank.rs

@@ -2724,11 +2724,15 @@ impl Bank {
             "distributed inflation: {} (rounded from: {})",
             validator_rewards_paid, validator_rewards
         );
-
-        let num_stake_accounts = self.stakes_cache.num_stake_accounts();
-        let num_vote_accounts = self.stakes_cache.num_vote_accounts();
-        let num_staked_nodes = self.stakes_cache.num_staked_nodes();
-
+        // TODO: staked_nodes forces an eager stakes calculation. remove it!
+        let (num_stake_accounts, num_vote_accounts, num_staked_nodes) = {
+            let stakes = self.stakes_cache.stakes();
+            (
+                stakes.stake_delegations().len(),
+                stakes.vote_accounts().len(),
+                stakes.staked_nodes().len(),
+            )
+        };
         self.capitalization
             .fetch_add(validator_rewards_paid, Relaxed);
 
@@ -2817,6 +2821,7 @@ impl Bank {
                         }
                     };
                     if cached_stake_account != &stake_account {
+                        invalid_cached_stake_accounts.fetch_add(1, Relaxed);
                         let mut cached_account = cached_stake_account.account().clone();
                         // We could have collected rent on the loaded account already in this new epoch (we could be at partition_index 12, for example).
                         // So, we may need to adjust the rent_epoch of the cached account. So, update rent_epoch and compare just the accounts.
@@ -2829,8 +2834,6 @@ impl Bank {
                             stake_pubkey,
                             &self.rewrites_skipped_this_slot,
                         );
-                        // prior behavior was to always count any account that mismatched for any reason
-                        invalid_cached_stake_accounts.fetch_add(1, Relaxed);
                         if &cached_account != stake_account.account() {
                             info!(
                                 "cached stake account mismatch: {}: {:?}, {:?}",
@@ -5051,9 +5054,7 @@ impl Bank {
                     None
                 } else {
                     total_staked += *staked;
-                    account
-                        .node_pubkey()
-                        .map(|node_pubkey| (node_pubkey, *staked))
+                    Some((account.node_pubkey()?, *staked))
                 }
             })
             .collect::<Vec<(Pubkey, u64)>>();

runtime/src/stake_account.rs

@@ -44,6 +44,10 @@ impl<T> StakeAccount<T> {
     pub(crate) fn stake_state(&self) -> &StakeState {
         &self.stake_state
     }
+
+    pub(crate) fn account(&self) -> &AccountSharedData {
+        &self.account
+    }
 }
 
 impl StakeAccount<Delegation> {
@@ -103,12 +107,6 @@ impl<T> From<StakeAccount<T>> for (AccountSharedData, StakeState) {
     }
 }
 
-impl<T> StakeAccount<T> {
-    pub fn account(&self) -> &AccountSharedData {
-        &self.account
-    }
-}
-
 impl<S, T> PartialEq<StakeAccount<S>> for StakeAccount<T> {
     fn eq(&self, other: &StakeAccount<S>) -> bool {
         let StakeAccount {

runtime/src/stakes.rs

@@ -57,21 +57,6 @@ impl StakesCache {
         self.0.read().unwrap()
     }
 
-    pub fn num_stake_accounts(&self) -> usize {
-        let stakes = self.0.read().unwrap();
-        stakes.stake_delegations.len()
-    }
-
-    pub fn num_vote_accounts(&self) -> usize {
-        let stakes = self.0.read().unwrap();
-        stakes.vote_accounts.num_vote_accounts()
-    }
-
-    pub fn num_staked_nodes(&self) -> usize {
-        let stakes = self.0.read().unwrap();
-        stakes.vote_accounts.num_staked_nodes()
-    }
-
     pub fn check_and_store(&self, pubkey: &Pubkey, account: &impl ReadableAccount) {
         // TODO: If the account is already cached as a vote or stake account
         // but the owner changes, then this needs to evict the account from

runtime/src/vote_account.rs

@@ -1,5 +1,6 @@
 use {
     itertools::Itertools,
+    once_cell::sync::OnceCell,
     serde::ser::{Serialize, Serializer},
     solana_sdk::{
         account::{accounts_equal, AccountSharedData, ReadableAccount},
@@ -11,17 +12,11 @@ use {
         cmp::Ordering,
         collections::{hash_map::Entry, HashMap},
         iter::FromIterator,
-        sync::{Arc, Once, RwLock, RwLockReadGuard},
+        sync::{Arc, Once, RwLock},
     },
     thiserror::Error,
 };
 
-// The value here does not matter. It will be overwritten
-// at the first call to VoteAccount::vote_state().
-const INVALID_VOTE_STATE: Result<VoteState, Error> = Err(Error::InstructionError(
-    InstructionError::InvalidAccountData,
-));
-
 #[derive(Clone, Debug, PartialEq, AbiExample, Deserialize)]
 #[serde(try_from = "AccountSharedData")]
 pub struct VoteAccount(Arc<VoteAccountInner>);
@@ -37,8 +32,7 @@ pub enum Error {
 #[derive(Debug, AbiExample)]
 struct VoteAccountInner {
     account: AccountSharedData,
-    vote_state: RwLock<Result<VoteState, Error>>,
-    vote_state_once: Once,
+    vote_state: OnceCell<Result<VoteState, Error>>,
 }
 
 pub type VoteAccountsHashMap = HashMap<Pubkey, (/*stake:*/ u64, VoteAccount)>;
@@ -60,16 +54,6 @@ pub struct VoteAccounts {
     staked_nodes_once: Once,
 }
 
-impl VoteAccounts {
-    pub fn num_vote_accounts(&self) -> usize {
-        self.vote_accounts.len()
-    }
-
-    pub fn num_staked_nodes(&self) -> usize {
-        self.staked_nodes().len()
-    }
-}
-
 impl VoteAccount {
     pub(crate) fn lamports(&self) -> u64 {
         self.0.account.lamports()
@@ -79,17 +63,16 @@ impl VoteAccount {
         self.0.account.owner()
     }
 
-    pub fn vote_state(&self) -> RwLockReadGuard<Result<VoteState, Error>> {
-        let inner = &self.0;
-        inner.vote_state_once.call_once(|| {
-            let vote_state = VoteState::deserialize(inner.account.data());
-            *inner.vote_state.write().unwrap() = vote_state.map_err(Error::from);
-        });
-        inner.vote_state.read().unwrap()
+    pub fn vote_state(&self) -> &Result<VoteState, Error> {
+        // VoteState::deserialize deserializes a VoteStateVersions and then
+        // calls VoteStateVersions::convert_to_current.
+        self.0
+            .vote_state
+            .get_or_init(|| VoteState::deserialize(self.0.account.data()).map_err(Error::from))
     }
 
     pub(crate) fn is_deserialized(&self) -> bool {
-        self.0.vote_state_once.is_completed()
+        self.0.vote_state.get().is_some()
     }
 
     /// VoteState.node_pubkey of this vote-account.
@@ -252,8 +235,7 @@ impl TryFrom<AccountSharedData> for VoteAccountInner {
         }
         Ok(Self {
             account,
-            vote_state: RwLock::new(INVALID_VOTE_STATE),
-            vote_state_once: Once::new(),
+            vote_state: OnceCell::new(),
         })
     }
 }
@@ -263,7 +245,6 @@ impl PartialEq<VoteAccountInner> for VoteAccountInner {
         let Self {
             account,
             vote_state: _,
-            vote_state_once: _,
         } = self;
         account == &other.account
     }