Keep track of zero-lamport stores and skip purge if there are none #8405
Conversation
sakridge
force-pushed
the
optimize-purge
branch
from
1bba10f to
4f6212f
Compare
sakridge
force-pushed
the
optimize-purge
branch
from
4f6212f to
f80c03c
Compare
Codecov Report
@@ Coverage Diff @@
## master #8405 +/- ##
======================================
Coverage 80.2% 80.2%
======================================
Files 253 253
Lines 55616 55632 +16
======================================
+ Hits 44640 44654 +14
- Misses 10976 10978 +2 |
| .map(|(pubkey, account)| { | ||
| stats.update(account); | ||
| if account.lamports == 0 { | ||
| zla_count += 1; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Yea.. that's true. Maybe need to re-think this.
| // Purge zero lamport accounts for garbage collection purposes | ||
| // Only remove those accounts where the entire rooted history of the account | ||
| // can be purged because there are no live append vecs in the ancestors | ||
| pub fn purge_zero_lamport_accounts(&self, ancestors: &HashMap<u64, usize>) { |
There was a problem hiding this comment.
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);
}
}
|
|
||
| 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( |
There was a problem hiding this comment.
I think this could be replaced with this?: https://doc.rust-lang.org/std/sync/atomic/struct.AtomicUsize.html#method.compare_exchange
|
@sakridge just reviewed! Nice optimization! |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
stale
bot
added
the
stale
|
This stale pull request has been automatically closed. Thank you for your contributions. |
2 participants
Problem
purge_zero_lamport accountsdoes an account scan even if there were no zero lamport accounts stored. On a testnet with tx_count=50k, bench-tps creates about 1.4m accounts. Just the scan of all those accounts can take about 500ms on a gce machine.Summary of Changes
Keep a counter of zero lamport account stores. Only if the value changes, then run the purge.
Fixes #