From e261e2704a1a874b4df5802cbeb6d119d3dc4240 Mon Sep 17 00:00:00 2001
From: "Jeff Washington (jwash)" <jeff.washington@solana.com>
Date: Thu, 28 Mar 2024 17:00:48 -0500
Subject: [PATCH] store only removes from read cache if slot is possibly
 present (#452)

* store only removes from read cache if slot is possibly present

* remove_assume_not_present
---
 accounts-db/src/accounts_db.rs              | 16 ++++++++++++----
 accounts-db/src/read_only_accounts_cache.rs | 20 ++++++++++++++++++++
 2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs
index 348ae90100fd1c..0aabae8e93ea1b 100644
--- a/accounts-db/src/accounts_db.rs
+++ b/accounts-db/src/accounts_db.rs
@@ -6427,10 +6427,18 @@ impl AccountsDb {
     ) -> Vec<AccountInfo> {
         let mut calc_stored_meta_time = Measure::start("calc_stored_meta");
         let slot = accounts.target_slot();
-        (0..accounts.len()).for_each(|index| {
-            let pubkey = accounts.pubkey(index);
-            self.read_only_accounts_cache.remove(*pubkey, slot);
-        });
+        if self
+            .read_only_accounts_cache
+            .can_slot_be_in_cache(accounts.target_slot())
+        {
+            (0..accounts.len()).for_each(|index| {
+                let pubkey = accounts.pubkey(index);
+                // based on the patterns of how a validator writes accounts, it is almost always the case that there is no read only cache entry
+                // for this pubkey and slot. So, we can give that hint to the `remove` for performance.
+                self.read_only_accounts_cache
+                    .remove_assume_not_present(*pubkey, slot);
+            });
+        }
         calc_stored_meta_time.stop();
         self.stats
             .calc_stored_meta
diff --git a/accounts-db/src/read_only_accounts_cache.rs b/accounts-db/src/read_only_accounts_cache.rs
index 27e0d848543c82..89cd5928f16786 100644
--- a/accounts-db/src/read_only_accounts_cache.rs
+++ b/accounts-db/src/read_only_accounts_cache.rs
@@ -72,11 +72,13 @@ pub(crate) struct ReadOnlyAccountsCache {
 
     // Performance statistics
     stats: ReadOnlyCacheStats,
+    highest_slot_stored: AtomicU64,
 }
 
 impl ReadOnlyAccountsCache {
     pub(crate) fn new(max_data_size: usize, ms_to_skip_lru_update: u32) -> Self {
         Self {
+            highest_slot_stored: AtomicU64::default(),
             max_data_size,
             cache: DashMap::default(),
             queue: Mutex::<IndexList<ReadOnlyCacheKey>>::default(),
@@ -134,6 +136,7 @@ impl ReadOnlyAccountsCache {
     }
 
     pub(crate) fn store(&self, pubkey: Pubkey, slot: Slot, account: AccountSharedData) {
+        self.highest_slot_stored.fetch_max(slot, Ordering::Release);
         let key = (pubkey, slot);
         let account_size = self.account_size(&account);
         self.data_size.fetch_add(account_size, Ordering::Relaxed);
@@ -169,6 +172,23 @@ impl ReadOnlyAccountsCache {
         self.stats.evicts.fetch_add(num_evicts, Ordering::Relaxed);
     }
 
+    /// true if any pubkeys could have ever been stored into the cache at `slot`
+    pub(crate) fn can_slot_be_in_cache(&self, slot: Slot) -> bool {
+        self.highest_slot_stored.load(Ordering::Acquire) >= slot
+    }
+
+    /// remove entry if it exists.
+    /// Assume the entry does not exist for performance.
+    pub(crate) fn remove_assume_not_present(
+        &self,
+        pubkey: Pubkey,
+        slot: Slot,
+    ) -> Option<AccountSharedData> {
+        // get read lock first to see if the entry exists
+        _ = self.cache.get(&(pubkey, slot))?;
+        self.remove(pubkey, slot)
+    }
+
     pub(crate) fn remove(&self, pubkey: Pubkey, slot: Slot) -> Option<AccountSharedData> {
         let (_, entry) = self.cache.remove(&(pubkey, slot))?;
         // self.queue should be modified only after removing the entry from the