Remove needless last_root for better reclaims (#8148) (#8584)

automerge
This commit is contained in:
mergify[bot]
2020-03-02 23:21:44 -08:00
committed by GitHub
parent 1d2c3fc81a
commit b286296064
2 changed files with 8 additions and 50 deletions

View File

@ -680,8 +680,6 @@ impl AccountsDB {
reclaims.extend(new_reclaims); reclaims.extend(new_reclaims);
} }
let last_root = accounts_index.last_root;
drop(accounts_index); drop(accounts_index);
drop(storage); drop(storage);
@ -692,16 +690,16 @@ impl AccountsDB {
} }
} }
self.handle_reclaims(&reclaims, last_root); self.handle_reclaims(&reclaims);
} }
fn handle_reclaims(&self, reclaims: &[(Slot, AccountInfo)], last_root: Slot) { fn handle_reclaims(&self, reclaims: &[(Slot, AccountInfo)]) {
let mut dead_accounts = Measure::start("reclaims::remove_dead_accounts"); let mut dead_accounts = Measure::start("reclaims::remove_dead_accounts");
let mut dead_slots = self.remove_dead_accounts(reclaims); let mut dead_slots = self.remove_dead_accounts(reclaims);
dead_accounts.stop(); dead_accounts.stop();
let mut cleanup_dead_slots = Measure::start("reclaims::purge_slots"); let mut cleanup_dead_slots = Measure::start("reclaims::purge_slots");
self.cleanup_dead_slots(&mut dead_slots, last_root); self.cleanup_dead_slots(&mut dead_slots);
cleanup_dead_slots.stop(); cleanup_dead_slots.stop();
let mut purge_slots = Measure::start("reclaims::purge_slots"); let mut purge_slots = Measure::start("reclaims::purge_slots");
@ -1106,7 +1104,7 @@ impl AccountsDB {
slot_id: Slot, slot_id: Slot,
infos: Vec<AccountInfo>, infos: Vec<AccountInfo>,
accounts: &[(&Pubkey, &Account)], accounts: &[(&Pubkey, &Account)],
) -> (Vec<(Slot, AccountInfo)>, u64) { ) -> Vec<(Slot, AccountInfo)> {
let mut reclaims: Vec<(Slot, AccountInfo)> = Vec::with_capacity(infos.len() * 2); let mut reclaims: Vec<(Slot, AccountInfo)> = Vec::with_capacity(infos.len() * 2);
let index = self.accounts_index.read().unwrap(); let index = self.accounts_index.read().unwrap();
let mut update_index_work = Measure::start("update_index_work"); let mut update_index_work = Measure::start("update_index_work");
@ -1121,7 +1119,6 @@ impl AccountsDB {
}) })
.collect(); .collect();
let last_root = index.last_root;
drop(index); drop(index);
if !inserts.is_empty() { if !inserts.is_empty() {
let mut index = self.accounts_index.write().unwrap(); let mut index = self.accounts_index.write().unwrap();
@ -1130,7 +1127,7 @@ impl AccountsDB {
} }
} }
update_index_work.stop(); update_index_work.stop();
(reclaims, last_root) reclaims
} }
fn remove_dead_accounts(&self, reclaims: &[(Slot, AccountInfo)]) -> HashSet<Slot> { fn remove_dead_accounts(&self, reclaims: &[(Slot, AccountInfo)]) -> HashSet<Slot> {
@ -1165,9 +1162,7 @@ impl AccountsDB {
dead_slots dead_slots
} }
fn cleanup_dead_slots(&self, dead_slots: &mut HashSet<Slot>, last_root: u64) { fn cleanup_dead_slots(&self, dead_slots: &mut HashSet<Slot>) {
// a slot is not totally dead until it is older than the root
dead_slots.retain(|slot| *slot < last_root);
if !dead_slots.is_empty() { if !dead_slots.is_empty() {
{ {
let mut index = self.accounts_index.write().unwrap(); let mut index = self.accounts_index.write().unwrap();
@ -1228,11 +1223,11 @@ impl AccountsDB {
store_accounts.stop(); store_accounts.stop();
let mut update_index = Measure::start("store::update_index"); let mut update_index = Measure::start("store::update_index");
let (reclaims, last_root) = self.update_index(slot_id, infos, accounts); let reclaims = self.update_index(slot_id, infos, accounts);
update_index.stop(); update_index.stop();
trace!("reclaim: {}", reclaims.len()); trace!("reclaim: {}", reclaims.len());
self.handle_reclaims(&reclaims, last_root); self.handle_reclaims(&reclaims);
} }
pub fn add_root(&self, slot: Slot) { pub fn add_root(&self, slot: Slot) {
@ -1795,9 +1790,7 @@ pub mod tests {
let (list, idx) = index.get(&pubkey, &ancestors).unwrap(); let (list, idx) = index.get(&pubkey, &ancestors).unwrap();
list[idx].1.store_id list[idx].1.store_id
}; };
//slot 0 is behind root, but it is not root, therefore it is purged
accounts.add_root(1); accounts.add_root(1);
assert!(accounts.accounts_index.read().unwrap().is_purged(0));
//slot is still there, since gc is lazy //slot is still there, since gc is lazy
assert!(accounts.storage.read().unwrap().0[&0].get(&id).is_some()); assert!(accounts.storage.read().unwrap().0[&0].get(&id).is_some());

View File

@ -10,9 +10,6 @@ pub struct AccountsIndex<T> {
pub account_maps: HashMap<Pubkey, RwLock<SlotList<T>>>, pub account_maps: HashMap<Pubkey, RwLock<SlotList<T>>>,
pub roots: HashSet<Slot>, pub roots: HashSet<Slot>,
// This value that needs to be stored to recover the index from AppendVec
pub last_root: Slot,
} }
impl<T: Clone> AccountsIndex<T> { impl<T: Clone> AccountsIndex<T> {
@ -149,10 +146,6 @@ impl<T: Clone> AccountsIndex<T> {
entry.write().unwrap().push((slot, account_info)); entry.write().unwrap().push((slot, account_info));
} }
pub fn is_purged(&self, slot: Slot) -> bool {
slot < self.last_root
}
pub fn can_purge(max_root: Slot, slot: Slot) -> bool { pub fn can_purge(max_root: Slot, slot: Slot) -> bool {
slot < max_root slot < max_root
} }
@ -162,11 +155,6 @@ impl<T: Clone> AccountsIndex<T> {
} }
pub fn add_root(&mut self, slot: Slot) { pub fn add_root(&mut self, slot: Slot) {
assert!(
(self.last_root == 0 && slot == 0) || (slot >= self.last_root),
"new roots must be increasing"
);
self.last_root = slot;
self.roots.insert(slot); self.roots.insert(slot);
} }
/// Remove the slot when the storage for the slot is freed /// Remove the slot when the storage for the slot is freed
@ -271,29 +259,6 @@ mod tests {
assert_eq!(list[idx], (0, true)); assert_eq!(list[idx], (0, true));
} }
#[test]
fn test_is_purged() {
let mut index = AccountsIndex::<bool>::default();
assert!(!index.is_purged(0));
index.add_root(1);
assert!(index.is_purged(0));
}
#[test]
fn test_max_last_root() {
let mut index = AccountsIndex::<bool>::default();
index.add_root(1);
assert_eq!(index.last_root, 1);
}
#[test]
#[should_panic]
fn test_max_last_root_old() {
let mut index = AccountsIndex::<bool>::default();
index.add_root(1);
index.add_root(0);
}
#[test] #[test]
fn test_cleanup_first() { fn test_cleanup_first() {
let mut index = AccountsIndex::<bool>::default(); let mut index = AccountsIndex::<bool>::default();