Removes StorableAccountsWithHashes (#862)

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 = (Slot::MAX, accounts_refs.as_slice());
-        let storable_accounts = StorableAccountsWithHashes::new_with_hashes(
-            &accounts_data,
-            vec![AccountHash(Hash::default()); accounts_count],
-        );
 
         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
+                    let res = append_vec.append_accounts(&storable_accounts, 0).unwrap();
-                        .append_accounts(storable_accounts.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
+                    let res = hot_storage.write_accounts(&storable_accounts, 0).unwrap();
-                        .write_accounts(storable_accounts.accounts, 0)
-                        .unwrap();
                     let accounts_written_count = res.len();
                     assert_eq!(accounts_written_count, accounts_count);
                 },

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<AccountHash>> {
-    /// 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<Vec<V>>,
-    _phantom: PhantomData<&'a ()>,
-}
-
-impl<'a: 'b, 'b, U: StorableAccounts<'a>, V: Borrow<AccountHash>>
-    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<V>) -> 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<Ret>(
-        &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<Ret>(
-        &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)]

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},
+            account::{Account, AccountSharedData, WritableAccount},
-            hash::Hash,
             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::<AccountHash>::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();