From 01ca7a3f81db829f229a33994880358b6da7e63f Mon Sep 17 00:00:00 2001 From: "Jeff Washington (jwash)" Date: Tue, 13 Dec 2022 13:26:28 -0600 Subject: [PATCH] remove add_root(caching_enabled) (#29245) --- runtime/benches/accounts_index.rs | 2 +- runtime/src/accounts_db.rs | 16 ++++---- runtime/src/accounts_index.rs | 66 +++++++++++++++---------------- runtime/src/bank.rs | 2 +- 4 files changed, 41 insertions(+), 45 deletions(-) diff --git a/runtime/benches/accounts_index.rs b/runtime/benches/accounts_index.rs index 791fb76e65d5ff..4539f72f5fc540 100644 --- a/runtime/benches/accounts_index.rs +++ b/runtime/benches/accounts_index.rs @@ -60,7 +60,7 @@ fn bench_accounts_index(bencher: &mut Bencher) { ); reclaims.clear(); } - index.add_root(root, false); + index.add_root(root); root += 1; fork += 1; }); diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index d2d6c678c4691c..6c528f01bc77a4 100644 --- a/runtime/src/accounts_db.rs +++ b/runtime/src/accounts_db.rs @@ -8561,7 +8561,7 @@ impl AccountsDb { pub fn add_root(&self, slot: Slot) -> AccountsAddRootTiming { let mut index_time = Measure::start("index_add_root"); - self.accounts_index.add_root(slot, true); + self.accounts_index.add_root(slot); index_time.stop(); let mut cache_time = Measure::start("cache_add_root"); self.accounts_cache.add_root(slot); @@ -9212,7 +9212,7 @@ impl AccountsDb { let uncleaned_roots = uncleaned_roots.into_inner().unwrap(); // Need to add these last, otherwise older updates will be cleaned for root in &slots { - self.accounts_index.add_root(*root, true); + self.accounts_index.add_root(*root); } self.accounts_index .add_uncleaned_roots(uncleaned_roots.into_iter()); @@ -14055,10 +14055,10 @@ pub mod tests { &mut reclaims, UPSERT_POPULATE_RECLAIMS, ); - accounts_index.add_root(0, false); - accounts_index.add_root(1, false); - accounts_index.add_root(2, false); - accounts_index.add_root(3, false); + accounts_index.add_root(0); + accounts_index.add_root(1); + accounts_index.add_root(2); + accounts_index.add_root(3); let mut purges = HashMap::new(); let (key0_entry, _) = accounts_index.get_for_tests(&key0, None, None).unwrap(); purges.insert( @@ -16514,7 +16514,7 @@ pub mod tests { let extra = 3; let active_root = 2; - db.accounts_index.add_root(active_root, false); + db.accounts_index.add_root(active_root); let result = db.calc_alive_ancient_historical_roots(extra); let expected_alive_roots = [active_root].into_iter().collect(); assert_eq!(result, expected_alive_roots, "extra: {extra}"); @@ -17969,7 +17969,7 @@ pub mod tests { .insert(slot0, BankHashInfo::default()); db.storage.map.insert(slot0, Arc::default()); assert!(!db.bank_hashes.read().unwrap().is_empty()); - db.accounts_index.add_root(slot0, false); + db.accounts_index.add_root(slot0); assert!(db.accounts_index.is_uncleaned_root(slot0)); assert!(db.accounts_index.is_alive_root(slot0)); db.handle_dropped_roots_for_ancient(dropped_roots); diff --git a/runtime/src/accounts_index.rs b/runtime/src/accounts_index.rs index 8be25bfe00e90c..e9413c5d842a2d 100644 --- a/runtime/src/accounts_index.rs +++ b/runtime/src/accounts_index.rs @@ -1817,7 +1817,7 @@ impl AccountsIndex { .contains(&slot) } - pub fn add_root(&self, slot: Slot, caching_enabled: bool) { + pub fn add_root(&self, slot: Slot) { self.roots_added.fetch_add(1, Ordering::Relaxed); let mut w_roots_tracker = self.roots_tracker.write().unwrap(); // `AccountsDb::flush_accounts_cache()` relies on roots being added in order @@ -1825,10 +1825,6 @@ impl AccountsIndex { // 'slot' is a root, so it is both 'root' and 'original' w_roots_tracker.alive_roots.insert(slot); w_roots_tracker.historical_roots.insert(slot); - // we delay cleaning until flushing! - if !caching_enabled { - w_roots_tracker.uncleaned_roots.insert(slot); - } } pub fn add_uncleaned_roots(&self, roots: I) @@ -2138,14 +2134,14 @@ pub mod tests { assert_eq!(index.get_next_original_root(slot, ancestors), None); } // roots are now [1]. 0 and 1 both return 1 - index.add_root(1, true); + index.add_root(1); for slot in 0..2 { assert_eq!(index.get_next_original_root(slot, ancestors), Some(1)); } assert_eq!(index.get_next_original_root(2, ancestors), None); // no roots after 1, so asking for root >= 2 is None // roots are now [1, 3]. 0 and 1 both return 1. 2 and 3 both return 3 - index.add_root(3, true); + index.add_root(3); for slot in 0..2 { assert_eq!(index.get_next_original_root(slot, ancestors), Some(1)); } @@ -2192,7 +2188,7 @@ pub mod tests { assert_eq!(index.get_next_original_root(slot, ancestors), None); } // roots are now [1]. 0 and 1 both return 1 - index.add_root(1, true); + index.add_root(1); for slot in 0..2 { assert_eq!(index.get_next_original_root(slot, ancestors), Some(1)); } @@ -2213,8 +2209,8 @@ pub mod tests { #[test] fn test_remove_old_historical_roots() { let index = AccountsIndex::::default_for_tests(); - index.add_root(1, true); - index.add_root(2, true); + index.add_root(1); + index.add_root(2); assert_eq!( index .roots_tracker @@ -2254,8 +2250,8 @@ pub mod tests { // now use 'keep' let index = AccountsIndex::::default_for_tests(); - index.add_root(1, true); - index.add_root(2, true); + index.add_root(1); + index.add_root(2); let hash_set_1 = vec![1].into_iter().collect(); assert_eq!( index @@ -2942,7 +2938,7 @@ pub mod tests { ); } - index.add_root(root_slot, false); + index.add_root(root_slot); (index, pubkeys) } @@ -3090,7 +3086,7 @@ pub mod tests { fn test_is_alive_root() { let index = AccountsIndex::::default_for_tests(); assert!(!index.is_alive_root(0)); - index.add_root(0, false); + index.add_root(0); assert!(index.is_alive_root(0)); } @@ -3111,7 +3107,7 @@ pub mod tests { ); assert!(gc.is_empty()); - index.add_root(0, false); + index.add_root(0); let (list, idx) = index.get_for_tests(&key, None, None).unwrap(); assert_eq!(list.slot_list()[idx], (0, true)); } @@ -3119,8 +3115,8 @@ pub mod tests { #[test] fn test_clean_first() { let index = AccountsIndex::::default_for_tests(); - index.add_root(0, false); - index.add_root(1, false); + index.add_root(0); + index.add_root(1); index.clean_dead_slot(0, &mut AccountsIndexRootsStats::default()); assert!(index.is_alive_root(1)); assert!(!index.is_alive_root(0)); @@ -3130,8 +3126,8 @@ pub mod tests { fn test_clean_last() { //this behavior might be undefined, clean up should only occur on older slots let index = AccountsIndex::::default_for_tests(); - index.add_root(0, false); - index.add_root(1, false); + index.add_root(0); + index.add_root(1); index.clean_dead_slot(1, &mut AccountsIndexRootsStats::default()); assert!(!index.is_alive_root(1)); assert!(index.is_alive_root(0)); @@ -3141,8 +3137,8 @@ pub mod tests { fn test_clean_and_unclean_slot() { let index = AccountsIndex::::default_for_tests(); assert_eq!(0, index.roots_tracker.read().unwrap().uncleaned_roots.len()); - index.add_root(0, false); - index.add_root(1, false); + index.add_root(0); + index.add_root(1); assert_eq!(2, index.roots_tracker.read().unwrap().uncleaned_roots.len()); assert_eq!( @@ -3167,8 +3163,8 @@ pub mod tests { .len() ); - index.add_root(2, false); - index.add_root(3, false); + index.add_root(2); + index.add_root(3); assert_eq!(4, index.roots_tracker.read().unwrap().alive_roots.len()); assert_eq!(2, index.roots_tracker.read().unwrap().uncleaned_roots.len()); assert_eq!( @@ -3327,9 +3323,9 @@ pub mod tests { &mut gc, UPSERT_POPULATE_RECLAIMS, ); - index.add_root(0, false); - index.add_root(1, false); - index.add_root(3, false); + index.add_root(0); + index.add_root(1); + index.add_root(3); index.upsert( 4, 4, @@ -3401,7 +3397,7 @@ pub mod tests { let purges = index.purge_roots(&key); assert_eq!(purges, (vec![], false)); - index.add_root(1, false); + index.add_root(1); let purges = index.purge_roots(&key); assert_eq!(purges, (vec![(1, 10)], true)); @@ -3429,7 +3425,7 @@ pub mod tests { assert!(index.latest_slot(None, &slot_slice, None).is_none()); // Given a root, should return the root - index.add_root(5, false); + index.add_root(5); assert_eq!(index.latest_slot(None, &slot_slice, None).unwrap(), 1); // Given a max_root == root, should still return the root @@ -3567,9 +3563,9 @@ pub mod tests { // Add a later root, earlier slots should be reclaimed slot_list = vec![(1, true), (2, true), (5, true), (9, true)]; - index.add_root(1, false); + index.add_root(1); // Note 2 is not a root - index.add_root(5, false); + index.add_root(5); reclaims = vec![]; index.purge_older_root_entries(&mut slot_list, &mut reclaims, None); assert_eq!(reclaims, vec![(1, true), (2, true)]); @@ -3577,7 +3573,7 @@ pub mod tests { // Add a later root that is not in the list, should not affect the outcome slot_list = vec![(1, true), (2, true), (5, true), (9, true)]; - index.add_root(6, false); + index.add_root(6); reclaims = vec![]; index.purge_older_root_entries(&mut slot_list, &mut reclaims, None); assert_eq!(reclaims, vec![(1, true), (2, true)]); @@ -3848,7 +3844,7 @@ pub mod tests { // If we set a root at `later_slot`, and clean, then even though the account with secondary_key1 // was outdated by the update in the later slot, the primary account key is still alive, // so both secondary keys will still be kept alive. - index.add_root(later_slot, false); + index.add_root(later_slot); index.slot_list_mut(&account_key, |slot_list| { index.purge_older_root_entries(slot_list, &mut vec![], None) }); @@ -4011,7 +4007,7 @@ pub mod tests { ); } - index.add_root(slot2, true); + index.add_root(slot2); { let roots_tracker = &index.roots_tracker.read().unwrap(); @@ -4124,7 +4120,7 @@ pub mod tests { // re-add it index.upsert_simple_test(&key, slot1, value); - index.add_root(slot1, value); + index.add_root(slot1); assert!(!index.clean_rooted_entries(&key, &mut gc, Some(slot2))); index.upsert_simple_test(&key, slot2, value); @@ -4163,7 +4159,7 @@ pub mod tests { ) ); } - index.add_root(slot2, true); + index.add_root(slot2); { let roots_tracker = &index.roots_tracker.read().unwrap(); let slot_list = vec![(slot2, value)]; diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 7d81b5945f6196..31397292b39d71 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -10054,7 +10054,7 @@ pub(crate) mod tests { .accounts .accounts_db .accounts_index - .add_root(genesis_bank1.slot() + 1, false); + .add_root(genesis_bank1.slot() + 1); bank1_without_zero .rc .accounts