Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Keep track of zero-lamport stores and skip purge if there are none #8405

Closed
sakridge wants to merge 1 commit into from
Closed

Keep track of zero-lamport stores and skip purge if there are none #8405

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
25 changes: 25 additions & 0 deletions runtime/src/accounts_db.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,12 @@ pub struct AccountsDB {
min_num_stores: usize,

pub bank_hashes: RwLock<HashMap<Slot, BankHashInfo>>,

/// Zero lamport account store count
zla_count: AtomicUsize,

/// Zero lamport account count at last purge
last_purge_zla_count: AtomicUsize,
}

impl Default for AccountsDB {
Expand All @@ -468,6 +474,8 @@ impl Default for AccountsDB {
.unwrap(),
min_num_stores: num_threads,
bank_hashes: RwLock::new(bank_hashes),
zla_count: AtomicUsize::new(0),
last_purge_zla_count: AtomicUsize::new(0),
}
}
}
Expand Down Expand Up @@ -634,6 +642,18 @@ impl AccountsDB {
// can be purged because there are no live append vecs in the ancestors
pub fn purge_zero_lamport_accounts(&self, ancestors: &HashMap<u64, usize>) {
Copy link
Contributor

@ryoqun ryoqun Feb 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is trivial, but I think making this checking code programmatic and adding a test for it is desirable by wrapping the existing code like this:

-   pub fn purge_zero_lamport_accounts(&self, ancestors: &HashMap<u64, usize>)
+   pub fn do_purge_zero_lamport_accounts(&self, ancestors: &HashMap<u64, usize>)
...

+ pub fn (try_to_)purge_zero_lamport_accounts(&self, ancestors: &HashMap<u64, usize>) {
   if self.is_zla_is_increased() { // <= unit test this newly added predicate
        self.do_purge_zero_lamport_accounts(ancestors);
   }
}

self.report_store_stats();

let last_zla_count = self.last_purge_zla_count.load(Ordering::Relaxed);
let zla_count = self.zla_count.load(Ordering::Relaxed);
let prev_zla_count = self.last_purge_zla_count.compare_and_swap(
Copy link
Contributor

@ryoqun ryoqun Feb 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could be replaced with this?: https://doc.rust-lang.org/std/sync/atomic/struct.AtomicUsize.html#method.compare_exchange

last_zla_count,
zla_count,
Ordering::Relaxed,
);
if zla_count == prev_zla_count {
return;
}

let mut purges = HashMap::new();
let accounts_index = self.accounts_index.read().unwrap();
accounts_index.scan_accounts(ancestors, |pubkey, (account_info, slot)| {
Expand Down Expand Up @@ -1308,13 +1328,18 @@ impl AccountsDB {

fn hash_accounts(&self, slot_id: Slot, accounts: &[(&Pubkey, &Account)]) -> Vec<Hash> {
let mut stats = BankHashStats::default();
let mut zla_count = 0;
let hashes: Vec<_> = accounts
.iter()
.map(|(pubkey, account)| {
stats.update(account);
if account.lamports == 0 {
zla_count += 1;
Copy link
Contributor

@ryoqun ryoqun Feb 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be incremented only when rooted. Not critical, though. it just introduces a bit of inaccuracy of timing of purge_zero() I think zlas are eventually purged anyway.

Copy link
Contributor Author

@sakridge sakridge Feb 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea.. that's true. Maybe need to re-think this.

Copy link
Contributor

@ryoqun ryoqun Feb 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some interesting idea here :)

}
Self::hash_account(slot_id, account, pubkey)
})
.collect();
self.zla_count.fetch_add(zla_count, Ordering::Relaxed);

let mut bank_hashes = self.bank_hashes.write().unwrap();
let slot_info = bank_hashes
Expand Down