From 53d8cad206de3ef42cfd4962e12f5c25b7aec1a9 Mon Sep 17 00:00:00 2001 From: "Jeff Washington (jwash)" <75863576+jeffwashington@users.noreply.github.com> Date: Tue, 27 Jul 2021 08:46:27 -0500 Subject: [PATCH] remove unused return value from account index upsert (#18895) --- runtime/src/accounts_index.rs | 82 +++++++++++++++++++++-------------- 1 file changed, 50 insertions(+), 32 deletions(-) diff --git a/runtime/src/accounts_index.rs b/runtime/src/accounts_index.rs index 948f0ae651..f290a042a5 100644 --- a/runtime/src/accounts_index.rs +++ b/runtime/src/accounts_index.rs @@ -1453,30 +1453,24 @@ impl< account_indexes: &AccountSecondaryIndexes, account_info: T, reclaims: &mut SlotList, - ) -> bool { - let is_newly_inserted = { - // We don't atomically update both primary index and secondary index together. - // This certainly creates small time window with inconsistent state across the two indexes. - // However, this is acceptable because: - // - // - A strict consistent view at any given moment of time is not necessary, because the only - // use case for the secondary index is `scan`, and `scans` are only supported/require consistency - // on frozen banks, and this inconsistency is only possible on working banks. - // - // - The secondary index is never consulted as primary source of truth for gets/stores. - // So, what the accounts_index sees alone is sufficient as a source of truth for other non-scan - // account operations. - if let Some((mut w_account_entry, account_info)) = - self.get_account_write_entry_else_create(pubkey, slot, account_info) - { - w_account_entry.update(slot, account_info, reclaims); - false - } else { - true - } - }; + ) { + // We don't atomically update both primary index and secondary index together. + // This certainly creates a small time window with inconsistent state across the two indexes. + // However, this is acceptable because: + // + // - A strict consistent view at any given moment of time is not necessary, because the only + // use case for the secondary index is `scan`, and `scans` are only supported/require consistency + // on frozen banks, and this inconsistency is only possible on working banks. + // + // - The secondary index is never consulted as primary source of truth for gets/stores. + // So, what the accounts_index sees alone is sufficient as a source of truth for other non-scan + // account operations. + if let Some((mut w_account_entry, account_info)) = + self.get_account_write_entry_else_create(pubkey, slot, account_info) + { + w_account_entry.update(slot, account_info, reclaims); + } self.update_secondary_indexes(pubkey, account_owner, account_data, account_indexes); - is_newly_inserted } pub fn unref_from_storage(&self, pubkey: &Pubkey) { @@ -3279,30 +3273,52 @@ pub mod tests { assert!(found_key); } + fn account_maps_len_expensive< + T: 'static + + Sync + + Send + + Clone + + IsCached + + ZeroLamport + + std::cmp::PartialEq + + std::fmt::Debug, + >( + index: &AccountsIndex, + ) -> usize { + index + .account_maps + .iter() + .map(|bin_map| bin_map.read().unwrap().len()) + .sum() + } + #[test] fn test_purge() { let key = Keypair::new(); let index = AccountsIndex::::default(); let mut gc = Vec::new(); - assert!(index.upsert( + assert_eq!(0, account_maps_len_expensive(&index)); + index.upsert( 1, &key.pubkey(), &Pubkey::default(), &[], &AccountSecondaryIndexes::default(), 12, - &mut gc - )); + &mut gc, + ); + assert_eq!(1, account_maps_len_expensive(&index)); - assert!(!index.upsert( + index.upsert( 1, &key.pubkey(), &Pubkey::default(), &[], &AccountSecondaryIndexes::default(), 10, - &mut gc - )); + &mut gc, + ); + assert_eq!(1, account_maps_len_expensive(&index)); let purges = index.purge_roots(&key.pubkey()); assert_eq!(purges, (vec![], false)); @@ -3311,15 +3327,17 @@ pub mod tests { let purges = index.purge_roots(&key.pubkey()); assert_eq!(purges, (vec![(1, 10)], true)); - assert!(!index.upsert( + assert_eq!(1, account_maps_len_expensive(&index)); + index.upsert( 1, &key.pubkey(), &Pubkey::default(), &[], &AccountSecondaryIndexes::default(), 9, - &mut gc - )); + &mut gc, + ); + assert_eq!(1, account_maps_len_expensive(&index)); } #[test]