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.

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

bank: add lightweight bank hash cache to reduce bank forks usage #3061

Closed
wants to merge 1 commit into from

Conversation

AshwinSekar
Copy link

@AshwinSekar AshwinSekar commented Oct 3, 2024

Problem

An upcoming change needs to access the replayed bank hash of select slots. It is overkill to grab the bank_forks lock for such scenarios.

Summary of Changes

Similar to the root_bank_cache, split out a cache just for the latest bank_hash of replayed slots. When a slot is dumped, the cache is not updated which should be fine for our usage. This cache is pruned alongside bank_forks as to keep in sync.

Implementation details:

  • Store a reference to the cache in bank_forks
  • When a bank is inserted into bank_forks, store a reference to the cache in the bank.
  • When the bank is frozen, write lock the cache and insert the new bank hash.
  • Users can grab a reference to the cache from bank_forks and read lock it when they wish to query bank hash without having to read lock bank_forks.

pub fn new_rw_arc(root_bank: Bank) -> Arc<RwLock<Self>> {
pub fn new_rw_arc(mut root_bank: Bank) -> Arc<RwLock<Self>> {
let bank_hash_cache = Arc::new(RwLock::new(BankHashCache::default()));
root_bank.set_bank_hash_cache(bank_hash_cache.clone());
Copy link
Author

Choose a reason for hiding this comment

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

root_bank is not frozen until after BankForks is created, so this is necessary.

)
.is_some()
{
// All ancestors have already been inserted by another fork
Copy link
Author

Choose a reason for hiding this comment

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

This is leftover from when BankForks could be initialized with a list of banks.

@AshwinSekar AshwinSekar force-pushed the bank-hash-cache branch 3 times, most recently from a89baf8 to 807c9aa Compare October 3, 2024 20:38
@AshwinSekar
Copy link
Author

Carl pointed out that this doesn't really save us much, write lock on insertion happens every 400ms at best so this is not needed.

@AshwinSekar AshwinSekar closed this Oct 5, 2024
@AshwinSekar AshwinSekar deleted the bank-hash-cache branch October 9, 2024 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant