Skip to content

Commit

Permalink
remove add_root(caching_enabled) (#29245)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jeffwashington
jeffwashington authored Dec 13, 2022
1 parent 8da82e6 commit 3e649d2
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 45 deletions.
2 changes: 1 addition & 1 deletion runtime/benches/accounts_index.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
Expand Down
16 changes: 8 additions & 8 deletions runtime/src/accounts_db.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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}");
Expand Down Expand Up @@ -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);
Expand Down
66 changes: 31 additions & 35 deletions runtime/src/accounts_index.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1817,18 +1817,14 @@ impl<T: IndexValue> AccountsIndex<T> {
.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
assert!(slot >= w_roots_tracker.alive_roots.max_inclusive());
// '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<I>(&self, roots: I)
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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));
}
Expand All @@ -2213,8 +2209,8 @@ pub mod tests {
#[test]
fn test_remove_old_historical_roots() {
let index = AccountsIndex::<bool>::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
Expand Down Expand Up @@ -2254,8 +2250,8 @@ pub mod tests {

// now use 'keep'
let index = AccountsIndex::<bool>::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
Expand Down Expand Up @@ -2942,7 +2938,7 @@ pub mod tests {
);
}

index.add_root(root_slot, false);
index.add_root(root_slot);

(index, pubkeys)
}
Expand Down Expand Up @@ -3090,7 +3086,7 @@ pub mod tests {
fn test_is_alive_root() {
let index = AccountsIndex::<bool>::default_for_tests();
assert!(!index.is_alive_root(0));
index.add_root(0, false);
index.add_root(0);
assert!(index.is_alive_root(0));
}

Expand All @@ -3111,16 +3107,16 @@ 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));
}

#[test]
fn test_clean_first() {
let index = AccountsIndex::<bool>::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));
Expand All @@ -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::<bool>::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));
Expand All @@ -3141,8 +3137,8 @@ pub mod tests {
fn test_clean_and_unclean_slot() {
let index = AccountsIndex::<bool>::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!(
Expand All @@ -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!(
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -3567,17 +3563,17 @@ 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)]);
assert_eq!(slot_list, vec![(5, true), (9, true)]);

// 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)]);
Expand Down Expand Up @@ -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)
});
Expand Down Expand Up @@ -4011,7 +4007,7 @@ pub mod tests {
);
}

index.add_root(slot2, true);
index.add_root(slot2);

{
let roots_tracker = &index.roots_tracker.read().unwrap();
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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)];
Expand Down
2 changes: 1 addition & 1 deletion runtime/src/bank.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 3e649d2

Please sign in to comment.