Skip to content

Removes StorableAccountsWithHashes #862

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mergify merged 1 commit into from
Apr 17, 2024
Merged

Removes StorableAccountsWithHashes #862

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 4 additions & 14 deletions accounts-db/benches/bench_accounts_file.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
};
Expand Down Expand Up @@ -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(
Expand All @@ -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);
},
Expand All @@ -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);
},
Expand Down
78 changes: 0 additions & 78 deletions accounts-db/src/account_storage/meta.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)]
Expand Down
111 changes: 1 addition & 110 deletions accounts-db/src/append_vec.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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();
Expand Down