diff --git a/accounts-db/src/account_storage.rs b/accounts-db/src/account_storage.rs index 92ea5a7bcbdce9..89c2b3a138ab41 100644 --- a/accounts-db/src/account_storage.rs +++ b/accounts-db/src/account_storage.rs @@ -10,16 +10,7 @@ use { pub mod meta; -#[derive(Clone, Debug)] -pub struct AccountStorageReference { - /// the single storage for a given slot - pub storage: Arc, - /// id can be read from 'storage', but it is an atomic read. - /// id will never change while a storage is held, so we store it separately here for faster runtime lookup in 'get_account_storage_entry' - pub id: AccountsFileId, -} - -pub type AccountStorageMap = DashMap>; +pub type AccountStorageMap = DashMap, BuildNoHashHasher>; #[derive(Default, Debug)] pub struct AccountStorage { @@ -28,7 +19,7 @@ pub struct AccountStorage { /// while shrink is operating on a slot, there can be 2 append vecs active for that slot /// Once the index has been updated to only refer to the new append vec, the single entry for the slot in 'map' can be updated. /// Entries in 'shrink_in_progress_map' can be found by 'get_account_storage_entry' - shrink_in_progress_map: DashMap, BuildNoHashHasher>, + shrink_in_progress_map: AccountStorageMap, } impl AccountStorage { @@ -56,9 +47,9 @@ impl AccountStorage { store_id: AccountsFileId, ) -> Option> { let lookup_in_map = || { - self.map - .get(&slot) - .and_then(|r| (r.id == store_id).then_some(Arc::clone(&r.storage))) + self.map.get(&slot).and_then(|entry| { + (entry.value().id() == store_id).then_some(Arc::clone(entry.value())) + }) }; lookup_in_map() @@ -92,8 +83,8 @@ impl AccountStorage { ) { assert_eq!(storage.slot(), slot); if let Some(mut existing_storage) = self.map.get_mut(&slot) { - assert_eq!(slot, existing_storage.value().storage.slot()); - existing_storage.value_mut().storage = storage; + assert_eq!(slot, existing_storage.value().slot()); + *existing_storage.value_mut() = storage; } } @@ -102,7 +93,7 @@ impl AccountStorage { &self, slot: Slot, ) -> Option> { - self.map.get(&slot).map(|entry| Arc::clone(&entry.storage)) + self.map.get(&slot).map(|entry| Arc::clone(entry.value())) } pub(crate) fn all_slots(&self) -> Vec { @@ -135,7 +126,7 @@ impl AccountStorage { shrink_can_be_active: bool, ) -> Option> { assert!(shrink_can_be_active || self.shrink_in_progress_map.is_empty()); - self.map.remove(slot).map(|(_, entry)| entry.storage) + self.map.remove(slot).map(|(_, storage)| storage) } /// iterate through all (slot, append-vec) @@ -149,16 +140,7 @@ impl AccountStorage { self.no_shrink_in_progress(), "self.no_shrink_in_progress(): {slot}" ); - assert!(self - .map - .insert( - slot, - AccountStorageReference { - id: store.id(), - storage: store, - } - ) - .is_none()); + assert!(self.map.insert(slot, store).is_none()); } /// called when shrinking begins on a slot and append vec. @@ -173,12 +155,10 @@ impl AccountStorage { new_store: Arc, ) -> ShrinkInProgress<'_> { let shrinking_store = Arc::clone( - &self - .map + self.map .get(&slot) .expect("no pre-existing storage for shrinking slot") - .value() - .storage, + .value(), ); // insert 'new_store' into 'shrink_in_progress_map' @@ -219,7 +199,7 @@ impl AccountStorage { .iter() .filter_map(|entry| { let slot = entry.key(); - let storage = &entry.value().storage; + let storage = entry.value(); predicate(slot, storage).then(|| (*slot, Arc::clone(storage))) }) .collect() @@ -228,7 +208,7 @@ impl AccountStorage { /// iterate contents of AccountStorage without exposing internals pub struct AccountStorageIter<'a> { - iter: dashmap::iter::Iter<'a, Slot, AccountStorageReference, BuildNoHashHasher>, + iter: dashmap::iter::Iter<'a, Slot, Arc, BuildNoHashHasher>, } impl<'a> AccountStorageIter<'a> { @@ -246,7 +226,7 @@ impl Iterator for AccountStorageIter<'_> { if let Some(entry) = self.iter.next() { let slot = entry.key(); let store = entry.value(); - return Some((*slot, Arc::clone(&store.storage))); + return Some((*slot, Arc::clone(store))); } None } @@ -270,14 +250,8 @@ impl Drop for ShrinkInProgress<'_> { assert_eq!( self.storage .map - .insert( - self.slot, - AccountStorageReference { - storage: Arc::clone(&self.new_store), - id: self.new_store.id() - } - ) - .map(|store| store.id), + .insert(self.slot, Arc::clone(&self.new_store)) + .map(|store| store.id()), Some(self.old_store.id()) ); @@ -349,9 +323,7 @@ pub(crate) mod tests { store_file_size2, AccountsFileProvider::AppendVec, )); - storage - .map - .insert(slot, AccountStorageReference { id, storage: entry }); + storage.map.insert(slot, entry); // look in map assert_eq!( @@ -471,13 +443,7 @@ pub(crate) mod tests { // already entry in shrink_in_progress_map let storage = AccountStorage::default(); let sample = storage.get_test_storage(); - storage.map.insert( - 0, - AccountStorageReference { - id: 0, - storage: sample.clone(), - }, - ); + storage.map.insert(0, sample.clone()); storage.shrink_in_progress_map.insert(0, sample.clone()); storage.shrinking_in_progress(0, sample); } @@ -489,13 +455,7 @@ pub(crate) mod tests { let storage = AccountStorage::default(); let sample_to_shrink = storage.get_test_storage(); let sample = storage.get_test_storage(); - storage.map.insert( - 0, - AccountStorageReference { - id: 0, - storage: sample_to_shrink, - }, - ); + storage.map.insert(0, sample_to_shrink); let _shrinking_in_progress = storage.shrinking_in_progress(0, sample.clone()); storage.shrinking_in_progress(0, sample); } @@ -510,16 +470,10 @@ pub(crate) mod tests { let id_shrunk = 0; let sample_to_shrink = storage.get_test_storage_with_id(id_to_shrink); let sample = storage.get_test_storage(); - storage.map.insert( - slot, - AccountStorageReference { - id: id_to_shrink, - storage: sample_to_shrink, - }, - ); + storage.map.insert(slot, sample_to_shrink); let shrinking_in_progress = storage.shrinking_in_progress(slot, sample.clone()); assert!(storage.map.contains_key(&slot)); - assert_eq!(id_to_shrink, storage.map.get(&slot).unwrap().storage.id()); + assert_eq!(id_to_shrink, storage.map.get(&slot).unwrap().id()); assert_eq!( (slot, id_shrunk), storage @@ -531,7 +485,7 @@ pub(crate) mod tests { ); drop(shrinking_in_progress); assert!(storage.map.contains_key(&slot)); - assert_eq!(id_shrunk, storage.map.get(&slot).unwrap().storage.id()); + assert_eq!(id_shrunk, storage.map.get(&slot).unwrap().id()); assert!(storage.shrink_in_progress_map.is_empty()); storage.shrinking_in_progress(slot, sample); } @@ -569,13 +523,7 @@ pub(crate) mod tests { assert!(storage .get_account_storage_entry(slot, missing_id) .is_none()); - storage.map.insert( - slot, - AccountStorageReference { - id, - storage: sample.clone(), - }, - ); + storage.map.insert(slot, sample.clone()); // id is found in map assert!(storage.get_account_storage_entry(slot, id).is_some()); assert!(storage @@ -613,13 +561,7 @@ pub(crate) mod tests { 5000, AccountsFileProvider::AppendVec, ); - storage.map.insert( - slot, - AccountStorageReference { - id, - storage: entry.into(), - }, - ); + storage.map.insert(slot, entry.into()); } // look 'em up diff --git a/runtime/src/bank/serde_snapshot.rs b/runtime/src/bank/serde_snapshot.rs index 430c86ddf89d78..c359de7561d63f 100644 --- a/runtime/src/bank/serde_snapshot.rs +++ b/runtime/src/bank/serde_snapshot.rs @@ -22,7 +22,7 @@ mod tests { stakes::{SerdeStakesToStakeFormat, Stakes, StakesEnum}, }, solana_accounts_db::{ - account_storage::{AccountStorageMap, AccountStorageReference}, + account_storage::AccountStorageMap, accounts_db::{ get_temp_accounts_paths, AccountStorageEntry, AccountsDb, AccountsDbConfig, AtomicAccountsFileId, ACCOUNTS_DB_CONFIG_FOR_TESTING, @@ -79,13 +79,7 @@ mod tests { num_accounts, ); next_append_vec_id = next_append_vec_id.max(new_storage_entry.id()); - storage.insert( - new_storage_entry.slot(), - AccountStorageReference { - id: new_storage_entry.id(), - storage: Arc::new(new_storage_entry), - }, - ); + storage.insert(new_storage_entry.slot(), Arc::new(new_storage_entry)); } Ok(StorageAndNextAccountsFileId { diff --git a/runtime/src/serde_snapshot/tests.rs b/runtime/src/serde_snapshot/tests.rs index 0cede18b3094f8..9dd997100d93b4 100644 --- a/runtime/src/serde_snapshot/tests.rs +++ b/runtime/src/serde_snapshot/tests.rs @@ -13,7 +13,7 @@ mod serde_snapshot_tests { log::info, rand::{thread_rng, Rng}, solana_accounts_db::{ - account_storage::{AccountStorageMap, AccountStorageReference}, + account_storage::AccountStorageMap, accounts::Accounts, accounts_db::{ get_temp_accounts_paths, test_utils::create_test_accounts, AccountStorageEntry, @@ -159,13 +159,7 @@ mod serde_snapshot_tests { num_accounts, ); next_append_vec_id = next_append_vec_id.max(new_storage_entry.id()); - storage.insert( - new_storage_entry.slot(), - AccountStorageReference { - id: new_storage_entry.id(), - storage: Arc::new(new_storage_entry), - }, - ); + storage.insert(new_storage_entry.slot(), Arc::new(new_storage_entry)); } Ok(StorageAndNextAccountsFileId { diff --git a/runtime/src/snapshot_utils/snapshot_storage_rebuilder.rs b/runtime/src/snapshot_utils/snapshot_storage_rebuilder.rs index 74e7ae6e98720d..e94b8f1884794f 100644 --- a/runtime/src/snapshot_utils/snapshot_storage_rebuilder.rs +++ b/runtime/src/snapshot_utils/snapshot_storage_rebuilder.rs @@ -15,8 +15,8 @@ use { }, regex::Regex, solana_accounts_db::{ - account_storage::{AccountStorageMap, AccountStorageReference}, - accounts_db::{AccountStorageEntry, AccountsFileId, AtomicAccountsFileId}, + account_storage::AccountStorageMap, + accounts_db::{AccountsFileId, AtomicAccountsFileId}, accounts_file::StorageAccess, }, solana_nohash_hasher::BuildNoHashHasher, @@ -342,10 +342,9 @@ impl SnapshotStorageRebuilder { )?, }; - Ok((storage_entry.id(), storage_entry)) + Ok(storage_entry) }) - .collect::>, SnapshotError>>( - )?; + .collect::, SnapshotError>>()?; if slot_stores.len() != 1 { return Err(SnapshotError::RebuildStorages(format!( @@ -355,10 +354,9 @@ impl SnapshotStorageRebuilder { } // SAFETY: The check above guarantees there is one item in slot_stores, // so `.next()` will always return `Some` - let (id, storage) = slot_stores.into_iter().next().unwrap(); + let storage = slot_stores.into_iter().next().unwrap(); - self.storage - .insert(slot, AccountStorageReference { id, storage }); + self.storage.insert(slot, storage); Ok(()) }