pass &Pubkey through account storage, slot clean code to reduce copies (#16778)

* &Pubkey

* use trait to pass &Hash or Hash

* use impl Borrow<Hash> instead of trait

* remove old code line commented out
This commit is contained in:
Jeff Washington (jwash) 2021-04-27 09:10:06 -05:00 committed by GitHub
parent 3b8d6b59fb
commit 81402ee7f5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 22 additions and 20 deletions

View File

@ -6,6 +6,7 @@ use solana_sdk::{
pubkey::Pubkey, pubkey::Pubkey,
}; };
use std::{ use std::{
borrow::Borrow,
collections::BTreeSet, collections::BTreeSet,
ops::Deref, ops::Deref,
sync::{ sync::{
@ -51,7 +52,7 @@ impl SlotCacheInner {
&self, &self,
pubkey: &Pubkey, pubkey: &Pubkey,
account: AccountSharedData, account: AccountSharedData,
hash: Option<Hash>, hash: Option<impl Borrow<Hash>>,
slot: Slot, slot: Slot,
) -> CachedAccount { ) -> CachedAccount {
if self.cache.contains_key(pubkey) { if self.cache.contains_key(pubkey) {
@ -64,7 +65,7 @@ impl SlotCacheInner {
} }
let item = Arc::new(CachedAccountInner { let item = Arc::new(CachedAccountInner {
account, account,
hash: RwLock::new(hash), hash: RwLock::new(hash.map(|h| *h.borrow())),
slot, slot,
pubkey: *pubkey, pubkey: *pubkey,
}); });
@ -169,7 +170,7 @@ impl AccountsCache {
slot: Slot, slot: Slot,
pubkey: &Pubkey, pubkey: &Pubkey,
account: AccountSharedData, account: AccountSharedData,
hash: Option<Hash>, hash: Option<impl Borrow<Hash>>,
) -> CachedAccount { ) -> CachedAccount {
let slot_cache = self.slot_cache(slot).unwrap_or_else(|| let slot_cache = self.slot_cache(slot).unwrap_or_else(||
// DashMap entry.or_insert() returns a RefMut, essentially a write lock, // DashMap entry.or_insert() returns a RefMut, essentially a write lock,
@ -283,7 +284,7 @@ pub mod tests {
inserted_slot, inserted_slot,
&Pubkey::new_unique(), &Pubkey::new_unique(),
AccountSharedData::new(1, 0, &Pubkey::default()), AccountSharedData::new(1, 0, &Pubkey::default()),
Some(Hash::default()), Some(&Hash::default()),
); );
// If the cache is told the size limit is 0, it should return the one slot // If the cache is told the size limit is 0, it should return the one slot
let removed = cache.remove_slots_le(0); let removed = cache.remove_slots_le(0);
@ -301,7 +302,7 @@ pub mod tests {
inserted_slot, inserted_slot,
&Pubkey::new_unique(), &Pubkey::new_unique(),
AccountSharedData::new(1, 0, &Pubkey::default()), AccountSharedData::new(1, 0, &Pubkey::default()),
Some(Hash::default()), Some(&Hash::default()),
); );
// If the cache is told the size limit is 0, it should return nothing because there's only // If the cache is told the size limit is 0, it should return nothing because there's only

View File

@ -51,7 +51,7 @@ use solana_sdk::{
}; };
use solana_vote_program::vote_state::MAX_LOCKOUT_HISTORY; use solana_vote_program::vote_state::MAX_LOCKOUT_HISTORY;
use std::{ use std::{
borrow::Cow, borrow::{Borrow, Cow},
boxed::Box, boxed::Box,
collections::{hash_map::Entry, BTreeMap, BTreeSet, HashMap, HashSet}, collections::{hash_map::Entry, BTreeMap, BTreeSet, HashMap, HashSet},
convert::TryFrom, convert::TryFrom,
@ -1937,7 +1937,7 @@ impl AccountsDb {
for (_pubkey, alive_account) in alive_accounts.iter() { for (_pubkey, alive_account) in alive_accounts.iter() {
stored_accounts.push(alive_account.account.clone_account()); stored_accounts.push(alive_account.account.clone_account());
hashes.push(*alive_account.account.hash); hashes.push(alive_account.account.hash);
write_versions.push(alive_account.account.meta.write_version); write_versions.push(alive_account.account.meta.write_version);
} }
let accounts = alive_accounts let accounts = alive_accounts
@ -3308,7 +3308,7 @@ impl AccountsDb {
fn write_accounts_to_storage<F: FnMut(Slot, usize) -> Arc<AccountStorageEntry>>( fn write_accounts_to_storage<F: FnMut(Slot, usize) -> Arc<AccountStorageEntry>>(
&self, &self,
slot: Slot, slot: Slot,
hashes: &[Hash], hashes: &[impl Borrow<Hash>],
mut storage_finder: F, mut storage_finder: F,
accounts_and_meta_to_store: &[(StoredMeta, &AccountSharedData)], accounts_and_meta_to_store: &[(StoredMeta, &AccountSharedData)],
) -> Vec<AccountInfo> { ) -> Vec<AccountInfo> {
@ -3691,7 +3691,7 @@ impl AccountsDb {
fn write_accounts_to_cache( fn write_accounts_to_cache(
&self, &self,
slot: Slot, slot: Slot,
hashes: Option<&[Hash]>, hashes: Option<&[impl Borrow<Hash>]>,
accounts_and_meta_to_store: &[(StoredMeta, &AccountSharedData)], accounts_and_meta_to_store: &[(StoredMeta, &AccountSharedData)],
) -> Vec<AccountInfo> { ) -> Vec<AccountInfo> {
let len = accounts_and_meta_to_store.len(); let len = accounts_and_meta_to_store.len();
@ -3704,7 +3704,7 @@ impl AccountsDb {
.iter() .iter()
.enumerate() .enumerate()
.map(|(i, (meta, account))| { .map(|(i, (meta, account))| {
let hash = hashes.map(|hashes| hashes[i]); let hash = hashes.map(|hashes| hashes[i].borrow());
let cached_account = let cached_account =
self.accounts_cache self.accounts_cache
.store(slot, &meta.pubkey, (*account).clone(), hash); .store(slot, &meta.pubkey, (*account).clone(), hash);
@ -3733,7 +3733,7 @@ impl AccountsDb {
&self, &self,
slot: Slot, slot: Slot,
accounts: &[(&Pubkey, &AccountSharedData)], accounts: &[(&Pubkey, &AccountSharedData)],
hashes: Option<&[Hash]>, hashes: Option<&[impl Borrow<Hash>]>,
storage_finder: F, storage_finder: F,
mut write_version_producer: P, mut write_version_producer: P,
is_cached_store: bool, is_cached_store: bool,
@ -4674,7 +4674,7 @@ impl AccountsDb {
&self, &self,
slot: Slot, slot: Slot,
accounts: &[(&Pubkey, &AccountSharedData)], accounts: &[(&Pubkey, &AccountSharedData)],
hashes: Option<&[Hash]>, hashes: Option<&[&Hash]>,
is_cached_store: bool, is_cached_store: bool,
) { ) {
// This path comes from a store to a non-frozen slot. // This path comes from a store to a non-frozen slot.
@ -4700,7 +4700,7 @@ impl AccountsDb {
&'a self, &'a self,
slot: Slot, slot: Slot,
accounts: &[(&Pubkey, &AccountSharedData)], accounts: &[(&Pubkey, &AccountSharedData)],
hashes: Option<&[Hash]>, hashes: Option<&[impl Borrow<Hash>]>,
storage_finder: Option<StorageFinder<'a>>, storage_finder: Option<StorageFinder<'a>>,
write_version_producer: Option<Box<dyn Iterator<Item = u64>>>, write_version_producer: Option<Box<dyn Iterator<Item = u64>>>,
) -> StoreAccountsTiming { ) -> StoreAccountsTiming {
@ -4724,7 +4724,7 @@ impl AccountsDb {
&'a self, &'a self,
slot: Slot, slot: Slot,
accounts: &[(&Pubkey, &AccountSharedData)], accounts: &[(&Pubkey, &AccountSharedData)],
hashes: Option<&[Hash]>, hashes: Option<&[impl Borrow<Hash>]>,
storage_finder: Option<StorageFinder<'a>>, storage_finder: Option<StorageFinder<'a>>,
write_version_producer: Option<Box<dyn Iterator<Item = u64>>>, write_version_producer: Option<Box<dyn Iterator<Item = u64>>>,
is_cached_store: bool, is_cached_store: bool,
@ -5178,7 +5178,7 @@ impl AccountsDb {
for (pubkey, alive_account) in alive_accounts { for (pubkey, alive_account) in alive_accounts {
accounts.push((pubkey, &alive_account.account)); accounts.push((pubkey, &alive_account.account));
hashes.push(alive_account.account_hash); hashes.push(&alive_account.account_hash);
write_versions.push(alive_account.write_version); write_versions.push(alive_account.write_version);
} }
start.stop(); start.stop();
@ -5809,7 +5809,7 @@ pub mod tests {
}; };
storages[0][0] storages[0][0]
.accounts .accounts
.append_accounts(&[(sm, &acc)], &[Hash::default()]); .append_accounts(&[(sm, &acc)], &[&Hash::default()]);
let calls = AtomicU64::new(0); let calls = AtomicU64::new(0);
let result = AccountsDb::scan_account_storage_no_bank( let result = AccountsDb::scan_account_storage_no_bank(
@ -7704,7 +7704,7 @@ pub mod tests {
} }
// provide bogus account hashes // provide bogus account hashes
let some_hash = Hash::new(&[0xca; HASH_BYTES]); let some_hash = Hash::new(&[0xca; HASH_BYTES]);
db.store_accounts_unfrozen(some_slot, accounts, Some(&[some_hash]), false); db.store_accounts_unfrozen(some_slot, accounts, Some(&[&some_hash]), false);
db.add_root(some_slot); db.add_root(some_slot);
assert_matches!( assert_matches!(
db.verify_bank_hash_and_lamports(some_slot, &ancestors, 1), db.verify_bank_hash_and_lamports(some_slot, &ancestors, 1),

View File

@ -11,6 +11,7 @@ use solana_sdk::{
pubkey::Pubkey, pubkey::Pubkey,
}; };
use std::{ use std::{
borrow::Borrow,
fs::{remove_file, OpenOptions}, fs::{remove_file, OpenOptions},
io, io,
io::{Seek, SeekFrom, Write}, io::{Seek, SeekFrom, Write},
@ -453,7 +454,7 @@ impl AppendVec {
pub fn append_accounts( pub fn append_accounts(
&self, &self,
accounts: &[(StoredMeta, &AccountSharedData)], accounts: &[(StoredMeta, &AccountSharedData)],
hashes: &[Hash], hashes: &[impl Borrow<Hash>],
) -> Vec<usize> { ) -> Vec<usize> {
let _lock = self.append_lock.lock().unwrap(); let _lock = self.append_lock.lock().unwrap();
let mut offset = self.len(); let mut offset = self.len();
@ -464,7 +465,7 @@ impl AppendVec {
let account_meta_ptr = &account_meta as *const AccountMeta; let account_meta_ptr = &account_meta as *const AccountMeta;
let data_len = stored_meta.data_len as usize; let data_len = stored_meta.data_len as usize;
let data_ptr = account.data().as_ptr(); let data_ptr = account.data().as_ptr();
let hash_ptr = hash.as_ref().as_ptr(); let hash_ptr = hash.borrow().as_ref().as_ptr();
let ptrs = [ let ptrs = [
(meta_ptr as *const u8, mem::size_of::<StoredMeta>()), (meta_ptr as *const u8, mem::size_of::<StoredMeta>()),
(account_meta_ptr as *const u8, mem::size_of::<AccountMeta>()), (account_meta_ptr as *const u8, mem::size_of::<AccountMeta>()),
@ -494,7 +495,7 @@ impl AppendVec {
account: &AccountSharedData, account: &AccountSharedData,
hash: Hash, hash: Hash,
) -> Option<usize> { ) -> Option<usize> {
let res = self.append_accounts(&[(storage_meta, account)], &[hash]); let res = self.append_accounts(&[(storage_meta, account)], &[&hash]);
if res.len() == 1 { if res.len() == 1 {
None None
} else { } else {