Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove add_root(caching_enabled) #29245

Merged
merged 1 commit into from
Dec 13, 2022
Merged

remove add_root(caching_enabled) #29245

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion runtime/benches/accounts_index.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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