diff --git a/accounts-db/benches/bench_accounts_file.rs b/accounts-db/benches/bench_accounts_file.rs index 51830940c..fc450439f 100644 --- a/accounts-db/benches/bench_accounts_file.rs +++ b/accounts-db/benches/bench_accounts_file.rs @@ -2,13 +2,11 @@ use { criterion::{criterion_group, criterion_main, BatchSize, BenchmarkId, Criterion, Throughput}, solana_accounts_db::{ - account_storage::meta::StorableAccountsWithHashes, - accounts_hash::AccountHash, append_vec::{self, AppendVec}, tiered_storage::hot::HotStorageWriter, }, solana_sdk::{ - account::AccountSharedData, clock::Slot, hash::Hash, pubkey::Pubkey, + account::AccountSharedData, clock::Slot, pubkey::Pubkey, rent_collector::RENT_EXEMPT_RENT_EPOCH, }, }; @@ -45,11 +43,7 @@ fn bench_write_accounts_file(c: &mut Criterion) { .take(accounts_count) .collect(); let accounts_refs: Vec<_> = accounts.iter().collect(); - let accounts_data = (Slot::MAX, accounts_refs.as_slice()); - let storable_accounts = StorableAccountsWithHashes::new_with_hashes( - &accounts_data, - vec![AccountHash(Hash::default()); accounts_count], - ); + let storable_accounts = (Slot::MAX, accounts_refs.as_slice()); group.bench_function(BenchmarkId::new("append_vec", accounts_count), |b| { b.iter_batched_ref( @@ -59,9 +53,7 @@ fn bench_write_accounts_file(c: &mut Criterion) { AppendVec::new(path, true, file_size) }, |append_vec| { - let res = append_vec - .append_accounts(storable_accounts.accounts, 0) - .unwrap(); + let res = append_vec.append_accounts(&storable_accounts, 0).unwrap(); let accounts_written_count = res.len(); assert_eq!(accounts_written_count, accounts_count); }, @@ -79,9 +71,7 @@ fn bench_write_accounts_file(c: &mut Criterion) { HotStorageWriter::new(path).unwrap() }, |hot_storage| { - let res = hot_storage - .write_accounts(storable_accounts.accounts, 0) - .unwrap(); + let res = hot_storage.write_accounts(&storable_accounts, 0).unwrap(); let accounts_written_count = res.len(); assert_eq!(accounts_written_count, accounts_count); }, diff --git a/accounts-db/src/account_storage/meta.rs b/accounts-db/src/account_storage/meta.rs index 06595a504..5f9ec647a 100644 --- a/accounts-db/src/account_storage/meta.rs +++ b/accounts-db/src/account_storage/meta.rs @@ -3,11 +3,9 @@ use { account_info::AccountInfo, accounts_hash::AccountHash, append_vec::AppendVecStoredAccountMeta, - storable_accounts::{AccountForStorage, StorableAccounts}, tiered_storage::hot::{HotAccount, HotAccountMeta}, }, solana_sdk::{account::ReadableAccount, hash::Hash, pubkey::Pubkey, stake_history::Epoch}, - std::{borrow::Borrow, marker::PhantomData}, }; pub type StoredMetaWriteVersion = u64; @@ -22,82 +20,6 @@ lazy_static! { static ref DEFAULT_ACCOUNT_HASH: AccountHash = AccountHash(Hash::default()); } -/// Goal is to eliminate copies and data reshaping given various code paths that store accounts. -/// This struct contains what is needed to store accounts to a storage -/// 1. account & pubkey (StorableAccounts) -/// 2. hash per account (Maybe in StorableAccounts, otherwise has to be passed in separately) -pub struct StorableAccountsWithHashes<'a: 'b, 'b, U: StorableAccounts<'a>, V: Borrow> { - /// accounts to store - /// always has pubkey and account - /// may also have hash per account - pub accounts: &'b U, - /// if accounts does not have hash, this has a hash per account - hashes: Option>, - _phantom: PhantomData<&'a ()>, -} - -impl<'a: 'b, 'b, U: StorableAccounts<'a>, V: Borrow> - StorableAccountsWithHashes<'a, 'b, U, V> -{ - /// used when accounts contains hash already - pub fn new(accounts: &'b U) -> Self { - assert!(accounts.has_hash()); - Self { - accounts, - hashes: None, - _phantom: PhantomData, - } - } - /// used when accounts does NOT contains hash - /// In this case, hashes have to be passed in separately. - pub fn new_with_hashes(accounts: &'b U, hashes: Vec) -> Self { - assert!(!accounts.has_hash()); - assert_eq!(accounts.len(), hashes.len()); - Self { - accounts, - hashes: Some(hashes), - _phantom: PhantomData, - } - } - - /// get all account fields at 'index' - pub fn get( - &self, - index: usize, - mut callback: impl FnMut(AccountForStorage, &AccountHash) -> Ret, - ) -> Ret { - let hash = if self.accounts.has_hash() { - self.accounts.hash(index) - } else { - let item = self.hashes.as_ref().unwrap(); - item[index].borrow() - }; - self.accounts - .account_default_if_zero_lamport(index, |account| callback(account, hash)) - } - - /// None if account at index has lamports == 0 - /// Otherwise, Some(account) - /// This is the only way to access the account. - pub fn account( - &self, - index: usize, - callback: impl for<'local> FnMut(AccountForStorage<'local>) -> Ret, - ) -> Ret { - self.accounts - .account_default_if_zero_lamport(index, callback) - } - - /// # accounts to write - pub fn len(&self) -> usize { - self.accounts.len() - } - - pub fn is_empty(&self) -> bool { - self.len() == 0 - } -} - /// References to account data stored elsewhere. Getting an `Account` requires cloning /// (see `StoredAccountMeta::clone_account()`). #[derive(PartialEq, Eq, Debug)] diff --git a/accounts-db/src/append_vec.rs b/accounts-db/src/append_vec.rs index 56c4050d8..f0864fbf8 100644 --- a/accounts-db/src/append_vec.rs +++ b/accounts-db/src/append_vec.rs @@ -824,13 +824,11 @@ impl AppendVec { pub mod tests { use { super::{test_utils::*, *}, - crate::account_storage::meta::StorableAccountsWithHashes, assert_matches::assert_matches, memoffset::offset_of, rand::{thread_rng, Rng}, solana_sdk::{ - account::{accounts_equal, Account, AccountSharedData, WritableAccount}, - hash::Hash, + account::{Account, AccountSharedData, WritableAccount}, timing::duration_as_ms, }, std::{mem::ManuallyDrop, time::Instant}, @@ -896,113 +894,6 @@ pub mod tests { // Hash is [u8; 32], which has no alignment static_assertions::assert_eq_align!(u64, StoredMeta, AccountMeta); - #[test] - #[should_panic(expected = "accounts.has_hash()")] - fn test_storable_accounts_with_hashes_new() { - let account = AccountSharedData::default(); - // for (Slot, &'a [(&'a Pubkey, &'a T)]) - let slot = 0 as Slot; - let pubkey = Pubkey::default(); - StorableAccountsWithHashes::<'_, '_, _, &AccountHash>::new(&( - slot, - &[(&pubkey, &account)][..], - )); - } - - fn test_mismatch(correct_hashes: bool) { - let account = AccountSharedData::default(); - // for (Slot, &'a [(&'a Pubkey, &'a T)]) - let slot = 0 as Slot; - let pubkey = Pubkey::default(); - // mismatch between lens of accounts and hashes - let mut hashes = Vec::default(); - if correct_hashes { - hashes.push(AccountHash(Hash::default())); - } - StorableAccountsWithHashes::new_with_hashes(&(slot, &[(&pubkey, &account)][..]), hashes); - } - - #[test] - // rust 1.73+ (our as-of-writing nightly version) changed panic message. we're stuck with this - // short common substring until the monorepo is fully 1.73+ including stable. - #[should_panic(expected = "left == right")] - fn test_storable_accounts_with_hashes_new2() { - test_mismatch(false); - } - - #[test] - fn test_storable_accounts_with_hashes_empty() { - // for (Slot, &'a [(&'a Pubkey, &'a T)]) - let account = AccountSharedData::default(); - let slot = 0 as Slot; - let pubkeys = [Pubkey::default()]; - let hashes = Vec::::default(); - let mut accounts = vec![(&pubkeys[0], &account)]; - accounts.clear(); - let accounts2 = (slot, &accounts[..]); - let storable = StorableAccountsWithHashes::new_with_hashes(&accounts2, hashes); - assert_eq!(storable.len(), 0); - assert!(storable.is_empty()); - } - - #[test] - fn test_storable_accounts_with_hashes_hash() { - // for (Slot, &'a [(&'a Pubkey, &'a T)]) - let account = AccountSharedData::default(); - let slot = 0 as Slot; - let pubkeys = [Pubkey::from([5; 32]), Pubkey::from([6; 32])]; - let hashes = vec![ - AccountHash(Hash::new(&[3; 32])), - AccountHash(Hash::new(&[4; 32])), - ]; - let accounts = [(&pubkeys[0], &account), (&pubkeys[1], &account)]; - let accounts2 = (slot, &accounts[..]); - let storable = StorableAccountsWithHashes::new_with_hashes(&accounts2, hashes.clone()); - assert_eq!(storable.len(), pubkeys.len()); - assert!(!storable.is_empty()); - (0..2).for_each(|i| { - storable.get(i, |account, hash| { - assert_eq!(hash, &hashes[i]); - assert_eq!(account.pubkey(), &pubkeys[i]); - }); - }); - } - - #[test] - fn test_storable_accounts_with_hashes_default() { - // 0 lamport account, should return default account (or None in this case) - let account = Account { - data: vec![0], - ..Account::default() - } - .to_account_shared_data(); - // for (Slot, &'a [(&'a Pubkey, &'a T)]) - let slot = 0 as Slot; - let pubkey = Pubkey::default(); - let hashes = vec![AccountHash(Hash::default())]; - let accounts = [(&pubkey, &account)]; - let accounts2 = (slot, &accounts[..]); - let storable = StorableAccountsWithHashes::new_with_hashes(&accounts2, hashes.clone()); - storable.account(0, |get_account| { - assert!(accounts_equal(&get_account, &AccountSharedData::default())); - }); - - // non-zero lamports, data should be correct - let account = Account { - lamports: 1, - data: vec![0], - ..Account::default() - } - .to_account_shared_data(); - // for (Slot, &'a [(&'a Pubkey, &'a T)]) - let accounts = [(&pubkey, &account)]; - let accounts2 = (slot, &accounts[..]); - let storable = StorableAccountsWithHashes::new_with_hashes(&accounts2, hashes); - storable.account(0, |get_account| { - assert!(accounts_equal(&account, &get_account)); - }); - } - #[test] fn test_account_meta_default() { let def1 = AccountMeta::default();