From d718ab24910ba70d500fc9f1f79c32b556e41317 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 15 Jul 2020 15:13:33 +0000 Subject: [PATCH] accounts_clean: Convert stack dependency calculation with iterative (#11067) (#11076) * accounts_clean: Convert stack dependency calculation with iterative * optimize clean with by creating a reverse-lookup hashset of the affected keys * Add dependency bench reduce bench * Huge clean (cherry picked from commit 8bf3a0aa05c9c0a8274e32a565465e80b4ac426b) Co-authored-by: sakridge --- runtime/benches/accounts.rs | 19 ++++++++ runtime/src/accounts.rs | 22 +++++++++ runtime/src/accounts_db.rs | 93 +++++++++++++++++-------------------- 3 files changed, 83 insertions(+), 51 deletions(-) diff --git a/runtime/benches/accounts.rs b/runtime/benches/accounts.rs index e5a4a7ab38..656b43f3bb 100644 --- a/runtime/benches/accounts.rs +++ b/runtime/benches/accounts.rs @@ -95,3 +95,22 @@ fn test_accounts_delta_hash(bencher: &mut Bencher) { accounts.accounts_db.get_accounts_delta_hash(0); }); } + +#[bench] +fn bench_delete_dependencies(bencher: &mut Bencher) { + solana_logger::setup(); + let accounts = Accounts::new(vec![PathBuf::from("accounts_delete_deps")]); + let mut old_pubkey = Pubkey::default(); + let zero_account = Account::new(0, 0, &Account::default().owner); + for i in 0..1000 { + let pubkey = Pubkey::new_rand(); + let account = Account::new((i + 1) as u64, 0, &Account::default().owner); + accounts.store_slow(i, &pubkey, &account); + accounts.store_slow(i, &old_pubkey, &zero_account); + old_pubkey = pubkey; + accounts.add_root(i); + } + bencher.iter(|| { + accounts.accounts_db.clean_accounts(); + }); +} diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 6997b98c68..d1c08b9602 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -1719,4 +1719,26 @@ mod tests { assert!(!Accounts::has_duplicates(&[1, 2])); assert!(Accounts::has_duplicates(&[1, 2, 1])); } + + #[test] + fn huge_clean() { + solana_logger::setup(); + let accounts = Accounts::new(Vec::new()); + let mut old_pubkey = Pubkey::default(); + let zero_account = Account::new(0, 0, &Account::default().owner); + info!("storing.."); + for i in 0..2_000 { + let pubkey = Pubkey::new_rand(); + let account = Account::new((i + 1) as u64, 0, &Account::default().owner); + accounts.store_slow(i, &pubkey, &account); + accounts.store_slow(i, &old_pubkey, &zero_account); + old_pubkey = pubkey; + accounts.add_root(i); + if i % 1_000 == 0 { + info!(" store {}", i); + } + } + info!("done..cleaning.."); + accounts.accounts_db.clean_accounts(); + } } diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index e26af47031..c6aae843a0 100644 --- a/runtime/src/accounts_db.rs +++ b/runtime/src/accounts_db.rs @@ -550,41 +550,9 @@ impl AccountsDB { .extend(previous_roots); } - fn inc_store_counts( - no_delete_id: AppendVecId, - purges: &HashMap, u64)>, - store_counts: &mut HashMap, - already_counted: &mut HashSet, - ) { - if already_counted.contains(&no_delete_id) { - return; - } - *store_counts.get_mut(&no_delete_id).unwrap() += 1; - already_counted.insert(no_delete_id); - let mut affected_pubkeys = HashSet::new(); - for (key, (account_infos, _ref_count)) in purges { - for (_slot, account_info) in account_infos { - if account_info.store_id == no_delete_id { - affected_pubkeys.insert(key); - break; - } - } - } - for key in affected_pubkeys { - for (_slot, account_info) in &purges.get(&key).unwrap().0 { - Self::inc_store_counts( - account_info.store_id, - purges, - store_counts, - already_counted, - ); - } - } - } - fn calc_delete_dependencies( purges: &HashMap, u64)>, - store_counts: &mut HashMap, + store_counts: &mut HashMap)>, ) { // Another pass to check if there are some filtered accounts which // do not match the criteria of deleting all appendvecs which contain them @@ -596,7 +564,7 @@ impl AccountsDB { } else { let mut no_delete = false; for (_slot, account_info) in account_infos { - if *store_counts.get(&account_info.store_id).unwrap() != 0 { + if store_counts.get(&account_info.store_id).unwrap().0 != 0 { no_delete = true; break; } @@ -604,13 +572,29 @@ impl AccountsDB { no_delete }; if no_delete { + let mut pending_store_ids: HashSet = HashSet::new(); for (_slot_id, account_info) in account_infos { - Self::inc_store_counts( - account_info.store_id, - &purges, - store_counts, - &mut already_counted, - ); + if !already_counted.contains(&account_info.store_id) { + pending_store_ids.insert(account_info.store_id); + } + } + while !pending_store_ids.is_empty() { + let id = pending_store_ids.iter().next().cloned().unwrap(); + pending_store_ids.remove(&id); + if already_counted.contains(&id) { + continue; + } + store_counts.get_mut(&id).unwrap().0 += 1; + already_counted.insert(id); + + let affected_pubkeys = &store_counts.get(&id).unwrap().1; + for key in affected_pubkeys { + for (_slot, account_info) in &purges.get(&key).unwrap().0 { + if !already_counted.contains(&account_info.store_id) { + pending_store_ids.insert(account_info.store_id); + } + } + } } } } @@ -693,18 +677,21 @@ impl AccountsDB { // Calculate store counts as if everything was purged // Then purge if we can - let mut store_counts: HashMap = HashMap::new(); + let mut store_counts: HashMap)> = HashMap::new(); let storage = self.storage.read().unwrap(); - for (account_infos, _ref_count) in purges.values() { + for (key, (account_infos, _ref_count)) in &purges { for (slot, account_info) in account_infos { let slot_storage = storage.0.get(&slot).unwrap(); let store = slot_storage.get(&account_info.store_id).unwrap(); if let Some(store_count) = store_counts.get_mut(&account_info.store_id) { - *store_count -= 1; + store_count.0 -= 1; + store_count.1.insert(*key); } else { + let mut key_set = HashSet::new(); + key_set.insert(*key); store_counts.insert( account_info.store_id, - store.count_and_status.read().unwrap().0 - 1, + (store.count_and_status.read().unwrap().0 - 1, key_set), ); } } @@ -721,7 +708,7 @@ impl AccountsDB { let mut purge_filter = Measure::start("purge_filter"); purges.retain(|_pubkey, (account_infos, _ref_count)| { for (_slot, account_info) in account_infos.iter() { - if *store_counts.get(&account_info.store_id).unwrap() != 0 { + if store_counts.get(&account_info.store_id).unwrap().0 != 0 { return false; } } @@ -4161,18 +4148,22 @@ pub mod tests { } let mut store_counts = HashMap::new(); - store_counts.insert(0, 0); - store_counts.insert(1, 0); - store_counts.insert(2, 0); - store_counts.insert(3, 1); + store_counts.insert(0, (0, HashSet::from_iter(vec![key0]))); + store_counts.insert(1, (0, HashSet::from_iter(vec![key0, key1]))); + store_counts.insert(2, (0, HashSet::from_iter(vec![key1, key2]))); + store_counts.insert(3, (1, HashSet::from_iter(vec![key2]))); AccountsDB::calc_delete_dependencies(&purges, &mut store_counts); let mut stores: Vec<_> = store_counts.keys().cloned().collect(); stores.sort(); for store in &stores { - info!("store: {:?} : {}", store, store_counts.get(&store).unwrap()); + info!( + "store: {:?} : {:?}", + store, + store_counts.get(&store).unwrap() + ); } for x in 0..3 { - assert!(store_counts[&x] >= 1); + assert!(store_counts[&x].0 >= 1); } } }