eliminate flatten().collect() of reclaims in clean (#26647)

This commit is contained in:
Jeff Washington (jwash) 2022-07-18 12:37:17 -05:00 committed by GitHub
parent dcbda2c6c5
commit 1c08f83c7e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 51 additions and 41 deletions

View File

@ -30,7 +30,7 @@ use {
accounts_index::{ accounts_index::{
AccountIndexGetResult, AccountSecondaryIndexes, AccountsIndex, AccountsIndexConfig, AccountIndexGetResult, AccountSecondaryIndexes, AccountsIndex, AccountsIndexConfig,
AccountsIndexRootsStats, AccountsIndexScanResult, IndexKey, IndexValue, IsCached, AccountsIndexRootsStats, AccountsIndexScanResult, IndexKey, IndexValue, IsCached,
RefCount, ScanConfig, ScanResult, SlotList, SlotSlice, UpsertReclaim, ZeroLamport, RefCount, ScanConfig, ScanResult, SlotList, UpsertReclaim, ZeroLamport,
ACCOUNTS_INDEX_CONFIG_FOR_BENCHMARKS, ACCOUNTS_INDEX_CONFIG_FOR_TESTING, ACCOUNTS_INDEX_CONFIG_FOR_BENCHMARKS, ACCOUNTS_INDEX_CONFIG_FOR_TESTING,
}, },
accounts_index_storage::Startup, accounts_index_storage::Startup,
@ -2160,15 +2160,15 @@ impl AccountsDb {
let mut clean_rooted = Measure::start("clean_old_root-ms"); let mut clean_rooted = Measure::start("clean_old_root-ms");
let reclaim_vecs = purges let reclaim_vecs = purges
.par_chunks(INDEX_CLEAN_BULK_COUNT) .par_chunks(INDEX_CLEAN_BULK_COUNT)
.map(|pubkeys: &[Pubkey]| { .filter_map(|pubkeys: &[Pubkey]| {
let mut reclaims = Vec::new(); let mut reclaims = Vec::new();
for pubkey in pubkeys { for pubkey in pubkeys {
self.accounts_index self.accounts_index
.clean_rooted_entries(pubkey, &mut reclaims, max_clean_root); .clean_rooted_entries(pubkey, &mut reclaims, max_clean_root);
} }
reclaims (!reclaims.is_empty()).then(|| reclaims)
}); })
let reclaims: Vec<_> = reclaim_vecs.flatten().collect(); .collect::<Vec<_>>();
clean_rooted.stop(); clean_rooted.stop();
inc_new_counter_info!("clean-old-root-par-clean-ms", clean_rooted.as_ms() as usize); inc_new_counter_info!("clean-old-root-par-clean-ms", clean_rooted.as_ms() as usize);
self.clean_accounts_stats self.clean_accounts_stats
@ -2183,7 +2183,7 @@ impl AccountsDb {
let mut reclaim_result = ReclaimResult::default(); let mut reclaim_result = ReclaimResult::default();
self.handle_reclaims( self.handle_reclaims(
&reclaims, (!reclaim_vecs.is_empty()).then(|| reclaim_vecs.iter().flatten()),
None, None,
Some((&self.clean_accounts_stats.purge_stats, &mut reclaim_result)), Some((&self.clean_accounts_stats.purge_stats, &mut reclaim_result)),
reset_accounts, reset_accounts,
@ -2740,7 +2740,7 @@ impl AccountsDb {
let reset_accounts = false; let reset_accounts = false;
let mut reclaim_result = ReclaimResult::default(); let mut reclaim_result = ReclaimResult::default();
self.handle_reclaims( self.handle_reclaims(
&reclaims, (!reclaims.is_empty()).then(|| reclaims.iter()),
None, None,
Some((&self.clean_accounts_stats.purge_stats, &mut reclaim_result)), Some((&self.clean_accounts_stats.purge_stats, &mut reclaim_result)),
reset_accounts, reset_accounts,
@ -2885,19 +2885,20 @@ impl AccountsDb {
/// * `reset_accounts` - Reset the append_vec store when the store is dead (count==0) /// * `reset_accounts` - Reset the append_vec store when the store is dead (count==0)
/// From the clean and shrink paths it should be false since there may be an in-progress /// From the clean and shrink paths it should be false since there may be an in-progress
/// hash operation and the stores may hold accounts that need to be unref'ed. /// hash operation and the stores may hold accounts that need to be unref'ed.
fn handle_reclaims( fn handle_reclaims<'a, I>(
&self, &'a self,
reclaims: SlotSlice<AccountInfo>, reclaims: Option<I>,
expected_single_dead_slot: Option<Slot>, expected_single_dead_slot: Option<Slot>,
purge_stats_and_reclaim_result: Option<(&PurgeStats, &mut ReclaimResult)>, purge_stats_and_reclaim_result: Option<(&PurgeStats, &mut ReclaimResult)>,
reset_accounts: bool, reset_accounts: bool,
) { ) where
if reclaims.is_empty() { I: Iterator<Item = &'a (Slot, AccountInfo)>,
return; {
} if let Some(reclaims) = reclaims {
let (purge_stats, purged_account_slots, reclaimed_offsets) = if let Some((
let (purge_stats, purged_account_slots, reclaimed_offsets) = purge_stats,
if let Some((purge_stats, (ref mut purged_account_slots, ref mut reclaimed_offsets))) = (ref mut purged_account_slots, ref mut reclaimed_offsets),
)) =
purge_stats_and_reclaim_result purge_stats_and_reclaim_result
{ {
( (
@ -2909,26 +2910,27 @@ impl AccountsDb {
(None, None, None) (None, None, None)
}; };
let dead_slots = self.remove_dead_accounts( let dead_slots = self.remove_dead_accounts(
reclaims, reclaims,
expected_single_dead_slot, expected_single_dead_slot,
reclaimed_offsets, reclaimed_offsets,
reset_accounts, reset_accounts,
); );
if let Some(purge_stats) = purge_stats { if let Some(purge_stats) = purge_stats {
if let Some(expected_single_dead_slot) = expected_single_dead_slot { if let Some(expected_single_dead_slot) = expected_single_dead_slot {
assert!(dead_slots.len() <= 1); assert!(dead_slots.len() <= 1);
if dead_slots.len() == 1 { if dead_slots.len() == 1 {
assert!(dead_slots.contains(&expected_single_dead_slot)); assert!(dead_slots.contains(&expected_single_dead_slot));
}
} }
}
self.process_dead_slots(&dead_slots, purged_account_slots, purge_stats); self.process_dead_slots(&dead_slots, purged_account_slots, purge_stats);
} else { } else {
// not sure why this fails yet with ancient append vecs // not sure why this fails yet with ancient append vecs
if !self.ancient_append_vecs { if !self.ancient_append_vecs {
assert!(dead_slots.is_empty()); assert!(dead_slots.is_empty());
}
} }
} }
} }
@ -5191,7 +5193,7 @@ impl AccountsDb {
// Slot should be dead after removing all its account entries // Slot should be dead after removing all its account entries
let expected_dead_slot = Some(remove_slot); let expected_dead_slot = Some(remove_slot);
self.handle_reclaims( self.handle_reclaims(
&reclaims, (!reclaims.is_empty()).then(|| reclaims.iter()),
expected_dead_slot, expected_dead_slot,
Some((purge_stats, &mut ReclaimResult::default())), Some((purge_stats, &mut ReclaimResult::default())),
false, false,
@ -7239,13 +7241,16 @@ impl AccountsDb {
} }
} }
fn remove_dead_accounts( fn remove_dead_accounts<'a, I>(
&self, &'a self,
reclaims: SlotSlice<AccountInfo>, reclaims: I,
expected_slot: Option<Slot>, expected_slot: Option<Slot>,
mut reclaimed_offsets: Option<&mut AppendVecOffsets>, mut reclaimed_offsets: Option<&mut AppendVecOffsets>,
reset_accounts: bool, reset_accounts: bool,
) -> HashSet<Slot> { ) -> HashSet<Slot>
where
I: Iterator<Item = &'a (Slot, AccountInfo)>,
{
let mut dead_slots = HashSet::new(); let mut dead_slots = HashSet::new();
let mut new_shrink_candidates: ShrinkCandidates = HashMap::new(); let mut new_shrink_candidates: ShrinkCandidates = HashMap::new();
let mut measure = Measure::start("remove"); let mut measure = Measure::start("remove");
@ -7799,7 +7804,12 @@ impl AccountsDb {
// From 1) and 2) we guarantee passing `no_purge_stats` == None, which is // From 1) and 2) we guarantee passing `no_purge_stats` == None, which is
// equivalent to asserting there will be no dead slots, is safe. // equivalent to asserting there will be no dead slots, is safe.
let mut handle_reclaims_time = Measure::start("handle_reclaims"); let mut handle_reclaims_time = Measure::start("handle_reclaims");
self.handle_reclaims(&reclaims, expected_single_dead_slot, None, reset_accounts); self.handle_reclaims(
(!reclaims.is_empty()).then(|| reclaims.iter()),
expected_single_dead_slot,
None,
reset_accounts,
);
handle_reclaims_time.stop(); handle_reclaims_time.stop();
self.stats self.stats
.store_handle_reclaims .store_handle_reclaims
@ -13698,7 +13708,7 @@ pub mod tests {
assert_eq!(removed_data_size, account.stored_size as StoredSize); assert_eq!(removed_data_size, account.stored_size as StoredSize);
assert_eq!(account_info.0, slot); assert_eq!(account_info.0, slot);
let reclaims = vec![account_info]; let reclaims = vec![account_info];
accounts_db.remove_dead_accounts(&reclaims, None, None, true); accounts_db.remove_dead_accounts(reclaims.iter(), None, None, true);
let after_size = storage0.alive_bytes.load(Ordering::Acquire); let after_size = storage0.alive_bytes.load(Ordering::Acquire);
assert_eq!(before_size, after_size + account.stored_size); assert_eq!(before_size, after_size + account.stored_size);
} }