From 0f16120b0342645a9ea93f490681ec3a0638d66b Mon Sep 17 00:00:00 2001 From: "Jeff Washington (jwash)" Date: Tue, 10 Jan 2023 15:57:34 -0600 Subject: [PATCH] remove FoundStoredAccount (#29591) --- runtime/src/accounts_db.rs | 104 ++++++----------------------- runtime/src/ancient_append_vecs.rs | 17 +++-- runtime/src/snapshot_minimizer.rs | 8 +-- runtime/src/storable_accounts.rs | 56 +++------------- 4 files changed, 43 insertions(+), 142 deletions(-) diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index 46885b2934..505938f956 100644 --- a/runtime/src/accounts_db.rs +++ b/runtime/src/accounts_db.rs @@ -333,7 +333,7 @@ struct ShrinkCollect<'a> { original_bytes: u64, aligned_total_bytes: u64, unrefed_pubkeys: Vec<&'a Pubkey>, - alive_accounts: Vec<&'a FoundStoredAccount<'a>>, + alive_accounts: Vec<&'a StoredAccountMeta<'a>>, /// total size in storage of all alive accounts alive_total_bytes: usize, total_starting_accounts: usize, @@ -366,7 +366,7 @@ struct LoadAccountsIndexForShrink<'a> { /// total stored bytes for all alive accounts alive_total_bytes: usize, /// the specific alive accounts - alive_accounts: Vec<&'a FoundStoredAccount<'a>>, + alive_accounts: Vec<&'a StoredAccountMeta<'a>>, /// pubkeys that were unref'd in the accounts index because they were dead unrefed_pubkeys: Vec<&'a Pubkey>, /// true if all alive accounts are zero lamport accounts @@ -374,7 +374,7 @@ struct LoadAccountsIndexForShrink<'a> { } pub struct GetUniqueAccountsResult<'a> { - pub stored_accounts: Vec>, + pub stored_accounts: Vec>, pub original_bytes: u64, } @@ -419,54 +419,6 @@ pub struct AccountsDbConfig { pub exhaustively_verify_refcounts: bool, } -pub struct FoundStoredAccount<'a> { - pub account: StoredAccountMeta<'a>, -} - -impl<'a> FoundStoredAccount<'a> { - pub fn pubkey(&self) -> &Pubkey { - self.account.pubkey() - } -} - -/// this tuple assumes storing from a slot to the same slot -/// accounts are `StoredAccountMeta` inside `FoundStoredAccount` -impl<'a> StorableAccounts<'a, StoredAccountMeta<'a>> - for (Slot, &'a [&FoundStoredAccount<'a>], IncludeSlotInHash) -{ - fn pubkey(&self, index: usize) -> &Pubkey { - self.1[index].pubkey() - } - fn account(&self, index: usize) -> &StoredAccountMeta<'a> { - &self.1[index].account - } - fn slot(&self, _index: usize) -> Slot { - // same other slot for all accounts - self.0 - } - fn target_slot(&self) -> Slot { - self.0 - } - fn len(&self) -> usize { - self.1.len() - } - fn contains_multiple_slots(&self) -> bool { - false - } - fn include_slot_in_hash(&self) -> IncludeSlotInHash { - self.2 - } - fn has_hash_and_write_version(&self) -> bool { - true - } - fn hash(&self, index: usize) -> &Hash { - self.1[index].account.hash - } - fn write_version(&self, index: usize) -> u64 { - self.1[index].account.meta.write_version_obsolete - } -} - #[cfg(not(test))] const ABSURD_CONSECUTIVE_FAILED_ITERATIONS: usize = 100; @@ -3608,7 +3560,7 @@ impl AccountsDb { /// return sum of account size for all alive accounts fn load_accounts_index_for_shrink<'a>( &'a self, - accounts: &'a [FoundStoredAccount<'a>], + accounts: &'a [StoredAccountMeta<'a>], stats: &ShrinkStats, slot_to_shrink: Slot, ) -> LoadAccountsIndexForShrink<'a> { @@ -3641,9 +3593,9 @@ impl AccountsDb { result = AccountsIndexScanResult::Unref; dead += 1; } else { - all_are_zero_lamports &= stored_account.account.lamports() == 0; + all_are_zero_lamports &= stored_account.lamports() == 0; alive_accounts.push(stored_account); - alive_total_bytes += stored_account.account.stored_size; + alive_total_bytes += stored_account.stored_size; alive += 1; } } @@ -3670,20 +3622,19 @@ impl AccountsDb { &self, store: &'a Arc, ) -> GetUniqueAccountsResult<'a> { - let mut stored_accounts: HashMap = HashMap::new(); + let mut stored_accounts: HashMap = HashMap::new(); let original_bytes = store.total_bytes(); store.accounts.account_iter().for_each(|account| { - let new_entry = FoundStoredAccount { account }; - match stored_accounts.entry(*new_entry.account.pubkey()) { + match stored_accounts.entry(*account.pubkey()) { Entry::Occupied(mut occupied_entry) => { assert!( - new_entry.account.meta.write_version_obsolete - > occupied_entry.get().account.meta.write_version_obsolete + account.meta.write_version_obsolete + > occupied_entry.get().meta.write_version_obsolete ); - occupied_entry.insert(new_entry); + occupied_entry.insert(account); } Entry::Vacant(vacant_entry) => { - vacant_entry.insert(new_entry); + vacant_entry.insert(account); } } }); @@ -3703,7 +3654,7 @@ impl AccountsDb { fn shrink_collect<'a: 'b, 'b>( &'a self, store: &'a Arc, - stored_accounts: &'b mut Vec>, + stored_accounts: &'b mut Vec>, stats: &ShrinkStats, ) -> ShrinkCollect<'b> { let ( @@ -4225,7 +4176,7 @@ impl AccountsDb { /// returns the pubkeys that are in 'accounts' that are already in 'existing_ancient_pubkeys' /// Also updated 'existing_ancient_pubkeys' to include all pubkeys in 'accounts' since they will soon be written into the ancient slot. fn get_keys_to_unref_ancient<'a>( - accounts: &'a [&FoundStoredAccount<'_>], + accounts: &'a [&StoredAccountMeta<'_>], existing_ancient_pubkeys: &mut HashSet, ) -> HashSet<&'a Pubkey> { let mut unref = HashSet::<&Pubkey>::default(); @@ -4247,7 +4198,7 @@ impl AccountsDb { /// As a side effect, on exit, 'existing_ancient_pubkeys' will now contain all pubkeys in 'accounts'. fn unref_accounts_already_in_storage( &self, - accounts: &[&FoundStoredAccount<'_>], + accounts: &[&StoredAccountMeta<'_>], existing_ancient_pubkeys: &mut HashSet, ) { let unref = Self::get_keys_to_unref_ancient(accounts, existing_ancient_pubkeys); @@ -9506,9 +9457,8 @@ pub mod tests { stored_size: account_size, hash: &hash, }; - let found = FoundStoredAccount { account }; - let map = vec![&found]; - let alive_total_bytes = found.account.stored_size; + let map = vec![&account]; + let alive_total_bytes = account.stored_size; let to_store = AccountsToStore::new(available_bytes, &map, alive_total_bytes, slot0); // Done: setup 'to_store' @@ -9626,20 +9576,8 @@ pub mod tests { stored_size, hash: &hash, }; - let found_account = FoundStoredAccount { - account: stored_account, - }; - let found_account2 = FoundStoredAccount { - account: stored_account2, - }; - let found_account3 = FoundStoredAccount { - account: stored_account3, - }; - let found_account4 = FoundStoredAccount { - account: stored_account4, - }; let mut existing_ancient_pubkeys = HashSet::default(); - let accounts = [&found_account]; + let accounts = [&stored_account]; // pubkey NOT in existing_ancient_pubkeys, so do NOT unref, but add to existing_ancient_pubkeys let unrefs = AccountsDb::get_keys_to_unref_ancient(&accounts, &mut existing_ancient_pubkeys); @@ -9657,7 +9595,7 @@ pub mod tests { ); assert_eq!(unrefs.iter().cloned().collect::>(), vec![&pubkey]); // pubkey2 NOT in existing_ancient_pubkeys, so do NOT unref, but add to existing_ancient_pubkeys - let accounts = [&found_account2]; + let accounts = [&stored_account2]; let unrefs = AccountsDb::get_keys_to_unref_ancient(&accounts, &mut existing_ancient_pubkeys); assert!(unrefs.is_empty()); @@ -9680,7 +9618,7 @@ pub mod tests { ); assert_eq!(unrefs.iter().cloned().collect::>(), vec![&pubkey2]); // pubkey3/4 NOT in existing_ancient_pubkeys, so do NOT unref, but add to existing_ancient_pubkeys - let accounts = [&found_account3, &found_account4]; + let accounts = [&stored_account3, &stored_account4]; let unrefs = AccountsDb::get_keys_to_unref_ancient(&accounts, &mut existing_ancient_pubkeys); assert!(unrefs.is_empty()); @@ -17415,7 +17353,7 @@ pub mod tests { .enumerate() .find_map(|(i, stored_ancient)| { (stored_ancient.pubkey() == original.pubkey()).then_some({ - assert!(accounts_equal(&stored_ancient.account, &original)); + assert!(accounts_equal(stored_ancient, &original)); i }) }) diff --git a/runtime/src/ancient_append_vecs.rs b/runtime/src/ancient_append_vecs.rs index 00bd7d8e17..959148f3fb 100644 --- a/runtime/src/ancient_append_vecs.rs +++ b/runtime/src/ancient_append_vecs.rs @@ -4,7 +4,7 @@ //! 2. multiple 'slots' squashed into a single older (ie. ancient) slot for convenience and performance //! Otherwise, an ancient append vec is the same as any other append vec use { - crate::{accounts_db::FoundStoredAccount, append_vec::AppendVec}, + crate::append_vec::{AppendVec, StoredAccountMeta}, solana_sdk::clock::Slot, }; @@ -22,7 +22,7 @@ pub enum StorageSelector { /// We need 1-2 of these slices constructed based on available bytes and individual account sizes. /// The slice arithmetic accross both hashes and account data gets messy. So, this struct abstracts that. pub struct AccountsToStore<'a> { - accounts: &'a [&'a FoundStoredAccount<'a>], + accounts: &'a [&'a StoredAccountMeta<'a>], /// if 'accounts' contains more items than can be contained in the primary storage, then we have to split these accounts. /// 'index_first_item_overflow' specifies the index of the first item in 'accounts' that will go into the overflow storage index_first_item_overflow: usize, @@ -34,7 +34,7 @@ impl<'a> AccountsToStore<'a> { /// available_bytes: how many bytes remain in the primary storage. Excess accounts will be directed to an overflow storage pub fn new( mut available_bytes: u64, - accounts: &'a [&'a FoundStoredAccount<'a>], + accounts: &'a [&'a StoredAccountMeta<'a>], alive_total_bytes: usize, slot: Slot, ) -> Self { @@ -44,7 +44,7 @@ impl<'a> AccountsToStore<'a> { if alive_total_bytes > available_bytes as usize { // not all the alive bytes fit, so we have to find how many accounts fit within available_bytes for (i, account) in accounts.iter().enumerate() { - let account_size = account.account.stored_size as u64; + let account_size = account.stored_size as u64; if available_bytes >= account_size { available_bytes = available_bytes.saturating_sub(account_size); } else if index_first_item_overflow == num_accounts { @@ -67,7 +67,7 @@ impl<'a> AccountsToStore<'a> { } /// get the accounts to store in the given 'storage' - pub fn get(&self, storage: StorageSelector) -> &[&'a FoundStoredAccount<'a>] { + pub fn get(&self, storage: StorageSelector) -> &[&'a StoredAccountMeta<'a>] { let range = match storage { StorageSelector::Primary => 0..self.index_first_item_overflow, StorageSelector::Overflow => self.index_first_item_overflow..self.accounts.len(), @@ -148,8 +148,7 @@ pub mod tests { stored_size: account_size, hash: &hash, }; - let found = FoundStoredAccount { account }; - let map = vec![&found]; + let map = vec![&account]; for (selector, available_bytes) in [ (StorageSelector::Primary, account_size), (StorageSelector::Overflow, account_size - 1), @@ -160,8 +159,8 @@ pub mod tests { AccountsToStore::new(available_bytes as u64, &map, alive_total_bytes, slot); let accounts = accounts_to_store.get(selector); assert_eq!( - accounts.iter().map(|b| &b.account).collect::>(), - map.iter().map(|b| &b.account).collect::>(), + accounts.iter().collect::>(), + map.iter().collect::>(), "mismatch" ); let accounts = accounts_to_store.get(get_opposite(&selector)); diff --git a/runtime/src/snapshot_minimizer.rs b/runtime/src/snapshot_minimizer.rs index 732cc43fa9..c3cfda2ea4 100644 --- a/runtime/src/snapshot_minimizer.rs +++ b/runtime/src/snapshot_minimizer.rs @@ -317,7 +317,7 @@ impl<'a> SnapshotMinimizer<'a> { let mut purge_pubkeys = Vec::with_capacity(CHUNK_SIZE); chunk.iter().for_each(|account| { if self.minimized_account_set.contains(account.pubkey()) { - chunk_bytes += account.account.stored_size; + chunk_bytes += account.stored_size; keep_accounts.push(account); } else if self .accounts_db() @@ -358,9 +358,9 @@ impl<'a> SnapshotMinimizer<'a> { let mut write_versions = Vec::with_capacity(keep_accounts.len()); for alive_account in keep_accounts { - accounts.push(&alive_account.account); - hashes.push(alive_account.account.hash); - write_versions.push(alive_account.account.meta.write_version_obsolete); + accounts.push(alive_account); + hashes.push(alive_account.hash); + write_versions.push(alive_account.meta.write_version_obsolete); } shrink_in_progress = Some(self.accounts_db().get_store_for_shrink(slot, aligned_total)); diff --git a/runtime/src/storable_accounts.rs b/runtime/src/storable_accounts.rs index cb00575407..34fb3eb566 100644 --- a/runtime/src/storable_accounts.rs +++ b/runtime/src/storable_accounts.rs @@ -1,9 +1,6 @@ //! trait for abstracting underlying storage of pubkey and account pairs to be written use { - crate::{ - accounts_db::{FoundStoredAccount, IncludeSlotInHash}, - append_vec::StoredAccountMeta, - }, + crate::{accounts_db::IncludeSlotInHash, append_vec::StoredAccountMeta}, solana_sdk::{account::ReadableAccount, clock::Slot, hash::Hash, pubkey::Pubkey}, }; @@ -154,6 +151,15 @@ impl<'a> StorableAccounts<'a, StoredAccountMeta<'a>> fn include_slot_in_hash(&self) -> IncludeSlotInHash { self.2 } + fn has_hash_and_write_version(&self) -> bool { + true + } + fn hash(&self, index: usize) -> &Hash { + self.1[index].hash + } + fn write_version(&self, index: usize) -> u64 { + self.1[index].meta.write_version_obsolete + } } /// this tuple contains a single different source slot that applies to all accounts @@ -199,48 +205,6 @@ impl<'a> StorableAccounts<'a, StoredAccountMeta<'a>> } } -/// this tuple contains a single different source slot that applies to all accounts -/// accounts are FoundStoredAccount -impl<'a> StorableAccounts<'a, StoredAccountMeta<'a>> - for ( - Slot, - &'a [&'a FoundStoredAccount<'a>], - IncludeSlotInHash, - Slot, - ) -{ - fn pubkey(&self, index: usize) -> &Pubkey { - self.1[index].pubkey() - } - fn account(&self, index: usize) -> &StoredAccountMeta<'a> { - &self.1[index].account - } - fn slot(&self, _index: usize) -> Slot { - // same other slot for all accounts - self.3 - } - fn target_slot(&self) -> Slot { - self.0 - } - fn len(&self) -> usize { - self.1.len() - } - fn contains_multiple_slots(&self) -> bool { - false - } - fn include_slot_in_hash(&self) -> IncludeSlotInHash { - self.2 - } - fn has_hash_and_write_version(&self) -> bool { - true - } - fn hash(&self, index: usize) -> &Hash { - self.1[index].account.hash - } - fn write_version(&self, index: usize) -> u64 { - self.1[index].account.meta.write_version_obsolete - } -} #[cfg(test)] pub mod tests { use {