From 2970b598537b0715c8dfcad4a8c73ab78d8d7a75 Mon Sep 17 00:00:00 2001 From: carllin Date: Wed, 3 Feb 2021 15:00:42 -0800 Subject: [PATCH] Don't load all accounts into memory for capitalization check (#14957) * Don't load all accounts into memory for capitalization check --- runtime/src/accounts.rs | 51 +++++++++---------- runtime/src/accounts_db.rs | 64 +++++++++++++++-------- runtime/src/accounts_index.rs | 95 ++++++++++++++++++++++++++++------- 3 files changed, 143 insertions(+), 67 deletions(-) diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index b2a6ebcec6..8affdbf6fe 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -547,19 +547,26 @@ impl Accounts { ancestors: &Ancestors, simple_capitalization_enabled: bool, ) -> u64 { - let balances = - self.load_all_unchecked(ancestors) - .into_iter() - .map(|(_pubkey, account, _slot)| { - AccountsDB::account_balance_for_capitalization( - account.lamports, - &account.owner, - account.executable, + self.accounts_db.unchecked_scan_accounts( + "calculate_capitalization_scan_elapsed", + ancestors, + |total_capitalization: &mut u64, (_pubkey, loaded_account, _slot)| { + let lamports = loaded_account.lamports(); + if Self::is_loadable(lamports) { + let account_cap = AccountsDB::account_balance_for_capitalization( + lamports, + &loaded_account.owner(), + loaded_account.executable(), simple_capitalization_enabled, - ) - }); + ); - AccountsDB::checked_sum_for_capitalization(balances) + *total_capitalization = AccountsDB::checked_iterative_sum_for_capitalization( + *total_capitalization, + account_cap, + ); + } + }, + ) } #[must_use] @@ -583,10 +590,10 @@ impl Accounts { } } - fn is_loadable(account: &Account) -> bool { + fn is_loadable(lamports: u64) -> bool { // Don't ever load zero lamport accounts into runtime because // the existence of zero-lamport accounts are never deterministic!! - account.lamports > 0 + lamports > 0 } fn load_while_filtering bool>( @@ -595,7 +602,7 @@ impl Accounts { filter: F, ) { if let Some(mapped_account_tuple) = some_account_tuple - .filter(|(_, account, _)| Self::is_loadable(account) && filter(account)) + .filter(|(_, account, _)| Self::is_loadable(account.lamports) && filter(account)) .map(|(pubkey, account, _slot)| (*pubkey, account)) { collector.push(mapped_account_tuple) @@ -653,20 +660,7 @@ impl Accounts { ancestors, |collector: &mut Vec<(Pubkey, Account, Slot)>, some_account_tuple| { if let Some((pubkey, account, slot)) = - some_account_tuple.filter(|(_, account, _)| Self::is_loadable(account)) - { - collector.push((*pubkey, account, slot)) - } - }, - ) - } - - fn load_all_unchecked(&self, ancestors: &Ancestors) -> Vec<(Pubkey, Account, Slot)> { - self.accounts_db.unchecked_scan_accounts( - ancestors, - |collector: &mut Vec<(Pubkey, Account, Slot)>, some_account_tuple| { - if let Some((pubkey, account, slot)) = - some_account_tuple.filter(|(_, account, _)| Self::is_loadable(account)) + some_account_tuple.filter(|(_, account, _)| Self::is_loadable(account.lamports)) { collector.push((*pubkey, account, slot)) } @@ -680,6 +674,7 @@ impl Accounts { range: R, ) -> Vec<(Pubkey, Account)> { self.accounts_db.range_scan_accounts( + "load_to_collect_rent_eagerly_scan_elapsed", ancestors, range, |collector: &mut Vec<(Pubkey, Account)>, option| { diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index d7951e9879..7afee0922e 100644 --- a/runtime/src/accounts_db.rs +++ b/runtime/src/accounts_db.rs @@ -270,6 +270,13 @@ impl<'a> LoadedAccount<'a> { } } + pub fn lamports(&self) -> u64 { + match self { + LoadedAccount::Stored(stored_account_meta) => stored_account_meta.account_meta.lamports, + LoadedAccount::Cached((_, cached_account)) => cached_account.account.lamports, + } + } + pub fn account(self) -> Account { match self { LoadedAccount::Stored(stored_account_meta) => stored_account_meta.clone_account(), @@ -1921,15 +1928,22 @@ impl AccountsDB { collector } - pub fn unchecked_scan_accounts(&self, ancestors: &Ancestors, scan_func: F) -> A + pub fn unchecked_scan_accounts( + &self, + metric_name: &'static str, + ancestors: &Ancestors, + scan_func: F, + ) -> A where - F: Fn(&mut A, Option<(&Pubkey, Account, Slot)>), + F: Fn(&mut A, (&Pubkey, LoadedAccount, Slot)), A: Default, { let mut collector = A::default(); - self.accounts_index - .unchecked_scan_accounts(ancestors, |pubkey, (account_info, slot)| { - let account_slot = self + self.accounts_index.unchecked_scan_accounts( + metric_name, + ancestors, + |pubkey, (account_info, slot)| { + if let Some(loaded_account) = self .get_account_accessor_from_cache_or_storage( slot, pubkey, @@ -1937,13 +1951,21 @@ impl AccountsDB { account_info.offset, ) .get_loaded_account() - .map(|loaded_account| (pubkey, loaded_account.account(), slot)); - scan_func(&mut collector, account_slot) - }); + { + scan_func(&mut collector, (pubkey, loaded_account, slot)); + } + }, + ); collector } - pub fn range_scan_accounts(&self, ancestors: &Ancestors, range: R, scan_func: F) -> A + pub fn range_scan_accounts( + &self, + metric_name: &'static str, + ancestors: &Ancestors, + range: R, + scan_func: F, + ) -> A where F: Fn(&mut A, Option<(&Pubkey, Account, Slot)>), A: Default, @@ -1951,6 +1973,7 @@ impl AccountsDB { { let mut collector = A::default(); self.accounts_index.range_scan_accounts( + metric_name, ancestors, range, |pubkey, (account_info, slot)| { @@ -3415,6 +3438,11 @@ impl AccountsDB { .expect("overflow is detected while summing capitalization") } + pub fn checked_iterative_sum_for_capitalization(total_cap: u64, new_cap: u64) -> u64 { + let new_total = total_cap as u128 + new_cap as u128; + Self::checked_cast_for_capitalization(new_total) + } + pub fn checked_sum_for_capitalization>(balances: T) -> u64 { Self::checked_cast_for_capitalization(balances.map(|b| b as u128).sum::()) } @@ -4890,10 +4918,8 @@ pub mod tests { assert_eq!(&db.load_slow(&ancestors, &key).unwrap().0, &account1); let accounts: Vec = - db.unchecked_scan_accounts(&ancestors, |accounts: &mut Vec, option| { - if let Some(data) = option { - accounts.push(data.1); - } + db.unchecked_scan_accounts("", &ancestors, |accounts: &mut Vec, option| { + accounts.push(option.1.account()); }); assert_eq!(accounts, vec![account1]); } @@ -6243,19 +6269,15 @@ pub mod tests { let ancestors = vec![(0, 0)].into_iter().collect(); let accounts: Vec = - db.unchecked_scan_accounts(&ancestors, |accounts: &mut Vec, option| { - if let Some(data) = option { - accounts.push(data.1); - } + db.unchecked_scan_accounts("", &ancestors, |accounts: &mut Vec, option| { + accounts.push(option.1.account()); }); assert_eq!(accounts, vec![account0]); let ancestors = vec![(1, 1), (0, 0)].into_iter().collect(); let accounts: Vec = - db.unchecked_scan_accounts(&ancestors, |accounts: &mut Vec, option| { - if let Some(data) = option { - accounts.push(data.1); - } + db.unchecked_scan_accounts("", &ancestors, |accounts: &mut Vec, option| { + accounts.push(option.1.account()); }); assert_eq!(accounts.len(), 2); } diff --git a/runtime/src/accounts_index.rs b/runtime/src/accounts_index.rs index a5a017f2ff..de453fb2e4 100644 --- a/runtime/src/accounts_index.rs +++ b/runtime/src/accounts_index.rs @@ -5,6 +5,7 @@ use crate::{ }; use dashmap::DashSet; use ouroboros::self_referencing; +use solana_measure::measure::Measure; use solana_sdk::{ clock::Slot, pubkey::{Pubkey, PUBKEY_BYTES}, @@ -260,6 +261,7 @@ impl AccountsIndex { fn do_checked_scan_accounts( &self, + metric_name: &'static str, ancestors: &Ancestors, func: F, scan_type: ScanTypes, @@ -402,7 +404,8 @@ impl AccountsIndex { */ match scan_type { ScanTypes::Unindexed(range) => { - self.do_scan_accounts(ancestors, func, range, Some(max_root)); + // Pass "" not to log metrics, so RPC doesn't get spammy + self.do_scan_accounts(metric_name, ancestors, func, range, Some(max_root)); } ScanTypes::Indexed(IndexKey::ProgramId(program_id)) => { self.do_scan_secondary_index( @@ -443,12 +446,17 @@ impl AccountsIndex { } } - fn do_unchecked_scan_accounts(&self, ancestors: &Ancestors, func: F, range: Option) - where + fn do_unchecked_scan_accounts( + &self, + metric_name: &'static str, + ancestors: &Ancestors, + func: F, + range: Option, + ) where F: FnMut(&Pubkey, (&T, Slot)), R: RangeBounds, { - self.do_scan_accounts(ancestors, func, range, None); + self.do_scan_accounts(metric_name, ancestors, func, range, None); } // Scan accounts and return latest version of each account that is either: @@ -456,6 +464,7 @@ impl AccountsIndex { // 2) present in ancestors fn do_scan_accounts( &self, + metric_name: &'static str, ancestors: &Ancestors, mut func: F, range: Option, @@ -466,13 +475,46 @@ impl AccountsIndex { { // TODO: expand to use mint index to find the `pubkey_list` below more efficiently // instead of scanning the entire range + let mut total_elapsed_timer = Measure::start("total"); + let mut num_keys_iterated = 0; + let mut latest_slot_elapsed = 0; + let mut load_account_elapsed = 0; + 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) { + iterator_timer.stop(); + iterator_elapsed += iterator_timer.as_us(); for (pubkey, list) in pubkey_list { + num_keys_iterated += 1; + let mut read_lock_timer = Measure::start("read_lock"); let list_r = &list.slot_list.read().unwrap(); + read_lock_timer.stop(); + read_lock_elapsed += read_lock_timer.as_us(); + let mut latest_slot_timer = Measure::start("latest_slot"); if let Some(index) = self.latest_slot(Some(ancestors), &list_r, max_root) { + latest_slot_timer.stop(); + latest_slot_elapsed += latest_slot_timer.as_us(); + let mut load_account_timer = Measure::start("load_account"); func(&pubkey, (&list_r[index].1, list_r[index].0)); + load_account_timer.stop(); + load_account_elapsed += load_account_timer.as_us(); } } + iterator_timer = Measure::start("iterator_elapsed"); + } + + total_elapsed_timer.stop(); + if !metric_name.is_empty() { + datapoint_info!( + metric_name, + ("total_elapsed", total_elapsed_timer.as_us(), i64), + ("latest_slot_elapsed", latest_slot_elapsed, i64), + ("read_lock_elapsed", read_lock_elapsed, i64), + ("load_account_elapsed", load_account_elapsed, i64), + ("iterator_elapsed", iterator_elapsed, i64), + ("num_keys_iterated", num_keys_iterated, i64), + ) } } @@ -577,24 +619,39 @@ impl AccountsIndex { where F: FnMut(&Pubkey, (&T, Slot)), { - self.do_checked_scan_accounts(ancestors, func, ScanTypes::Unindexed(None::>)); + // Pass "" not to log metrics, so RPC doesn't get spammy + self.do_checked_scan_accounts( + "", + ancestors, + func, + ScanTypes::Unindexed(None::>), + ); } - pub(crate) fn unchecked_scan_accounts(&self, ancestors: &Ancestors, func: F) - where + pub(crate) fn unchecked_scan_accounts( + &self, + metric_name: &'static str, + ancestors: &Ancestors, + func: F, + ) where F: FnMut(&Pubkey, (&T, Slot)), { - self.do_unchecked_scan_accounts(ancestors, func, None::>); + self.do_unchecked_scan_accounts(metric_name, ancestors, func, None::>); } /// call func with every pubkey and index visible from a given set of ancestors with range - pub(crate) fn range_scan_accounts(&self, ancestors: &Ancestors, range: R, func: F) - where + pub(crate) fn range_scan_accounts( + &self, + metric_name: &'static str, + ancestors: &Ancestors, + range: R, + 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(ancestors, func, Some(range)); + self.do_unchecked_scan_accounts(metric_name, ancestors, func, Some(range)); } /// call func with every pubkey and index visible from a given set of ancestors @@ -602,7 +659,9 @@ impl AccountsIndex { where F: FnMut(&Pubkey, (&T, Slot)), { + // Pass "" not to log metrics, so RPC doesn't get spammy self.do_checked_scan_accounts( + "", ancestors, func, ScanTypes::>::Indexed(index_key), @@ -1128,7 +1187,7 @@ 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); assert_eq!(num, 0); } @@ -1153,7 +1212,7 @@ 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); assert_eq!(num, 0); } @@ -1177,7 +1236,7 @@ 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); assert_eq!(num, 0); } @@ -1203,7 +1262,7 @@ pub mod tests { let mut num = 0; let mut found_key = false; - index.unchecked_scan_accounts(&ancestors, |pubkey, _index| { + index.unchecked_scan_accounts("", &ancestors, |pubkey, _index| { if pubkey == &key.pubkey() { found_key = true }; @@ -1274,7 +1333,7 @@ pub mod tests { let ancestors: Ancestors = HashMap::new(); let mut scanned_keys = HashSet::new(); - index.range_scan_accounts(&ancestors, pubkey_range, |pubkey, _index| { + index.range_scan_accounts("", &ancestors, pubkey_range, |pubkey, _index| { scanned_keys.insert(*pubkey); }); @@ -1345,7 +1404,7 @@ pub mod tests { let ancestors: Ancestors = HashMap::new(); let mut scanned_keys = HashSet::new(); - index.unchecked_scan_accounts(&ancestors, |pubkey, _index| { + index.unchecked_scan_accounts("", &ancestors, |pubkey, _index| { scanned_keys.insert(*pubkey); }); assert_eq!(scanned_keys.len(), num_pubkeys); @@ -1631,7 +1690,7 @@ pub mod tests { let mut num = 0; let mut found_key = false; - index.unchecked_scan_accounts(&Ancestors::new(), |pubkey, _index| { + index.unchecked_scan_accounts("", &Ancestors::new(), |pubkey, _index| { if pubkey == &key.pubkey() { found_key = true; assert_eq!(_index, (&true, 3));