From 3d03f7b47ec6d831ded61bd663d8b91def35cc13 Mon Sep 17 00:00:00 2001 From: "Jeff Washington (jwash)" Date: Tue, 30 Aug 2022 15:42:46 -0500 Subject: [PATCH] remove unused acct idx::upsert_on_disk (#27479) --- runtime/src/in_mem_accounts_index.rs | 160 +++++++-------------------- 1 file changed, 41 insertions(+), 119 deletions(-) diff --git a/runtime/src/in_mem_accounts_index.rs b/runtime/src/in_mem_accounts_index.rs index 731bfc827..09219eb8f 100644 --- a/runtime/src/in_mem_accounts_index.rs +++ b/runtime/src/in_mem_accounts_index.rs @@ -13,10 +13,7 @@ use { solana_measure::measure::Measure, solana_sdk::{clock::Slot, pubkey::Pubkey}, std::{ - collections::{ - hash_map::{Entry, VacantEntry}, - HashMap, - }, + collections::{hash_map::Entry, HashMap}, fmt::Debug, ops::{Bound, RangeBounds, RangeInclusive}, sync::{ @@ -483,40 +480,26 @@ impl InMemAccountsIndex { // not in cache, look on disk updated_in_mem = false; - // desired to be this for filler accounts: self.storage.get_startup(); - // but, this has proven to be far too slow at high account counts - let directly_to_disk = false; - if directly_to_disk { - // We may like this to always run, but it is unclear. - // If disk bucket needs to resize, then this call can stall for a long time. - // Right now, we know it is safe during startup. - let already_existed = self - .upsert_on_disk(vacant, new_value, other_slot, reclaims, reclaim); - if !already_existed { - self.stats().inc_insert(); - } + // go to in-mem cache first + let disk_entry = self.load_account_entry_from_disk(vacant.key()); + let new_value = if let Some(disk_entry) = disk_entry { + // on disk, so merge new_value with what was on disk + Self::lock_and_update_slot_list( + &disk_entry, + new_value.into(), + other_slot, + reclaims, + reclaim, + ); + disk_entry } else { - // go to in-mem cache first - let disk_entry = self.load_account_entry_from_disk(vacant.key()); - let new_value = if let Some(disk_entry) = disk_entry { - // on disk, so merge new_value with what was on disk - Self::lock_and_update_slot_list( - &disk_entry, - new_value.into(), - other_slot, - reclaims, - reclaim, - ); - disk_entry - } else { - // not on disk, so insert new thing - self.stats().inc_insert(); - new_value.into_account_map_entry(&self.storage) - }; - assert!(new_value.dirty()); - vacant.insert(new_value); - self.stats().inc_mem_count(self.bin); - } + // not on disk, so insert new thing + self.stats().inc_insert(); + new_value.into_account_map_entry(&self.storage) + }; + assert!(new_value.dirty()); + vacant.insert(new_value); + self.stats().inc_mem_count(self.bin); } } @@ -709,46 +692,31 @@ impl InMemAccountsIndex { } Entry::Vacant(vacant) => { // not in cache, look on disk - let initial_insert_directly_to_disk = false; - if initial_insert_directly_to_disk { - // This is more direct, but becomes too slow with very large acct #. - // disk buckets will be improved to make them more performant. Tuning the disks may also help. - // This may become a config tuning option. - let already_existed = self.upsert_on_disk( - vacant, - new_entry, - None, // not changing slots here since it doesn't exist in the index at all + let disk_entry = self.load_account_entry_from_disk(vacant.key()); + self.stats().inc_mem_count(self.bin); + if let Some(disk_entry) = disk_entry { + let (slot, account_info) = new_entry.into(); + InMemAccountsIndex::lock_and_update_slot_list( + &disk_entry, + (slot, account_info), + // None because we are inserting the first element in the slot list for this pubkey. + // There can be no 'other' slot in the list. + None, &mut Vec::default(), UpsertReclaim::PopulateReclaims, ); - (false, already_existed) + vacant.insert(disk_entry); + ( + false, /* found in mem */ + true, /* already existed */ + ) } else { - let disk_entry = self.load_account_entry_from_disk(vacant.key()); - self.stats().inc_mem_count(self.bin); - if let Some(disk_entry) = disk_entry { - let (slot, account_info) = new_entry.into(); - InMemAccountsIndex::lock_and_update_slot_list( - &disk_entry, - (slot, account_info), - // None because we are inserting the first element in the slot list for this pubkey. - // There can be no 'other' slot in the list. - None, - &mut Vec::default(), - UpsertReclaim::PopulateReclaims, - ); - vacant.insert(disk_entry); - ( - false, /* found in mem */ - true, /* already existed */ - ) - } else { - // not on disk, so insert new thing and we're done - let new_entry: AccountMapEntry = - new_entry.into_account_map_entry(&self.storage); - assert!(new_entry.dirty()); - vacant.insert(new_entry); - (false, false) - } + // not on disk, so insert new thing and we're done + let new_entry: AccountMapEntry = + new_entry.into_account_map_entry(&self.storage); + assert!(new_entry.dirty()); + vacant.insert(new_entry); + (false, false) } } }; @@ -769,52 +737,6 @@ impl InMemAccountsIndex { } } - /// return true if item already existed in the index - fn upsert_on_disk( - &self, - vacant: VacantEntry>, - new_entry: PreAllocatedAccountMapEntry, - other_slot: Option, - reclaims: &mut SlotList, - reclaim: UpsertReclaim, - ) -> bool { - if let Some(disk) = self.bucket.as_ref() { - let mut existed = false; - let (slot, account_info) = new_entry.into(); - disk.update(vacant.key(), |current| { - if let Some((slot_list, mut ref_count)) = current { - // on disk, so merge and update disk - let mut slot_list = slot_list.to_vec(); - let addref = Self::update_slot_list( - &mut slot_list, - slot, - account_info, - other_slot, - reclaims, - reclaim, - ); - if addref { - ref_count += 1 - }; - existed = true; // found on disk, so it did exist - Some((slot_list, ref_count)) - } else { - // doesn't exist on disk yet, so insert it - let ref_count = if account_info.is_cached() { 0 } else { 1 }; - Some((vec![(slot, account_info)], ref_count)) - } - }); - existed - } else { - // not using disk, so insert into mem - self.stats().inc_mem_count(self.bin); - let new_entry: AccountMapEntry = new_entry.into_account_map_entry(&self.storage); - assert!(new_entry.dirty()); - vacant.insert(new_entry); - false // not using disk, not in mem, so did not exist - } - } - /// Look at the currently held ranges. If 'range' is already included in what is /// being held, then add 'range' to the currently held list AND return true /// If 'range' is NOT already included in what is being held, then return false