From d4cc975fe90ef93a15d477318dd6e21b526c7e37 Mon Sep 17 00:00:00 2001 From: "Jeff Washington (jwash)" <75863576+jeffwashington@users.noreply.github.com> Date: Mon, 14 Jun 2021 08:53:07 -0500 Subject: [PATCH] calculate_capitalization uses hash calculation (#17443) * calculate_capitalization uses hash calculation * feedback * remove debugging code, clean up slot math --- accounts-bench/src/main.rs | 1 + ledger-tool/src/main.rs | 3 +- ledger/src/blockstore_processor.rs | 6 +- runtime/src/accounts.rs | 32 ++++---- runtime/src/accounts_cache.rs | 4 + runtime/src/accounts_db.rs | 113 +++++++++++++++++++++++++---- runtime/src/accounts_index.rs | 4 + runtime/src/bank.rs | 54 ++++++++------ runtime/src/snapshot_utils.rs | 1 + runtime/src/sorted_storages.rs | 37 ++++++++-- 10 files changed, 194 insertions(+), 61 deletions(-) diff --git a/accounts-bench/src/main.rs b/accounts-bench/src/main.rs index 469b7ccc60..e32c01f1d8 100644 --- a/accounts-bench/src/main.rs +++ b/accounts-bench/src/main.rs @@ -121,6 +121,7 @@ fn main() { solana_sdk::clock::Slot::default(), &ancestors, None, + false, ); time_store.stop(); if results != results_store { diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index 1084e69047..4a3aaf047b 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -783,7 +783,8 @@ fn open_genesis_config_by(ledger_path: &Path, matches: &ArgMatches<'_>) -> Genes } fn assert_capitalization(bank: &Bank) { - assert!(bank.calculate_and_verify_capitalization()); + let debug_verify = true; + assert!(bank.calculate_and_verify_capitalization(debug_verify)); } #[allow(clippy::cognitive_complexity)] diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index f032960839..17e28e715f 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -558,10 +558,14 @@ fn do_process_blockstore_from_root( ); assert!(bank_forks.active_banks().is_empty()); + let debug_verify = false; // We might be promptly restarted after bad capitalization was detected while creating newer snapshot. // In that case, we're most likely restored from the last good snapshot and replayed up to this root. // So again check here for the bad capitalization to avoid to continue until the next snapshot creation. - if !bank_forks.root_bank().calculate_and_verify_capitalization() { + if !bank_forks + .root_bank() + .calculate_and_verify_capitalization(debug_verify) + { return Err(BlockstoreProcessorError::RootBankWithMismatchedCapitalization(root)); } diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 1767a75374..4deb309bd6 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -627,20 +627,24 @@ impl Accounts { .collect() } - pub fn calculate_capitalization(&self, ancestors: &Ancestors) -> u64 { - 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) { - *total_capitalization = AccountsDb::checked_iterative_sum_for_capitalization( - *total_capitalization, - lamports, - ); - } - }, - ) + pub fn calculate_capitalization( + &self, + ancestors: &Ancestors, + slot: Slot, + can_cached_slot_be_unflushed: bool, + debug_verify: bool, + ) -> u64 { + let use_index = false; + self.accounts_db + .update_accounts_hash_with_index_option( + use_index, + debug_verify, + slot, + ancestors, + None, + can_cached_slot_be_unflushed, + ) + .1 } #[must_use] diff --git a/runtime/src/accounts_cache.rs b/runtime/src/accounts_cache.rs index 08a4e7eb17..8896a1eea0 100644 --- a/runtime/src/accounts_cache.rs +++ b/runtime/src/accounts_cache.rs @@ -48,6 +48,10 @@ impl SlotCacheInner { ); } + pub fn get_all_pubkeys(&self) -> Vec { + self.cache.iter().map(|item| *item.key()).collect() + } + pub fn insert( &self, pubkey: &Pubkey, diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index 8b61a41850..a39c0b7899 100644 --- a/runtime/src/accounts_db.rs +++ b/runtime/src/accounts_db.rs @@ -4451,11 +4451,11 @@ impl AccountsDb { } pub fn update_accounts_hash(&self, slot: Slot, ancestors: &Ancestors) -> (Hash, u64) { - self.update_accounts_hash_with_index_option(true, false, slot, ancestors, None) + self.update_accounts_hash_with_index_option(true, false, slot, ancestors, None, false) } pub fn update_accounts_hash_test(&self, slot: Slot, ancestors: &Ancestors) -> (Hash, u64) { - self.update_accounts_hash_with_index_option(true, true, slot, ancestors, None) + self.update_accounts_hash_with_index_option(true, true, slot, ancestors, None, false) } fn scan_multiple_account_storages_one_slot( @@ -4512,6 +4512,11 @@ impl AccountsDb { /// Scan through all the account storage in parallel fn scan_account_storage_no_bank( + accounts_cache_and_ancestors: Option<( + &AccountsCache, + &Ancestors, + &AccountInfoAccountsIndex, + )>, snapshot_storages: &SortedStorages, scan_func: F, ) -> Vec @@ -4531,7 +4536,9 @@ impl AccountsDb { let end = std::cmp::min(start + MAX_ITEMS_PER_CHUNK, snapshot_storages.range().end); for slot in start..end { let sub_storages = snapshot_storages.get(slot); + let mut valid_slot = false; if let Some(sub_storages) = sub_storages { + valid_slot = true; Self::scan_multiple_account_storages_one_slot( sub_storages, &scan_func, @@ -4539,6 +4546,26 @@ impl AccountsDb { &mut retval, ); } + if let Some((cache, ancestors, accounts_index)) = accounts_cache_and_ancestors { + if let Some(slot_cache) = cache.slot_cache(slot) { + if valid_slot + || ancestors.contains_key(&slot) + || accounts_index.is_root(slot) + { + let keys = slot_cache.get_all_pubkeys(); + for key in keys { + if let Some(cached_account) = slot_cache.get_cloned(&key) { + let mut accessor = LoadedAccountAccessor::Cached(Some(( + key, + Cow::Owned(cached_account), + ))); + let account = accessor.get_loaded_account().unwrap(); + scan_func(account, &mut retval, slot); + }; + } + } + } + } } retval }) @@ -4551,14 +4578,26 @@ impl AccountsDb { slot: Slot, ancestors: &Ancestors, check_hash: bool, + can_cached_slot_be_unflushed: bool, ) -> Result<(Hash, u64), BankHashVerificationError> { if !use_index { + let accounts_cache_and_ancestors = if can_cached_slot_be_unflushed { + Some((&self.accounts_cache, ancestors, &self.accounts_index)) + } else { + None + }; + let mut collect_time = Measure::start("collect"); let (combined_maps, slots) = self.get_snapshot_storages(slot, Some(ancestors)); collect_time.stop(); let mut sort_time = Measure::start("sort_storages"); - let storages = SortedStorages::new_with_slots(combined_maps.iter().zip(slots.iter())); + let min_root = self.accounts_index.min_root(); + let storages = SortedStorages::new_with_slots( + combined_maps.iter().zip(slots.iter()), + min_root, + Some(slot), + ); sort_time.stop(); let timings = HashStats { @@ -4572,6 +4611,7 @@ impl AccountsDb { Some(&self.thread_pool_clean), timings, check_hash, + accounts_cache_and_ancestors, ) } else { self.calculate_accounts_hash(slot, ancestors, check_hash) @@ -4585,15 +4625,28 @@ impl AccountsDb { slot: Slot, ancestors: &Ancestors, expected_capitalization: Option, + can_cached_slot_be_unflushed: bool, ) -> (Hash, u64) { let check_hash = false; let (hash, total_lamports) = self - .calculate_accounts_hash_helper(use_index, slot, ancestors, check_hash) + .calculate_accounts_hash_helper( + use_index, + slot, + ancestors, + check_hash, + can_cached_slot_be_unflushed, + ) .unwrap(); // unwrap here will never fail since check_hash = false if debug_verify { // calculate the other way (store or non-store) and verify results match. let (hash_other, total_lamports_other) = self - .calculate_accounts_hash_helper(!use_index, slot, ancestors, check_hash) + .calculate_accounts_hash_helper( + !use_index, + slot, + ancestors, + check_hash, + can_cached_slot_be_unflushed, + ) .unwrap(); // unwrap here will never fail since check_hash = false let success = hash == hash_other @@ -4607,12 +4660,17 @@ impl AccountsDb { (hash, total_lamports) } - fn scan_snapshot_stores( + fn scan_snapshot_stores_with_cache( storage: &SortedStorages, mut stats: &mut crate::accounts_hash::HashStats, bins: usize, bin_range: &Range, check_hash: bool, + accounts_cache_and_ancestors: Option<( + &AccountsCache, + &Ancestors, + &AccountInfoAccountsIndex, + )>, ) -> Result>>, BankHashVerificationError> { let bin_calculator = PubkeyBinCalculator16::new(bins); assert!(bin_range.start < bins && bin_range.end <= bins && bin_range.start < bin_range.end); @@ -4621,6 +4679,7 @@ impl AccountsDb { let mismatch_found = AtomicU64::new(0); let result: Vec>> = Self::scan_account_storage_no_bank( + accounts_cache_and_ancestors, storage, |loaded_account: LoadedAccount, accum: &mut Vec>, @@ -4685,6 +4744,11 @@ impl AccountsDb { thread_pool: Option<&ThreadPool>, mut stats: HashStats, check_hash: bool, + accounts_cache_and_ancestors: Option<( + &AccountsCache, + &Ancestors, + &AccountInfoAccountsIndex, + )>, ) -> Result<(Hash, u64), BankHashVerificationError> { let mut scan_and_hash = move || { // When calculating hashes, it is helpful to break the pubkeys found into bins based on the pubkey value. @@ -4711,12 +4775,13 @@ impl AccountsDb { end: (pass + 1) * bins_per_pass, }; - let result = Self::scan_snapshot_stores( + let result = Self::scan_snapshot_stores_with_cache( &storages, &mut stats, PUBKEY_BINS_FOR_CALCULATING_HASHES, &bounds, check_hash, + accounts_cache_and_ancestors, )?; let (hash, lamports, for_next_pass) = AccountsHash::rest_of_hash_calculation( @@ -4746,7 +4811,7 @@ impl AccountsDb { use BankHashVerificationError::*; let (calculated_hash, calculated_lamports) = - self.calculate_accounts_hash_helper(true, slot, ancestors, true)?; + self.calculate_accounts_hash_helper(true, slot, ancestors, true, false)?; if calculated_lamports != total_lamports { warn!( @@ -5506,7 +5571,7 @@ impl AccountsDb { pub fn get_snapshot_storages( &self, snapshot_slot: Slot, - _ancestors: Option<&Ancestors>, + ancestors: Option<&Ancestors>, ) -> (SnapshotStorages, Vec) { let mut m = Measure::start("get slots"); let slots = self @@ -5526,7 +5591,12 @@ impl AccountsDb { slots .iter() .filter_map(|slot| { - if *slot <= snapshot_slot && self.accounts_index.is_root(*slot) { + if *slot <= snapshot_slot + && (self.accounts_index.is_root(*slot) + || ancestors + .map(|ancestors| ancestors.contains_key(&slot)) + .unwrap_or_default()) + { self.storage.0.get(&slot).map_or_else( || None, |item| { @@ -6033,6 +6103,18 @@ pub mod tests { SortedStorages::new(&[]) } + impl AccountsDb { + fn scan_snapshot_stores( + storage: &SortedStorages, + stats: &mut crate::accounts_hash::HashStats, + bins: usize, + bin_range: &Range, + check_hash: bool, + ) -> Result>>, BankHashVerificationError> { + Self::scan_snapshot_stores_with_cache(storage, stats, bins, bin_range, check_hash, None) + } + } + #[test] #[should_panic( expected = "bin_range.start < bins && bin_range.end <= bins &&\\n bin_range.start < bin_range.end" @@ -6368,6 +6450,7 @@ pub mod tests { None, HashStats::default(), false, + None, ) .unwrap(); let expected_hash = Hash::from_str("GKot5hBsd81kMupNCXHaqbhv3huEbxAFMLnpcX2hniwn").unwrap(); @@ -6389,6 +6472,7 @@ pub mod tests { None, HashStats::default(), false, + None, ) .unwrap(); @@ -6436,6 +6520,7 @@ pub mod tests { let calls = AtomicU64::new(0); let result = AccountsDb::scan_account_storage_no_bank( + None, &get_storage_refs(&storages), |loaded_account: LoadedAccount, accum: &mut Vec, slot: Slot| { calls.fetch_add(1, Ordering::Relaxed); @@ -8478,10 +8563,10 @@ pub mod tests { db.add_root(some_slot); let check_hash = true; assert!(db - .calculate_accounts_hash_helper(false, some_slot, &ancestors, check_hash) + .calculate_accounts_hash_helper(false, some_slot, &ancestors, check_hash, false) .is_err()); assert!(db - .calculate_accounts_hash_helper(true, some_slot, &ancestors, check_hash) + .calculate_accounts_hash_helper(true, some_slot, &ancestors, check_hash, false) .is_err()); } @@ -8501,9 +8586,9 @@ pub mod tests { db.add_root(some_slot); let check_hash = true; assert_eq!( - db.calculate_accounts_hash_helper(false, some_slot, &ancestors, check_hash) + db.calculate_accounts_hash_helper(false, some_slot, &ancestors, check_hash, false) .unwrap(), - db.calculate_accounts_hash_helper(true, some_slot, &ancestors, check_hash) + db.calculate_accounts_hash_helper(true, some_slot, &ancestors, check_hash, false) .unwrap(), ); } diff --git a/runtime/src/accounts_index.rs b/runtime/src/accounts_index.rs index 0f4773a6da..9bfd857d16 100644 --- a/runtime/src/accounts_index.rs +++ b/runtime/src/accounts_index.rs @@ -1550,6 +1550,10 @@ impl AccountsIndex { }) } + pub fn min_root(&self) -> Option { + self.roots_tracker.read().unwrap().min_root() + } + pub fn reset_uncleaned_roots(&self, max_clean_root: Option) -> HashSet { let mut cleaned_roots = HashSet::new(); let mut w_roots_tracker = self.roots_tracker.write().unwrap(); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index c144b0f6c5..bbeb2399c7 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -4568,12 +4568,18 @@ impl Bank { } } - pub fn calculate_capitalization(&self) -> u64 { - self.rc.accounts.calculate_capitalization(&self.ancestors) + pub fn calculate_capitalization(&self, debug_verify: bool) -> u64 { + let can_cached_slot_be_unflushed = true; // implied yes + self.rc.accounts.calculate_capitalization( + &self.ancestors, + self.slot(), + can_cached_slot_be_unflushed, + debug_verify, + ) } - pub fn calculate_and_verify_capitalization(&self) -> bool { - let calculated = self.calculate_capitalization(); + pub fn calculate_and_verify_capitalization(&self, debug_verify: bool) -> bool { + let calculated = self.calculate_capitalization(debug_verify); let expected = self.capitalization(); if calculated == expected { true @@ -4590,8 +4596,9 @@ impl Bank { /// This should only be used for developing purposes. pub fn set_capitalization(&self) -> u64 { let old = self.capitalization(); + let debug_verify = true; self.capitalization - .store(self.calculate_capitalization(), Relaxed); + .store(self.calculate_capitalization(debug_verify), Relaxed); old } @@ -4618,6 +4625,7 @@ impl Bank { self.slot(), &self.ancestors, Some(self.capitalization()), + false, ); if total_lamports != self.capitalization() { datapoint_info!( @@ -5849,7 +5857,7 @@ pub(crate) mod tests { updater(); let new = bank.capitalization(); asserter(old, new); - assert_eq!(bank.capitalization(), bank.calculate_capitalization()); + assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); } #[test] @@ -6145,7 +6153,7 @@ pub(crate) mod tests { burned_portion ); - assert!(bank.calculate_and_verify_capitalization()); + assert!(bank.calculate_and_verify_capitalization(true)); assert_eq!( rent_to_be_distributed, @@ -7321,7 +7329,7 @@ pub(crate) mod tests { )] ); bank1.freeze(); - assert!(bank1.calculate_and_verify_capitalization()); + assert!(bank1.calculate_and_verify_capitalization(true)); } fn do_test_bank_update_rewards_determinism() -> u64 { @@ -7399,7 +7407,7 @@ pub(crate) mod tests { assert_ne!(bank1.capitalization(), bank.capitalization()); bank1.freeze(); - assert!(bank1.calculate_and_verify_capitalization()); + assert!(bank1.calculate_and_verify_capitalization(true)); // verify voting and staking rewards are recorded let rewards = bank1.rewards.read().unwrap(); @@ -8630,7 +8638,7 @@ pub(crate) mod tests { // First, initialize the clock sysvar activate_all_features(&mut genesis_config); let bank1 = Arc::new(Bank::new(&genesis_config)); - assert_eq!(bank1.calculate_capitalization(), bank1.capitalization()); + assert_eq!(bank1.calculate_capitalization(true), bank1.capitalization()); assert_capitalization_diff( &bank1, @@ -9389,7 +9397,7 @@ pub(crate) mod tests { // Non-native loader accounts can not be used for instruction processing assert!(bank.stakes.read().unwrap().vote_accounts().is_empty()); assert!(bank.stakes.read().unwrap().stake_delegations().is_empty()); - assert_eq!(bank.calculate_capitalization(), bank.capitalization()); + assert_eq!(bank.calculate_capitalization(true), bank.capitalization()); let ((vote_id, vote_account), (stake_id, stake_account)) = crate::stakes::tests::create_staked_node_accounts(1_0000); @@ -9399,13 +9407,13 @@ pub(crate) mod tests { bank.store_account(&stake_id, &stake_account); assert!(!bank.stakes.read().unwrap().vote_accounts().is_empty()); assert!(!bank.stakes.read().unwrap().stake_delegations().is_empty()); - assert_eq!(bank.calculate_capitalization(), bank.capitalization()); + assert_eq!(bank.calculate_capitalization(true), bank.capitalization()); bank.add_builtin("mock_program1", vote_id, mock_ix_processor); bank.add_builtin("mock_program2", stake_id, mock_ix_processor); assert!(bank.stakes.read().unwrap().vote_accounts().is_empty()); assert!(bank.stakes.read().unwrap().stake_delegations().is_empty()); - assert_eq!(bank.calculate_capitalization(), bank.capitalization()); + assert_eq!(bank.calculate_capitalization(true), bank.capitalization()); assert_eq!( "mock_program1", String::from_utf8_lossy(&bank.get_account(&vote_id).unwrap_or_default().data()) @@ -9425,7 +9433,7 @@ pub(crate) mod tests { assert_eq!(old_hash, new_hash); assert!(bank.stakes.read().unwrap().vote_accounts().is_empty()); assert!(bank.stakes.read().unwrap().stake_delegations().is_empty()); - assert_eq!(bank.calculate_capitalization(), bank.capitalization()); + assert_eq!(bank.calculate_capitalization(true), bank.capitalization()); assert_eq!( "mock_program1", String::from_utf8_lossy(&bank.get_account(&vote_id).unwrap_or_default().data()) @@ -11037,16 +11045,16 @@ pub(crate) mod tests { let program_id = solana_sdk::pubkey::new_rand(); bank.add_native_program("mock_program", &program_id, false); - assert_eq!(bank.capitalization(), bank.calculate_capitalization()); + assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); // someone mess with program_id's balance bank.withdraw(&mint_keypair.pubkey(), 10).unwrap(); - assert_ne!(bank.capitalization(), bank.calculate_capitalization()); + assert_ne!(bank.capitalization(), bank.calculate_capitalization(true)); bank.deposit(&program_id, 10).unwrap(); - assert_eq!(bank.capitalization(), bank.calculate_capitalization()); + assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); bank.add_native_program("mock_program v2", &program_id, true); - assert_eq!(bank.capitalization(), bank.calculate_capitalization()); + assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); } #[test] @@ -11057,12 +11065,12 @@ pub(crate) mod tests { // someone managed to squat at program_id! bank.withdraw(&mint_keypair.pubkey(), 10).unwrap(); - assert_ne!(bank.capitalization(), bank.calculate_capitalization()); + assert_ne!(bank.capitalization(), bank.calculate_capitalization(true)); bank.deposit(&program_id, 10).unwrap(); - assert_eq!(bank.capitalization(), bank.calculate_capitalization()); + assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); bank.add_native_program("mock_program", &program_id, false); - assert_eq!(bank.capitalization(), bank.calculate_capitalization()); + assert_eq!(bank.capitalization(), bank.calculate_capitalization(true)); } #[test] @@ -11213,13 +11221,13 @@ pub(crate) mod tests { bank0.capitalization() + 1 + 1_000_000_000 + sysvar_and_native_proram_delta, bank1.capitalization() ); - assert_eq!(bank1.capitalization(), bank1.calculate_capitalization()); + assert_eq!(bank1.capitalization(), bank1.calculate_capitalization(true)); // Depending on RUSTFLAGS, this test exposes rust's checked math behavior or not... // So do some convolted setup; anyway this test itself will just be temporary let bank0 = std::panic::AssertUnwindSafe(bank0); let overflowing_capitalization = - std::panic::catch_unwind(|| bank0.calculate_capitalization()); + std::panic::catch_unwind(|| bank0.calculate_capitalization(true)); if let Ok(overflowing_capitalization) = overflowing_capitalization { info!("asserting overflowing capitalization for bank0"); assert_eq!(overflowing_capitalization, bank0.capitalization()); diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 274d259608..a8ec476129 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -1010,6 +1010,7 @@ pub fn process_accounts_package_pre( thread_pool, crate::accounts_hash::HashStats::default(), false, + None, ) .unwrap(); diff --git a/runtime/src/sorted_storages.rs b/runtime/src/sorted_storages.rs index 808faf4e8b..cb87cad55a 100644 --- a/runtime/src/sorted_storages.rs +++ b/runtime/src/sorted_storages.rs @@ -45,7 +45,7 @@ impl<'a> SortedStorages<'a> { storage.slot() // this must be unique. Will be enforced in new_with_slots }) .collect::>(); - Self::new_with_slots(source.iter().zip(slots.iter())) + Self::new_with_slots(source.iter().zip(slots.iter()), None, None) } // source[i] is in slot slots[i] @@ -54,17 +54,34 @@ impl<'a> SortedStorages<'a> { // 2. slots and source are the same len pub fn new_with_slots<'b>( source: impl Iterator + Clone, + // A slot used as a lower bound, but potentially smaller than the smallest slot in the given 'source' iterator + min_slot: Option, + // highest valid slot. Only matters if source array does not contain a slot >= max_slot_inclusive. + // An example is a slot that has accounts in the write cache at slots <= 'max_slot_inclusive' but no storages at those slots. + // None => self.range.end = source.1.max() + 1 + // Some(slot) => self.range.end = std::cmp::max(slot, source.1.max()) + max_slot_inclusive: Option, ) -> Self { let mut min = Slot::MAX; let mut max = Slot::MIN; + let mut adjust_min_max = |slot| { + min = std::cmp::min(slot, min); + max = std::cmp::max(slot + 1, max); + }; + // none, either, or both of min/max could be specified + if let Some(slot) = min_slot { + adjust_min_max(slot); + } + if let Some(slot) = max_slot_inclusive { + adjust_min_max(slot); + } + let mut slot_count = 0; let mut time = Measure::start("get slot"); let source_ = source.clone(); source_.for_each(|(_, slot)| { slot_count += 1; - let slot = *slot; - min = std::cmp::min(slot, min); - max = std::cmp::max(slot + 1, max); + adjust_min_max(*slot); }); time.stop(); let mut time2 = Measure::start("sort"); @@ -128,12 +145,16 @@ pub mod tests { #[test] #[should_panic(expected = "slots are not unique")] fn test_sorted_storages_duplicate_slots() { - SortedStorages::new_with_slots([Vec::new(), Vec::new()].iter().zip([0, 0].iter())); + SortedStorages::new_with_slots( + [Vec::new(), Vec::new()].iter().zip([0, 0].iter()), + None, + None, + ); } #[test] fn test_sorted_storages_none() { - let result = SortedStorages::new_with_slots([].iter().zip([].iter())); + let result = SortedStorages::new_with_slots([].iter().zip([].iter()), None, None); assert_eq!(result.range, Range::default()); assert_eq!(result.slot_count, 0); assert_eq!(result.storages.len(), 0); @@ -146,7 +167,7 @@ pub mod tests { let vec_check = vec.clone(); let slot = 4; let vecs = [vec]; - let result = SortedStorages::new_with_slots(vecs.iter().zip([slot].iter())); + let result = SortedStorages::new_with_slots(vecs.iter().zip([slot].iter()), None, None); assert_eq!( result.range, Range { @@ -165,7 +186,7 @@ pub mod tests { let vec_check = vec.clone(); let slots = [4, 7]; let vecs = [vec.clone(), vec]; - let result = SortedStorages::new_with_slots(vecs.iter().zip(slots.iter())); + let result = SortedStorages::new_with_slots(vecs.iter().zip(slots.iter()), None, None); assert_eq!( result.range, Range {