From 02b050e0f57a2cf3c02085582e4c6e45f8ceacba Mon Sep 17 00:00:00 2001 From: "Jeff Washington (jwash)" <75863576+jeffwashington@users.noreply.github.com> Date: Thu, 26 Aug 2021 18:12:43 -0500 Subject: [PATCH] replace AccountsIndex btree with hashmap of 8k bins (#19212) --- runtime/src/accounts.rs | 1 + runtime/src/accounts_db.rs | 15 +- runtime/src/accounts_index.rs | 295 +++++++++++++++++++++++++++------- 3 files changed, 250 insertions(+), 61 deletions(-) diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 8267240ef9..3eac2d635a 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -802,6 +802,7 @@ impl Accounts { "load_to_collect_rent_eagerly_scan_elapsed", ancestors, range, + true, |collector: &mut Vec<(Pubkey, AccountSharedData)>, option| { Self::load_while_filtering(collector, option, |_| true) }, diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index 9b9218d051..d2a260cd99 100644 --- a/runtime/src/accounts_db.rs +++ b/runtime/src/accounts_db.rs @@ -2643,6 +2643,7 @@ impl AccountsDb { metric_name: &'static str, ancestors: &Ancestors, scan_func: F, + collect_all_unsorted: bool, ) -> A where F: Fn(&mut A, (&Pubkey, LoadedAccount, Slot)), @@ -2660,6 +2661,7 @@ impl AccountsDb { scan_func(&mut collector, (pubkey, loaded_account, slot)); } }, + collect_all_unsorted, ); collector } @@ -2669,6 +2671,7 @@ impl AccountsDb { metric_name: &'static str, ancestors: &Ancestors, range: R, + collect_all_unsorted: bool, scan_func: F, ) -> A where @@ -2681,6 +2684,7 @@ impl AccountsDb { metric_name, ancestors, range, + collect_all_unsorted, |pubkey, (account_info, slot)| { // unlike other scan fns, this is called from Bank::collect_rent_eagerly(), // which is on-consensus processing in the banking/replaying stage. @@ -4589,7 +4593,11 @@ impl AccountsDb { .accounts_index .account_maps .iter() - .map(|btree| btree.read().unwrap().keys().cloned().collect::>()) + .map(|map| { + let mut keys = map.read().unwrap().keys().cloned().collect::>(); + keys.sort_unstable(); // hashmap is not ordered, but bins are relative to each other + keys + }) .flatten() .collect(); collect.stop(); @@ -7152,6 +7160,8 @@ pub mod tests { ); } + const COLLECT_ALL_UNSORTED_FALSE: bool = false; + #[test] fn test_accountsdb_latest_ancestor() { solana_logger::setup(); @@ -7182,6 +7192,7 @@ pub mod tests { |accounts: &mut Vec, option| { accounts.push(option.1.take_account()); }, + COLLECT_ALL_UNSORTED_FALSE, ); assert_eq!(accounts, vec![account1]); } @@ -8717,6 +8728,7 @@ pub mod tests { |accounts: &mut Vec, option| { accounts.push(option.1.take_account()); }, + COLLECT_ALL_UNSORTED_FALSE, ); assert_eq!(accounts, vec![account0]); @@ -8727,6 +8739,7 @@ pub mod tests { |accounts: &mut Vec, option| { accounts.push(option.1.take_account()); }, + COLLECT_ALL_UNSORTED_FALSE, ); assert_eq!(accounts.len(), 2); } diff --git a/runtime/src/accounts_index.rs b/runtime/src/accounts_index.rs index 76ab801509..3b1f644275 100644 --- a/runtime/src/accounts_index.rs +++ b/runtime/src/accounts_index.rs @@ -14,10 +14,7 @@ use solana_sdk::{ pubkey::{Pubkey, PUBKEY_BYTES}, }; use std::{ - collections::{ - btree_map::{self, BTreeMap, Entry}, - HashSet, - }, + collections::{btree_map::BTreeMap, hash_map::Entry, HashMap, HashSet}, fmt::Debug, ops::{ Bound, @@ -32,7 +29,7 @@ use std::{ use thiserror::Error; pub const ITER_BATCH_SIZE: usize = 1000; -pub const BINS_DEFAULT: usize = 16; +pub const BINS_DEFAULT: usize = 8192; pub const ACCOUNTS_INDEX_CONFIG_FOR_TESTING: AccountsIndexConfig = AccountsIndexConfig { bins: Some(BINS_DEFAULT), }; @@ -43,7 +40,7 @@ pub type ScanResult = Result; pub type SlotList = Vec<(Slot, T)>; pub type SlotSlice<'s, T> = &'s [(Slot, T)]; pub type RefCount = u64; -pub type AccountMap = BTreeMap; +pub type AccountMap = HashMap; type AccountMapEntry = Arc>; @@ -624,9 +621,30 @@ pub struct AccountsIndexIterator<'a, T> { start_bound: Bound, end_bound: Bound, is_finished: bool, + collect_all_unsorted: bool, } impl<'a, T> AccountsIndexIterator<'a, T> { + fn range<'b, R>( + map: &'b AccountMapsReadLock<'b, T>, + range: R, + collect_all_unsorted: bool, + ) -> Vec<(&'b Pubkey, &'b AccountMapEntry)> + where + R: RangeBounds, + { + let mut result = Vec::with_capacity(map.len()); + for (k, v) in map.iter() { + if range.contains(k) { + result.push((k, v)); + } + } + if !collect_all_unsorted { + result.sort_unstable_by(|a, b| a.0.cmp(b.0)); + } + result + } + fn clone_bound(bound: Bound<&Pubkey>) -> Bound { match bound { Unbounded => Unbounded, @@ -670,7 +688,7 @@ impl<'a, T> AccountsIndexIterator<'a, T> { (start_bin, bin_range) } - pub fn new(index: &'a AccountsIndex, range: Option) -> Self + pub fn new(index: &'a AccountsIndex, range: Option, collect_all_unsorted: bool) -> Self where R: RangeBounds, { @@ -686,6 +704,7 @@ impl<'a, T> AccountsIndexIterator<'a, T> { account_maps: &index.account_maps, is_finished: false, bin_calculator: &index.bin_calculator, + collect_all_unsorted, } } } @@ -699,10 +718,12 @@ impl<'a, T: 'static + Clone> Iterator for AccountsIndexIterator<'a, T> { let (start_bin, bin_range) = self.bin_start_and_range(); let mut chunk: Vec<(Pubkey, AccountMapEntry)> = Vec::with_capacity(ITER_BATCH_SIZE); 'outer: for i in self.account_maps.iter().skip(start_bin).take(bin_range) { - for (pubkey, account_map_entry) in - i.read().unwrap().range((self.start_bound, self.end_bound)) - { - if chunk.len() >= ITER_BATCH_SIZE { + for (pubkey, account_map_entry) in Self::range( + &i.read().unwrap(), + (self.start_bound, self.end_bound), + self.collect_all_unsorted, + ) { + if chunk.len() >= ITER_BATCH_SIZE && !self.collect_all_unsorted { break 'outer; } let item = (*pubkey, account_map_entry.clone()); @@ -713,6 +734,8 @@ impl<'a, T: 'static + Clone> Iterator for AccountsIndexIterator<'a, T> { if chunk.is_empty() { self.is_finished = true; return None; + } else if self.collect_all_unsorted { + self.is_finished = true; } self.start_bound = Excluded(chunk.last().unwrap().0); @@ -807,11 +830,11 @@ impl AccountsIndex { (account_maps, bin_calculator) } - fn iter(&self, range: Option) -> AccountsIndexIterator + fn iter(&self, range: Option, collect_all_unsorted: bool) -> AccountsIndexIterator where R: RangeBounds, { - AccountsIndexIterator::new(self, range) + AccountsIndexIterator::new(self, range, collect_all_unsorted) } fn do_checked_scan_accounts( @@ -821,6 +844,7 @@ impl AccountsIndex { scan_bank_id: BankId, func: F, scan_type: ScanTypes, + collect_all_unsorted: bool, ) -> Result<(), ScanError> where F: FnMut(&Pubkey, (&T, Slot)), @@ -973,7 +997,14 @@ impl AccountsIndex { match scan_type { ScanTypes::Unindexed(range) => { // Pass "" not to log metrics, so RPC doesn't get spammy - self.do_scan_accounts(metric_name, ancestors, func, range, Some(max_root)); + self.do_scan_accounts( + metric_name, + ancestors, + func, + range, + Some(max_root), + collect_all_unsorted, + ); } ScanTypes::Indexed(IndexKey::ProgramId(program_id)) => { self.do_scan_secondary_index( @@ -1037,11 +1068,19 @@ impl AccountsIndex { ancestors: &Ancestors, func: F, range: Option, + collect_all_unsorted: bool, ) where F: FnMut(&Pubkey, (&T, Slot)), R: RangeBounds, { - self.do_scan_accounts(metric_name, ancestors, func, range, None); + self.do_scan_accounts( + metric_name, + ancestors, + func, + range, + None, + collect_all_unsorted, + ); } // Scan accounts and return latest version of each account that is either: @@ -1054,6 +1093,7 @@ impl AccountsIndex { mut func: F, range: Option, max_root: Option, + collect_all_unsorted: bool, ) where F: FnMut(&Pubkey, (&T, Slot)), R: RangeBounds, @@ -1067,7 +1107,7 @@ impl AccountsIndex { let mut read_lock_elapsed = 0; let mut iterator_elapsed = 0; let mut iterator_timer = Measure::start("iterator_elapsed"); - for pubkey_list in self.iter(range) { + for pubkey_list in self.iter(range, collect_all_unsorted) { iterator_timer.stop(); iterator_elapsed += iterator_timer.as_us(); for (pubkey, list) in pubkey_list { @@ -1185,7 +1225,7 @@ impl AccountsIndex { if !dead_keys.is_empty() { for key in dead_keys.iter() { let mut w_index = self.get_account_maps_write_lock(key); - if let btree_map::Entry::Occupied(index_entry) = w_index.entry(**key) { + if let Entry::Occupied(index_entry) = w_index.entry(**key) { if index_entry.get().slot_list.read().unwrap().is_empty() { index_entry.remove(); @@ -1209,6 +1249,7 @@ impl AccountsIndex { where F: FnMut(&Pubkey, (&T, Slot)), { + let collect_all_unsorted = false; // Pass "" not to log metrics, so RPC doesn't get spammy self.do_checked_scan_accounts( "", @@ -1216,6 +1257,7 @@ impl AccountsIndex { scan_bank_id, func, ScanTypes::Unindexed(None::>), + collect_all_unsorted, ) } @@ -1224,10 +1266,17 @@ impl AccountsIndex { metric_name: &'static str, ancestors: &Ancestors, func: F, + collect_all_unsorted: bool, ) where F: FnMut(&Pubkey, (&T, Slot)), { - self.do_unchecked_scan_accounts(metric_name, ancestors, func, None::>); + self.do_unchecked_scan_accounts( + metric_name, + ancestors, + func, + None::>, + collect_all_unsorted, + ); } /// call func with every pubkey and index visible from a given set of ancestors with range @@ -1236,13 +1285,20 @@ impl AccountsIndex { metric_name: &'static str, ancestors: &Ancestors, range: R, + collect_all_unsorted: bool, func: F, ) where F: FnMut(&Pubkey, (&T, Slot)), R: RangeBounds, { // Only the rent logic should be calling this, which doesn't need the safety checks - self.do_unchecked_scan_accounts(metric_name, ancestors, func, Some(range)); + self.do_unchecked_scan_accounts( + metric_name, + ancestors, + func, + Some(range), + collect_all_unsorted, + ); } /// call func with every pubkey and index visible from a given set of ancestors @@ -1256,6 +1312,8 @@ impl AccountsIndex { where F: FnMut(&Pubkey, (&T, Slot)), { + let collect_all_unsorted = false; + // Pass "" not to log metrics, so RPC doesn't get spammy self.do_checked_scan_accounts( "", @@ -1263,6 +1321,7 @@ impl AccountsIndex { scan_bank_id, func, ScanTypes::>::Indexed(index_key), + collect_all_unsorted, ) } @@ -2584,6 +2643,8 @@ pub mod tests { } } + const COLLECT_ALL_UNSORTED_FALSE: bool = false; + #[test] fn test_get_empty() { let key = Keypair::new(); @@ -2593,7 +2654,12 @@ pub mod tests { assert!(index.get(&key.pubkey(), None, None).is_none()); let mut num = 0; - index.unchecked_scan_accounts("", &ancestors, |_pubkey, _index| num += 1); + index.unchecked_scan_accounts( + "", + &ancestors, + |_pubkey, _index| num += 1, + COLLECT_ALL_UNSORTED_FALSE, + ); assert_eq!(num, 0); } @@ -2666,7 +2732,12 @@ pub mod tests { assert!(index.get(&key.pubkey(), None, None).is_none()); let mut num = 0; - index.unchecked_scan_accounts("", &ancestors, |_pubkey, _index| num += 1); + index.unchecked_scan_accounts( + "", + &ancestors, + |_pubkey, _index| num += 1, + COLLECT_ALL_UNSORTED_FALSE, + ); assert_eq!(num, 0); } @@ -2699,12 +2770,22 @@ pub mod tests { assert!(index.get(pubkey, None, None).is_none()); let mut num = 0; - index.unchecked_scan_accounts("", &ancestors, |_pubkey, _index| num += 1); + index.unchecked_scan_accounts( + "", + &ancestors, + |_pubkey, _index| num += 1, + COLLECT_ALL_UNSORTED_FALSE, + ); assert_eq!(num, 0); ancestors.insert(slot, 0); assert!(index.get(pubkey, Some(&ancestors), None).is_some()); assert_eq!(index.ref_count_from_storage(pubkey), 1); - index.unchecked_scan_accounts("", &ancestors, |_pubkey, _index| num += 1); + index.unchecked_scan_accounts( + "", + &ancestors, + |_pubkey, _index| num += 1, + COLLECT_ALL_UNSORTED_FALSE, + ); assert_eq!(num, 1); // not zero lamports @@ -2718,12 +2799,22 @@ pub mod tests { assert!(index.get(pubkey, None, None).is_none()); let mut num = 0; - index.unchecked_scan_accounts("", &ancestors, |_pubkey, _index| num += 1); + index.unchecked_scan_accounts( + "", + &ancestors, + |_pubkey, _index| num += 1, + COLLECT_ALL_UNSORTED_FALSE, + ); assert_eq!(num, 0); ancestors.insert(slot, 0); assert!(index.get(pubkey, Some(&ancestors), None).is_some()); assert_eq!(index.ref_count_from_storage(pubkey), 0); // cached, so 0 - index.unchecked_scan_accounts("", &ancestors, |_pubkey, _index| num += 1); + index.unchecked_scan_accounts( + "", + &ancestors, + |_pubkey, _index| num += 1, + COLLECT_ALL_UNSORTED_FALSE, + ); assert_eq!(num, 1); } @@ -2915,11 +3006,21 @@ pub mod tests { assert!(index.get(&key.pubkey(), None, None).is_none()); let mut num = 0; - index.unchecked_scan_accounts("", &ancestors, |_pubkey, _index| num += 1); + index.unchecked_scan_accounts( + "", + &ancestors, + |_pubkey, _index| num += 1, + COLLECT_ALL_UNSORTED_FALSE, + ); assert_eq!(num, 0); ancestors.insert(slot, 0); assert!(index.get(&key.pubkey(), Some(&ancestors), None).is_some()); - index.unchecked_scan_accounts("", &ancestors, |_pubkey, _index| num += 1); + index.unchecked_scan_accounts( + "", + &ancestors, + |_pubkey, _index| num += 1, + COLLECT_ALL_UNSORTED_FALSE, + ); assert_eq!(num, 1); } @@ -2944,7 +3045,12 @@ pub mod tests { assert!(index.get(&key.pubkey(), Some(&ancestors), None).is_none()); let mut num = 0; - index.unchecked_scan_accounts("", &ancestors, |_pubkey, _index| num += 1); + index.unchecked_scan_accounts( + "", + &ancestors, + |_pubkey, _index| num += 1, + COLLECT_ALL_UNSORTED_FALSE, + ); assert_eq!(num, 0); } @@ -2971,12 +3077,17 @@ pub mod tests { let mut num = 0; let mut found_key = false; - index.unchecked_scan_accounts("", &ancestors, |pubkey, _index| { - if pubkey == &key.pubkey() { - found_key = true - }; - num += 1 - }); + index.unchecked_scan_accounts( + "", + &ancestors, + |pubkey, _index| { + if pubkey == &key.pubkey() { + found_key = true + }; + num += 1 + }, + COLLECT_ALL_UNSORTED_FALSE, + ); assert_eq!(num, 1); assert!(found_key); } @@ -3044,9 +3155,15 @@ pub mod tests { let ancestors = Ancestors::default(); let mut scanned_keys = HashSet::new(); - index.range_scan_accounts("", &ancestors, pubkey_range, |pubkey, _index| { - scanned_keys.insert(*pubkey); - }); + index.range_scan_accounts( + "", + &ancestors, + pubkey_range, + COLLECT_ALL_UNSORTED_FALSE, + |pubkey, _index| { + scanned_keys.insert(*pubkey); + }, + ); let mut expected_len = 0; for key in &pubkeys[index_start..index_end] { @@ -3115,9 +3232,14 @@ pub mod tests { let ancestors = Ancestors::default(); let mut scanned_keys = HashSet::new(); - index.unchecked_scan_accounts("", &ancestors, |pubkey, _index| { - scanned_keys.insert(*pubkey); - }); + index.unchecked_scan_accounts( + "", + &ancestors, + |pubkey, _index| { + scanned_keys.insert(*pubkey); + }, + COLLECT_ALL_UNSORTED_FALSE, + ); assert_eq!(scanned_keys.len(), num_pubkeys); } @@ -3133,7 +3255,7 @@ pub mod tests { #[test] fn test_accounts_iter_finished() { let (index, _) = setup_accounts_index_keys(0); - let mut iter = index.iter(None::>); + let mut iter = index.iter(None::>, COLLECT_ALL_UNSORTED_FALSE); assert!(iter.next().is_none()); let mut gc = vec![]; index.upsert( @@ -3412,13 +3534,18 @@ pub mod tests { let mut num = 0; let mut found_key = false; - index.unchecked_scan_accounts("", &Ancestors::default(), |pubkey, _index| { - if pubkey == &key.pubkey() { - found_key = true; - assert_eq!(_index, (&true, 3)); - }; - num += 1 - }); + index.unchecked_scan_accounts( + "", + &Ancestors::default(), + |pubkey, _index| { + if pubkey == &key.pubkey() { + found_key = true; + assert_eq!(_index, (&true, 3)); + }; + num += 1 + }, + COLLECT_ALL_UNSORTED_FALSE, + ); assert_eq!(num, 1); assert!(found_key); } @@ -3976,20 +4103,40 @@ pub mod tests { #[test] fn test_bin_start_and_range() { let index = AccountsIndex::::default_for_tests(); - let iter = AccountsIndexIterator::new(&index, None::>); + let iter = AccountsIndexIterator::new( + &index, + None::>, + COLLECT_ALL_UNSORTED_FALSE, + ); assert_eq!((0, usize::MAX), iter.bin_start_and_range()); let key_0 = Pubkey::new(&[0; 32]); let key_ff = Pubkey::new(&[0xff; 32]); - let iter = AccountsIndexIterator::new(&index, Some(RangeInclusive::new(key_0, key_ff))); + let iter = AccountsIndexIterator::new( + &index, + Some(RangeInclusive::new(key_0, key_ff)), + COLLECT_ALL_UNSORTED_FALSE, + ); let bins = index.bins(); assert_eq!((0, bins), iter.bin_start_and_range()); - let iter = AccountsIndexIterator::new(&index, Some(RangeInclusive::new(key_ff, key_0))); + let iter = AccountsIndexIterator::new( + &index, + Some(RangeInclusive::new(key_ff, key_0)), + COLLECT_ALL_UNSORTED_FALSE, + ); assert_eq!((bins - 1, 0), iter.bin_start_and_range()); - let iter = AccountsIndexIterator::new(&index, Some((Included(key_0), Unbounded))); + let iter = AccountsIndexIterator::new( + &index, + Some((Included(key_0), Unbounded)), + COLLECT_ALL_UNSORTED_FALSE, + ); assert_eq!((0, usize::MAX), iter.bin_start_and_range()); - let iter = AccountsIndexIterator::new(&index, Some((Included(key_ff), Unbounded))); + let iter = AccountsIndexIterator::new( + &index, + Some((Included(key_ff), Unbounded)), + COLLECT_ALL_UNSORTED_FALSE, + ); assert_eq!((bins - 1, usize::MAX), iter.bin_start_and_range()); assert_eq!( @@ -4006,30 +4153,58 @@ pub mod tests { fn test_start_end_bin() { let index = AccountsIndex::::default_for_tests(); assert_eq!(index.bins(), BINS_DEFAULT); - let iter = AccountsIndexIterator::new(&index, None::>); + let iter = AccountsIndexIterator::new( + &index, + None::>, + COLLECT_ALL_UNSORTED_FALSE, + ); assert_eq!(iter.start_bin(), 0); // no range, so 0 assert_eq!(iter.end_bin_inclusive(), usize::MAX); // no range, so max let key = Pubkey::new(&[0; 32]); - let iter = AccountsIndexIterator::new(&index, Some(RangeInclusive::new(key, key))); + let iter = AccountsIndexIterator::new( + &index, + Some(RangeInclusive::new(key, key)), + COLLECT_ALL_UNSORTED_FALSE, + ); assert_eq!(iter.start_bin(), 0); // start at pubkey 0, so 0 assert_eq!(iter.end_bin_inclusive(), 0); // end at pubkey 0, so 0 - let iter = AccountsIndexIterator::new(&index, Some((Included(key), Excluded(key)))); + let iter = AccountsIndexIterator::new( + &index, + Some((Included(key), Excluded(key))), + COLLECT_ALL_UNSORTED_FALSE, + ); assert_eq!(iter.start_bin(), 0); // start at pubkey 0, so 0 assert_eq!(iter.end_bin_inclusive(), 0); // end at pubkey 0, so 0 - let iter = AccountsIndexIterator::new(&index, Some((Excluded(key), Excluded(key)))); + let iter = AccountsIndexIterator::new( + &index, + Some((Excluded(key), Excluded(key))), + COLLECT_ALL_UNSORTED_FALSE, + ); assert_eq!(iter.start_bin(), 0); // start at pubkey 0, so 0 assert_eq!(iter.end_bin_inclusive(), 0); // end at pubkey 0, so 0 let key = Pubkey::new(&[0xff; 32]); - let iter = AccountsIndexIterator::new(&index, Some(RangeInclusive::new(key, key))); + let iter = AccountsIndexIterator::new( + &index, + Some(RangeInclusive::new(key, key)), + COLLECT_ALL_UNSORTED_FALSE, + ); let bins = index.bins(); assert_eq!(iter.start_bin(), bins - 1); // start at highest possible pubkey, so bins - 1 assert_eq!(iter.end_bin_inclusive(), bins - 1); - let iter = AccountsIndexIterator::new(&index, Some((Included(key), Excluded(key)))); + let iter = AccountsIndexIterator::new( + &index, + Some((Included(key), Excluded(key))), + COLLECT_ALL_UNSORTED_FALSE, + ); assert_eq!(iter.start_bin(), bins - 1); // start at highest possible pubkey, so bins - 1 assert_eq!(iter.end_bin_inclusive(), bins - 1); - let iter = AccountsIndexIterator::new(&index, Some((Excluded(key), Excluded(key)))); + let iter = AccountsIndexIterator::new( + &index, + Some((Excluded(key), Excluded(key))), + COLLECT_ALL_UNSORTED_FALSE, + ); assert_eq!(iter.start_bin(), bins - 1); // start at highest possible pubkey, so bins - 1 assert_eq!(iter.end_bin_inclusive(), bins - 1); }