Skip to content

Commit

Permalink
AcctIdx: upsert avoids unnecessary allocation (solana-labs#21488)
Browse files Browse the repository at this point in the history
  • Loading branch information
jeffwashington authored Nov 30, 2021
1 parent 7ec8822 commit d8fb7ce
Showing 1 changed file with 61 additions and 25 deletions.
86 changes: 61 additions & 25 deletions runtime/src/accounts_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1694,6 +1694,9 @@ impl<T: IndexValue> AccountsIndex<T> {
reclaims: &mut SlotList<T>,
previous_slot_entry_was_cached: bool,
) {
// vast majority of updates are to item already in accounts index, so store as raw to avoid unnecessary allocations
let store_raw = true;

// We don't atomically update both primary index and secondary index together.
// This certainly creates a small time window with inconsistent state across the two indexes.
// However, this is acceptable because:
Expand All @@ -1706,7 +1709,7 @@ impl<T: IndexValue> AccountsIndex<T> {
// So, what the accounts_index sees alone is sufficient as a source of truth for other non-scan
// account operations.
let new_item =
PreAllocatedAccountMapEntry::new(slot, account_info, &self.storage.storage, false);
PreAllocatedAccountMapEntry::new(slot, account_info, &self.storage.storage, store_raw);
let map = &self.account_maps[self.bin_calculator.bin_from_pubkey(pubkey)];

{
Expand Down Expand Up @@ -2883,36 +2886,69 @@ pub mod tests {
assert_eq!(num, 1);
}

fn get_pre_allocated<T: IndexValue>(
slot: Slot,
account_info: T,
storage: &Arc<BucketMapHolder<T>>,
store_raw: bool,
to_raw_first: bool,
) -> PreAllocatedAccountMapEntry<T> {
let entry = PreAllocatedAccountMapEntry::new(slot, account_info, storage, store_raw);

if to_raw_first {
// convert to raw
let (slot2, account_info2) = entry.into();
// recreate using extracted raw
PreAllocatedAccountMapEntry::new(slot2, account_info2, storage, store_raw)
} else {
entry
}
}

#[test]
fn test_new_entry() {
let slot = 0;
// account_info type that IS cached
let account_info = AccountInfoTest::default();
let index = AccountsIndex::default_for_tests();

let new_entry: AccountMapEntry<_> =
PreAllocatedAccountMapEntry::new(slot, account_info, &index.storage.storage, false)
for store_raw in [false, true] {
for to_raw_first in [false, true] {
let slot = 0;
// account_info type that IS cached
let account_info = AccountInfoTest::default();
let index = AccountsIndex::default_for_tests();

let new_entry = get_pre_allocated(
slot,
account_info,
&index.storage.storage,
store_raw,
to_raw_first,
)
.into_account_map_entry(&index.storage.storage);
assert_eq!(new_entry.ref_count.load(Ordering::Relaxed), 0);
assert_eq!(new_entry.slot_list.read().unwrap().capacity(), 1);
assert_eq!(
new_entry.slot_list.read().unwrap().to_vec(),
vec![(slot, account_info)]
);
assert_eq!(new_entry.ref_count.load(Ordering::Relaxed), 0);
assert_eq!(new_entry.slot_list.read().unwrap().capacity(), 1);
assert_eq!(
new_entry.slot_list.read().unwrap().to_vec(),
vec![(slot, account_info)]
);

// account_info type that is NOT cached
let account_info = true;
let index = AccountsIndex::default_for_tests();
// account_info type that is NOT cached
let account_info = true;
let index = AccountsIndex::default_for_tests();

let new_entry: AccountMapEntry<_> =
PreAllocatedAccountMapEntry::new(slot, account_info, &index.storage.storage, false)
let new_entry = get_pre_allocated(
slot,
account_info,
&index.storage.storage,
store_raw,
to_raw_first,
)
.into_account_map_entry(&index.storage.storage);
assert_eq!(new_entry.ref_count.load(Ordering::Relaxed), 1);
assert_eq!(new_entry.slot_list.read().unwrap().capacity(), 1);
assert_eq!(
new_entry.slot_list.read().unwrap().to_vec(),
vec![(slot, account_info)]
);
assert_eq!(new_entry.ref_count.load(Ordering::Relaxed), 1);
assert_eq!(new_entry.slot_list.read().unwrap().capacity(), 1);
assert_eq!(
new_entry.slot_list.read().unwrap().to_vec(),
vec![(slot, account_info)]
);
}
}
}

#[test]
Expand Down

0 comments on commit d8fb7ce

Please sign in to comment.