From 0eb0d7f73b5fcf7c11e82cc593d530b42d852be4 Mon Sep 17 00:00:00 2001
From: "Jeff Washington (jwash)"
 <75863576+jeffwashington@users.noreply.github.com>
Date: Wed, 22 Sep 2021 13:40:19 -0500
Subject: [PATCH] AcctIdx: consolidate lock_and_update_slot_list (#20090)

---
 runtime/src/accounts_index.rs        | 26 ++++++++------------------
 runtime/src/in_mem_accounts_index.rs | 25 ++++++++++++++-----------
 2 files changed, 22 insertions(+), 29 deletions(-)

diff --git a/runtime/src/accounts_index.rs b/runtime/src/accounts_index.rs
index 0a9babb95e..2e0ebeccab 100644
--- a/runtime/src/accounts_index.rs
+++ b/runtime/src/accounts_index.rs
@@ -283,22 +283,6 @@ impl<T: IndexValue> WriteAccountMapEntry<T> {
             AccountMapEntryMeta::new_dirty(storage),
         ))
     }
-
-    // Try to update an item in the slot list the given `slot` If an item for the slot
-    // already exists in the list, remove the older item, add it to `reclaims`, and insert
-    // the new item.
-    pub fn update(&mut self, slot: Slot, account_info: T, reclaims: &mut SlotList<T>) {
-        let mut addref = !account_info.is_cached();
-        self.slot_list_mut(|list| {
-            addref =
-                InMemAccountsIndex::update_slot_list(list, slot, account_info, reclaims, false);
-        });
-        if addref {
-            // If it's the first non-cache insert, also bump the stored ref count
-            self.borrow_owned_entry().add_un_ref(true);
-        }
-        self.borrow_owned_entry().set_dirty(true);
-    }
 }
 
 #[derive(Debug, Default, AbiExample, Clone)]
@@ -1578,9 +1562,15 @@ impl<T: IndexValue> AccountsIndex<T> {
             items.into_iter().for_each(|(pubkey, new_item)| {
                 let already_exists =
                     w_account_maps.insert_new_entry_if_missing_with_lock(pubkey, new_item);
-                if let Some((mut w_account_entry, account_info, pubkey)) = already_exists {
+                if let Some((account_entry, account_info, pubkey)) = already_exists {
                     let is_zero_lamport = account_info.is_zero_lamport();
-                    w_account_entry.update(slot, account_info, &mut _reclaims);
+                    InMemAccountsIndex::lock_and_update_slot_list(
+                        &account_entry,
+                        (slot, account_info),
+                        &mut _reclaims,
+                        false,
+                    );
+
                     if !is_zero_lamport {
                         // zero lamports were already added to dirty_pubkeys above
                         dirty_pubkeys.push(pubkey);
diff --git a/runtime/src/in_mem_accounts_index.rs b/runtime/src/in_mem_accounts_index.rs
index 2297eca808..8221a074ce 100644
--- a/runtime/src/in_mem_accounts_index.rs
+++ b/runtime/src/in_mem_accounts_index.rs
@@ -1,6 +1,6 @@
 use crate::accounts_index::{
     AccountMapEntry, AccountMapEntryInner, AccountMapEntryMeta, IndexValue, RefCount, SlotList,
-    SlotSlice, WriteAccountMapEntry,
+    SlotSlice,
 };
 use crate::bucket_map_holder::{Age, BucketMapHolder};
 use crate::bucket_map_holder_stats::BucketMapHolderStats;
@@ -239,7 +239,7 @@ impl<T: IndexValue> InMemAccountsIndex<T> {
                 let current = occupied.get_mut();
                 Self::lock_and_update_slot_list(
                     current,
-                    &new_value,
+                    new_value.slot_list.write().unwrap().remove(0),
                     reclaims,
                     previous_slot_entry_was_cached,
                 );
@@ -252,7 +252,7 @@ impl<T: IndexValue> InMemAccountsIndex<T> {
                     // on disk, so merge new_value with what was on disk
                     Self::lock_and_update_slot_list(
                         &disk_entry,
-                        &new_value,
+                        new_value.slot_list.write().unwrap().remove(0),
                         reclaims,
                         previous_slot_entry_was_cached,
                     );
@@ -268,14 +268,17 @@ impl<T: IndexValue> InMemAccountsIndex<T> {
         }
     }
 
+    // Try to update an item in the slot list the given `slot` If an item for the slot
+    // already exists in the list, remove the older item, add it to `reclaims`, and insert
+    // the new item.
     pub fn lock_and_update_slot_list(
-        current: &Arc<AccountMapEntryInner<T>>,
-        new_value: &AccountMapEntry<T>,
+        current: &AccountMapEntryInner<T>,
+        new_value: (Slot, T),
         reclaims: &mut SlotList<T>,
         previous_slot_entry_was_cached: bool,
     ) {
         let mut slot_list = current.slot_list.write().unwrap();
-        let (slot, new_entry) = new_value.slot_list.write().unwrap().remove(0);
+        let (slot, new_entry) = new_value;
         let addref = Self::update_slot_list(
             &mut slot_list,
             slot,
@@ -291,7 +294,7 @@ impl<T: IndexValue> InMemAccountsIndex<T> {
 
     // modifies slot_list
     // returns true if caller should addref
-    pub fn update_slot_list(
+    fn update_slot_list(
         list: &mut SlotList<T>,
         slot: Slot,
         account_info: T,
@@ -351,7 +354,7 @@ impl<T: IndexValue> InMemAccountsIndex<T> {
         if let Some(current) = self.map().read().unwrap().get(pubkey) {
             Self::lock_and_update_slot_list(
                 current,
-                new_value,
+                new_value.slot_list.write().unwrap().remove(0),
                 reclaims,
                 previous_slot_entry_was_cached,
             );
@@ -373,9 +376,9 @@ impl<T: IndexValue> InMemAccountsIndex<T> {
         existing: &AccountMapEntry<T>,
         pubkey: &Pubkey,
         new_entry: AccountMapEntry<T>,
-    ) -> (WriteAccountMapEntry<T>, T, Pubkey) {
+    ) -> (AccountMapEntry<T>, T, Pubkey) {
         (
-            WriteAccountMapEntry::from_account_map_entry(Arc::clone(existing)),
+            Arc::clone(existing),
             // extract the new account_info from the unused 'new_entry'
             new_entry.slot_list.write().unwrap().remove(0).1,
             *pubkey,
@@ -388,7 +391,7 @@ impl<T: IndexValue> InMemAccountsIndex<T> {
         &self,
         pubkey: Pubkey,
         new_entry: AccountMapEntry<T>,
-    ) -> Option<(WriteAccountMapEntry<T>, T, Pubkey)> {
+    ) -> Option<(AccountMapEntry<T>, T, Pubkey)> {
         let m = Measure::start("entry");
         let mut map = self.map().write().unwrap();
         let entry = map.entry(pubkey);