Skip to content

Commit

Permalink
move bank_hash_stats into bank
Browse files Browse the repository at this point in the history
  • Loading branch information
@HaoranYi
HaoranYi committed Nov 27, 2024
1 parent 20f4f8a commit 56713c1
Show file tree
Hide file tree
Showing 14 changed files with 199 additions and 191 deletions.
10 changes: 5 additions & 5 deletions accounts-db/benches/accounts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ fn bench_delete_dependencies(bencher: &mut Bencher) {
for i in 0..1000 {
let pubkey = solana_sdk::pubkey::new_rand();
let account = AccountSharedData::new(i + 1, 0, AccountSharedData::default().owner());
accounts.store_slow_uncached(i, &pubkey, &account);
accounts.store_slow_uncached(i, &old_pubkey, &zero_account);
accounts.store_slow_uncached(i, &pubkey, &account, None);
accounts.store_slow_uncached(i, &old_pubkey, &zero_account, None);
old_pubkey = pubkey;
accounts.add_root(i);
}
Expand Down Expand Up @@ -149,7 +149,7 @@ where
.take(num_keys)
.collect();
let storable_accounts: Vec<_> = pubkeys.iter().zip(accounts_data.iter()).collect();
accounts.store_accounts_cached((slot, storable_accounts.as_slice()));
accounts.store_accounts_cached((slot, storable_accounts.as_slice()), None);
accounts.add_root(slot);
accounts
.accounts_db
Expand All @@ -176,7 +176,7 @@ where
// Write to a different slot than the one being read from. Because
// there's a new account pubkey being written to every time, will
// compete for the accounts index lock on every store
accounts.store_accounts_cached((slot + 1, new_storable_accounts.as_slice()));
accounts.store_accounts_cached((slot + 1, new_storable_accounts.as_slice()), None);
});
}

Expand Down Expand Up @@ -326,7 +326,7 @@ fn bench_load_largest_accounts(b: &mut Bencher) {
let lamports = rng.gen();
let pubkey = Pubkey::new_unique();
let account = AccountSharedData::new(lamports, 0, &Pubkey::default());
accounts.store_slow_uncached(0, &pubkey, &account);
accounts.store_slow_uncached(0, &pubkey, &account, None);
}
let ancestors = Ancestors::from(vec![0]);
let bank_id = 0;
Expand Down
48 changes: 27 additions & 21 deletions accounts-db/src/accounts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ use {
crate::{
account_locks::{validate_account_locks, AccountLocks},
accounts_db::{
AccountStorageEntry, AccountsAddRootTiming, AccountsDb, LoadHint, LoadedAccount,
ScanAccountStorageData, ScanStorageResult, VerifyAccountsHashAndLamportsConfig,
stats::AtomicBankHashStats, AccountStorageEntry, AccountsAddRootTiming, AccountsDb,
LoadHint, LoadedAccount, ScanAccountStorageData, ScanStorageResult,
VerifyAccountsHashAndLamportsConfig,
},
accounts_index::{IndexKey, ScanConfig, ScanError, ScanResult},
ancestors::Ancestors,
Expand Down Expand Up @@ -551,8 +552,15 @@ impl Accounts {
/// Slow because lock is held for 1 operation instead of many.
/// WARNING: This noncached version is only to be used for tests/benchmarking
/// as bypassing the cache in general is not supported
pub fn store_slow_uncached(&self, slot: Slot, pubkey: &Pubkey, account: &AccountSharedData) {
self.accounts_db.store_uncached(slot, &[(pubkey, account)]);
pub fn store_slow_uncached(
&self,
slot: Slot,
pubkey: &Pubkey,
account: &AccountSharedData,
bank_hash_stats: Option<&AtomicBankHashStats>,
) {
self.accounts_db
.store_uncached(slot, &[(pubkey, account)], bank_hash_stats);
}

/// This function will prevent multiple threads from modifying the same account state at the
Expand Down Expand Up @@ -633,13 +641,19 @@ impl Accounts {
&self,
accounts: impl StorableAccounts<'a>,
transactions: Option<&'a [&'a SanitizedTransaction]>,
bank_hash_stats: Option<&AtomicBankHashStats>,
) {
self.accounts_db
.store_cached_inline_update_index(accounts, transactions);
.store_cached_inline_update_index(accounts, transactions, bank_hash_stats);
}

pub fn store_accounts_cached<'a>(&self, accounts: impl StorableAccounts<'a>) {
self.accounts_db.store_cached(accounts, None)
pub fn store_accounts_cached<'a>(
&self,
accounts: impl StorableAccounts<'a>,
bank_hash_stats: Option<&AtomicBankHashStats>,
) {
self.accounts_db
.store_cached(accounts, None, bank_hash_stats)
}

/// Add a slot to root. Root slots cannot be purged
Expand Down Expand Up @@ -770,7 +784,7 @@ mod tests {
let invalid_table_key = Pubkey::new_unique();
let mut invalid_table_account = AccountSharedData::default();
invalid_table_account.set_lamports(1);
accounts.store_slow_uncached(0, &invalid_table_key, &invalid_table_account);
accounts.store_slow_uncached(0, &invalid_table_key, &invalid_table_account, None);

let address_table_lookup = MessageAddressTableLookup {
account_key: invalid_table_key,
Expand All @@ -797,7 +811,7 @@ mod tests {
let invalid_table_key = Pubkey::new_unique();
let invalid_table_account =
AccountSharedData::new(1, 0, &address_lookup_table::program::id());
accounts.store_slow_uncached(0, &invalid_table_key, &invalid_table_account);
accounts.store_slow_uncached(0, &invalid_table_key, &invalid_table_account, None);

let address_table_lookup = MessageAddressTableLookup {
account_key: invalid_table_key,
Expand Down Expand Up @@ -836,7 +850,7 @@ mod tests {
0,
)
};
accounts.store_slow_uncached(0, &table_key, &table_account);
accounts.store_slow_uncached(0, &table_key, &table_account, None);

let address_table_lookup = MessageAddressTableLookup {
account_key: table_key,
Expand Down Expand Up @@ -868,13 +882,13 @@ mod tests {
// Load accounts owned by various programs into AccountsDb
let pubkey0 = solana_sdk::pubkey::new_rand();
let account0 = AccountSharedData::new(1, 0, &Pubkey::from([2; 32]));
accounts.store_slow_uncached(0, &pubkey0, &account0);
accounts.store_slow_uncached(0, &pubkey0, &account0, None);
let pubkey1 = solana_sdk::pubkey::new_rand();
let account1 = AccountSharedData::new(1, 0, &Pubkey::from([2; 32]));
accounts.store_slow_uncached(0, &pubkey1, &account1);
accounts.store_slow_uncached(0, &pubkey1, &account1, None);
let pubkey2 = solana_sdk::pubkey::new_rand();
let account2 = AccountSharedData::new(1, 0, &Pubkey::from([3; 32]));
accounts.store_slow_uncached(0, &pubkey2, &account2);
accounts.store_slow_uncached(0, &pubkey2, &account2, None);

let loaded = accounts.load_by_program_slot(0, Some(&Pubkey::from([2; 32])));
assert_eq!(loaded.len(), 2);
Expand All @@ -884,14 +898,6 @@ mod tests {
assert_eq!(loaded, vec![]);
}

#[test]
fn test_accounts_empty_bank_hash_stats() {
let accounts_db = AccountsDb::new_single_for_tests();
let accounts = Accounts::new(Arc::new(accounts_db));
assert!(accounts.accounts_db.get_bank_hash_stats(0).is_some());
assert!(accounts.accounts_db.get_bank_hash_stats(1).is_none());
}

#[test]
fn test_lock_accounts_with_duplicates() {
let accounts_db = AccountsDb::new_single_for_tests();
Expand Down
76 changes: 19 additions & 57 deletions accounts-db/src/accounts_db.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1533,8 +1533,6 @@ pub struct AccountsDb {

pub thread_pool_hash: ThreadPool,

bank_hash_stats: DashMap<Slot, AtomicBankHashStats>,

accounts_delta_hashes: Mutex<HashMap<Slot, AccountsDeltaHash>>,
accounts_hashes: Mutex<HashMap<Slot, (AccountsHash, /*capitalization*/ u64)>>,
incremental_accounts_hashes:
Expand Down Expand Up @@ -1867,7 +1865,7 @@ impl solana_frozen_abi::abi_example::AbiExample for AccountsDb {
let some_data_len = 5;
let some_slot: Slot = 0;
let account = AccountSharedData::new(1, some_data_len, &key);
accounts_db.store_uncached(some_slot, &[(&key, &account)]);
accounts_db.store_uncached(some_slot, &[(&key, &account)], None);
accounts_db.add_root(0);

accounts_db
Expand Down Expand Up @@ -1980,9 +1978,6 @@ impl AccountsDb {
Self::DEFAULT_MAX_READ_ONLY_CACHE_DATA_SIZE_HI,
));

let bank_hash_stats =
DashMap::from_iter(std::iter::once((0, AtomicBankHashStats::default())));

// Increase the stack for foreground threads
// rayon needs a lot of stack
const ACCOUNTS_STACK_SIZE: usize = 8 * 1024 * 1024;
Expand Down Expand Up @@ -2051,7 +2046,6 @@ impl AccountsDb {
.into(),
verify_experimental_accumulator_hash: accounts_db_config
.verify_experimental_accumulator_hash,
bank_hash_stats,
thread_pool,
thread_pool_clean,
thread_pool_hash,
Expand Down Expand Up @@ -4559,7 +4553,6 @@ impl AccountsDb {
dropped_roots.for_each(|slot| {
self.accounts_index.clean_dead_slot(slot);
accounts_delta_hashes.remove(&slot);
self.bank_hash_stats.remove(&slot);
// the storage has been removed from this slot and recycled or dropped
assert!(self.storage.remove(&slot, false).is_none());
debug_assert!(
Expand Down Expand Up @@ -4971,21 +4964,6 @@ impl AccountsDb {
}
}

/// Insert a default bank hash stats for `slot`
///
/// This fn is called when creating a new bank from parent.
pub fn insert_default_bank_hash_stats(&self, slot: Slot, parent_slot: Slot) {
if self.bank_hash_stats.get(&slot).is_some() {
error!(
"set_hash: already exists; multiple forks with shared slot {slot} as child \
(parent: {parent_slot})!?"
);
return;
}
self.bank_hash_stats
.insert(slot, AtomicBankHashStats::default());
}

pub fn load(
&self,
ancestors: &Ancestors,
Expand Down Expand Up @@ -7652,32 +7630,6 @@ impl AccountsDb {
.cloned()
}

/// When reconstructing AccountsDb from a snapshot, insert the `bank_hash_stats` into the
/// internal bank hash stats map.
///
/// This fn is only called when loading from a snapshot, which means AccountsDb is new and its
/// bank hash stats map is unpopulated. Except for slot 0.
///
/// Slot 0 is a special case. When a new AccountsDb is created--like when loading from a
/// snapshot--the bank hash stats map is populated with a default entry at slot 0. Remove the
/// default entry at slot 0, and then insert the new value at `slot`.
pub fn update_bank_hash_stats_from_snapshot(
&mut self,
slot: Slot,
stats: BankHashStats,
) -> Option<BankHashStats> {
self.bank_hash_stats.remove(&0);

self.bank_hash_stats
.insert(slot, AtomicBankHashStats::new(&stats))
.map(|old_stat| old_stat.load())
}

/// Get the bank hash stats for `slot` in the `bank_hash_stats` map
pub fn get_bank_hash_stats(&self, slot: Slot) -> Option<BankHashStats> {
self.bank_hash_stats.get(&slot).map(|stats| stats.load())
}

fn update_index<'a>(
&self,
infos: Vec<AccountInfo>,
Expand Down Expand Up @@ -7901,7 +7853,6 @@ impl AccountsDb {
let mut accounts_delta_hashes = self.accounts_delta_hashes.lock().unwrap();
for slot in dead_slots_iter {
accounts_delta_hashes.remove(slot);
self.bank_hash_stats.remove(slot);
}
drop(accounts_delta_hashes);

Expand Down Expand Up @@ -8082,40 +8033,50 @@ impl AccountsDb {
&self,
accounts: impl StorableAccounts<'a>,
transactions: Option<&'a [&'a SanitizedTransaction]>,
bank_hash_stat: Option<&AtomicBankHashStats>,
) {
self.store(
accounts,
&StoreTo::Cache,
transactions,
StoreReclaims::Default,
UpdateIndexThreadSelection::PoolWithThreshold,
bank_hash_stat,
);
}

pub(crate) fn store_cached_inline_update_index<'a>(
&self,
accounts: impl StorableAccounts<'a>,
transactions: Option<&'a [&'a SanitizedTransaction]>,
bank_hash_stat: Option<&AtomicBankHashStats>,
) {
self.store(
accounts,
&StoreTo::Cache,
transactions,
StoreReclaims::Default,
UpdateIndexThreadSelection::Inline,
bank_hash_stat,
);
}

/// Store the account update.
/// only called by tests
pub fn store_uncached(&self, slot: Slot, accounts: &[(&Pubkey, &AccountSharedData)]) {
pub fn store_uncached(
&self,
slot: Slot,
accounts: &[(&Pubkey, &AccountSharedData)],
bank_hash_stat: Option<&AtomicBankHashStats>,
) {
let storage = self.find_storage_candidate(slot);
self.store(
(slot, accounts),
&StoreTo::Storage(&storage),
None,
StoreReclaims::Default,
UpdateIndexThreadSelection::PoolWithThreshold,
bank_hash_stat,
);
}

Expand All @@ -8126,6 +8087,7 @@ impl AccountsDb {
transactions: Option<&'a [&'a SanitizedTransaction]>,
reclaim: StoreReclaims,
update_index_thread_selection: UpdateIndexThreadSelection,
bank_hash_stats: Option<&AtomicBankHashStats>,
) {
// If all transactions in a batch are errored,
// it's possible to get a store with no accounts.
Expand All @@ -8146,10 +8108,9 @@ impl AccountsDb {
.store_total_data
.fetch_add(total_data as u64, Ordering::Relaxed);

self.bank_hash_stats
.entry(accounts.target_slot())
.or_default()
.accumulate(&stats);
if let Some(bank_hash_stats) = bank_hash_stats {
bank_hash_stats.accumulate(&stats);
}

// we use default hashes for now since the same account may be stored to the cache multiple times
self.store_accounts_unfrozen(
Expand Down Expand Up @@ -9425,6 +9386,7 @@ impl AccountsDb {
None,
StoreReclaims::Default,
UpdateIndexThreadSelection::PoolWithThreshold,
None,
);
}

Expand Down Expand Up @@ -9617,7 +9579,7 @@ pub mod test_utils {
data_size,
AccountSharedData::default().owner(),
);
accounts.store_slow_uncached(slot, &pubkey, &account);
accounts.store_slow_uncached(slot, &pubkey, &account, None);
pubkeys.push(pubkey);
}
}
Expand All @@ -9628,7 +9590,7 @@ pub mod test_utils {
for pubkey in pubkeys {
let amount = thread_rng().gen_range(0..10);
let account = AccountSharedData::new(amount, 0, AccountSharedData::default().owner());
accounts.store_slow_uncached(slot, pubkey, &account);
accounts.store_slow_uncached(slot, pubkey, &account, None);
}
}
}
Loading

0 comments on commit 56713c1

Please sign in to comment.