Skip to content

Commit

Permalink
make store_id an Option in mem acct idx
Browse files Browse the repository at this point in the history
  • Loading branch information
@jeffwashington
jeffwashington committed Feb 24, 2023
1 parent 45a34f6 commit aec466e
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 48 deletions.
37 changes: 26 additions & 11 deletions runtime/src/account_info.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub type StoredSize = u32;
/// specify where account data is located
#[derive(Debug, PartialEq, Eq)]
pub enum StorageLocation {
AppendVec(AppendVecId, Offset),
AppendVec(Option<AppendVecId>, Offset),
Cached,
}

Expand Down Expand Up @@ -84,8 +84,9 @@ pub struct PackedOffsetAndFlags {

#[derive(Default, Debug, PartialEq, Eq, Clone, Copy)]
pub struct AccountInfo {
/// index identifying the append storage
store_id: AppendVecId,
/// index identifying the append vec storage
/// None means the default one associated with the slot
store_id: Option<AppendVecId>,

account_offset_and_flags: AccountOffsetAndFlags,
}
Expand All @@ -97,6 +98,23 @@ pub struct AccountOffsetAndFlags {
packed_offset_and_flags: PackedOffsetAndFlags,
}

impl From<AccountOffsetAndFlags> for AccountInfo {
fn from(account_offset_and_flags: AccountOffsetAndFlags) -> Self {
Self {
store_id: None,
account_offset_and_flags,
}
}
}

impl From<AccountInfo> for AccountOffsetAndFlags {
fn from(info: AccountInfo) -> Self {
Self {
packed_offset_and_flags: info.account_offset_and_flags.packed_offset_and_flags,
}
}
}

impl ZeroLamport for AccountInfo {
fn is_zero_lamport(&self) -> bool {
self.account_offset_and_flags
Expand All @@ -120,9 +138,6 @@ impl IsCached for StorageLocation {
}
}

/// We have to have SOME value for store_id when we are cached
const CACHE_VIRTUAL_STORAGE_ID: AppendVecId = AppendVecId::MAX;

impl AccountInfo {
pub fn new(storage_location: StorageLocation, lamports: u64) -> Self {
let mut packed_offset_and_flags = PackedOffsetAndFlags::default();
Expand All @@ -143,7 +158,7 @@ impl AccountInfo {
}
StorageLocation::Cached => {
packed_offset_and_flags.set_offset_reduced(CACHED_OFFSET);
CACHE_VIRTUAL_STORAGE_ID
None
}
};
packed_offset_and_flags.set_is_zero_lamport(lamports == 0);
Expand All @@ -160,7 +175,7 @@ impl AccountInfo {
(offset / ALIGN_BOUNDARY_OFFSET) as OffsetReduced
}

pub fn store_id(&self) -> AppendVecId {
pub fn store_id(&self) -> Option<AppendVecId> {
// if the account is in a cached store, the store_id is meaningless
assert!(!self.is_cached());
self.store_id
Expand Down Expand Up @@ -202,7 +217,7 @@ mod test {
ALIGN_BOUNDARY_OFFSET,
4 * ALIGN_BOUNDARY_OFFSET,
] {
let info = AccountInfo::new(StorageLocation::AppendVec(0, offset), 0);
let info = AccountInfo::new(StorageLocation::AppendVec(None, offset), 0);
assert!(info.offset() == offset);
}
}
Expand All @@ -211,13 +226,13 @@ mod test {
#[should_panic(expected = "illegal offset")]
fn test_illegal_offset() {
let offset = (MAXIMUM_APPEND_VEC_FILE_SIZE - (ALIGN_BOUNDARY_OFFSET as u64)) as Offset;
AccountInfo::new(StorageLocation::AppendVec(0, offset), 0);
AccountInfo::new(StorageLocation::AppendVec(None, offset), 0);
}

#[test]
#[should_panic(expected = "illegal offset")]
fn test_alignment() {
let offset = 1; // not aligned
AccountInfo::new(StorageLocation::AppendVec(0, offset), 0);
AccountInfo::new(StorageLocation::AppendVec(None, offset), 0);
}
}
42 changes: 25 additions & 17 deletions runtime/src/account_storage.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,23 +50,30 @@ impl AccountStorage {
pub(crate) fn get_account_storage_entry(
&self,
slot: Slot,
store_id: AppendVecId,
store_id: Option<AppendVecId>,
) -> Option<Arc<AccountStorageEntry>> {
let lookup_in_map = || {
self.map
.get(&slot)
.and_then(|r| (r.id == store_id).then_some(Arc::clone(&r.storage)))
self.map.get(&slot).and_then(|r| {
Self::match_store_id(store_id, r.id).then_some(Arc::clone(&r.storage))
})
};

lookup_in_map()
.or_else(|| {
self.shrink_in_progress_map.get(&slot).and_then(|entry| {
(entry.value().append_vec_id() == store_id).then(|| Arc::clone(entry.value()))
Self::match_store_id(store_id, entry.value().append_vec_id())
.then(|| Arc::clone(entry.value()))
})
})
.or_else(lookup_in_map)
}

fn match_store_id(expected: Option<AppendVecId>, store_id: AppendVecId) -> bool {
expected
.map(|expected| expected == store_id)
.unwrap_or(true)
}

/// assert if shrink in progress is active
pub(crate) fn assert_no_shrink_in_progress(&self) {
assert!(self.shrink_in_progress_map.is_empty());
Expand Down Expand Up @@ -276,7 +283,7 @@ pub(crate) mod tests {
let slot = 0;
let id = 0;
// empty everything
assert!(storage.get_account_storage_entry(slot, id).is_none());
assert!(storage.get_account_storage_entry(slot, None).is_none());

// add a map store
let common_store_path = Path::new("");
Expand All @@ -299,11 +306,12 @@ pub(crate) mod tests {
.map
.insert(slot, AccountStorageReference { id, storage: entry });

//todo not sure about these append vec id values
// look in map
assert_eq!(
store_file_size,
storage
.get_account_storage_entry(slot, id)
.get_account_storage_entry(slot, None)
.map(|entry| entry.accounts.capacity())
.unwrap_or_default()
);
Expand All @@ -315,7 +323,7 @@ pub(crate) mod tests {
assert_eq!(
store_file_size,
storage
.get_account_storage_entry(slot, id)
.get_account_storage_entry(slot, None)
.map(|entry| entry.accounts.capacity())
.unwrap_or_default()
);
Expand All @@ -327,7 +335,7 @@ pub(crate) mod tests {
assert_eq!(
store_file_size2,
storage
.get_account_storage_entry(slot, id)
.get_account_storage_entry(slot, None)
.map(|entry| entry.accounts.capacity())
.unwrap_or_default()
);
Expand Down Expand Up @@ -515,10 +523,10 @@ pub(crate) mod tests {
let missing_id = 9999;
let slot = sample.slot();
// id is missing since not in maps at all
assert!(storage.get_account_storage_entry(slot, id).is_none());
assert!(storage.get_account_storage_entry(slot, None).is_none());
// missing should always be missing
assert!(storage
.get_account_storage_entry(slot, missing_id)
.get_account_storage_entry(slot, Some(missing_id))
.is_none());
storage.map.insert(
slot,
Expand All @@ -528,23 +536,23 @@ pub(crate) mod tests {
},
);
// id is found in map
assert!(storage.get_account_storage_entry(slot, id).is_some());
assert!(storage.get_account_storage_entry(slot, None).is_some());
assert!(storage
.get_account_storage_entry(slot, missing_id)
.get_account_storage_entry(slot, Some(missing_id))
.is_none());
storage
.shrink_in_progress_map
.insert(slot, Arc::clone(&sample));
// id is found in map
assert!(storage
.get_account_storage_entry(slot, missing_id)
.get_account_storage_entry(slot, Some(missing_id))
.is_none());
assert!(storage.get_account_storage_entry(slot, id).is_some());
assert!(storage.get_account_storage_entry(slot, None).is_some());
storage.map.remove(&slot);
// id is found in shrink_in_progress_map
assert!(storage
.get_account_storage_entry(slot, missing_id)
.get_account_storage_entry(slot, Some(missing_id))
.is_none());
assert!(storage.get_account_storage_entry(slot, id).is_some());
assert!(storage.get_account_storage_entry(slot, None).is_some());
}
}
45 changes: 25 additions & 20 deletions runtime/src/accounts_db.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

use {
crate::{
account_info::{AccountInfo, StorageLocation},
account_info::{AccountInfo, AccountOffsetAndFlags, StorageLocation},
account_storage::{
meta::{
StorableAccountsWithHashesAndWriteVersions, StoredAccountMeta,
Expand Down Expand Up @@ -678,6 +678,7 @@ impl GenerateIndexTimings {

impl IndexValue for AccountInfo {}
impl DiskIndexValue for AccountInfo {}
impl DiskIndexValue for AccountOffsetAndFlags {}

impl ZeroLamport for AccountSharedData {
fn is_zero_lamport(&self) -> bool {
Expand Down Expand Up @@ -1346,7 +1347,7 @@ struct RemoveUnrootedSlotsSynchronization {
signal: Condvar,
}

type AccountInfoAccountsIndex = AccountsIndex<AccountInfo, AccountInfo>;
type AccountInfoAccountsIndex = AccountsIndex<AccountInfo, AccountOffsetAndFlags>;

// This structure handles the load/store of the accounts
#[derive(Debug)]
Expand Down Expand Up @@ -3360,7 +3361,7 @@ impl AccountsDb {
.unwrap()
- 1;
debug!(
"store_counts, inserting slot: {}, store id: {}, count: {}",
"store_counts, inserting slot: {}, store id: {:?}, count: {}",
slot, account_info.store_id(), count
);
store_counts.insert(*slot, (count, key_set));
Expand Down Expand Up @@ -6174,7 +6175,7 @@ impl AccountsDb {
storage.add_account(stored_size);

infos.push(AccountInfo::new(
StorageLocation::AppendVec(store_id, offsets[0]),
StorageLocation::AppendVec(Some(store_id), offsets[0]),
accounts_and_meta_to_store
.account(i)
.map(|account| account.lamports())
Expand Down Expand Up @@ -8654,7 +8655,7 @@ impl AccountsDb {
pubkey,
IndexAccountMapEntry {
write_version: _write_version,
store_id,
store_id: _store_id,
stored_account,
},
)| {
Expand All @@ -8681,7 +8682,7 @@ impl AccountsDb {
(
pubkey,
AccountInfo::new(
StorageLocation::AppendVec(store_id, stored_account.offset), // will never be cached
StorageLocation::AppendVec(None, stored_account.offset), // will never be cached
stored_account.account_meta.lamports,
),
)
Expand Down Expand Up @@ -8913,7 +8914,7 @@ impl AccountsDb {
count += 1;
let ai = AccountInfo::new(
StorageLocation::AppendVec(
account_info.store_id,
None,
account_info.stored_account.offset,
), // will never be cached
account_info.stored_account.account_meta.lamports,
Expand Down Expand Up @@ -9329,7 +9330,9 @@ impl AccountsDb {
pub fn get_append_vec_id(&self, pubkey: &Pubkey, slot: Slot) -> Option<AppendVecId> {
let ancestors = vec![(slot, 1)].into_iter().collect();
let result = self.accounts_index.get(pubkey, Some(&ancestors), None);
result.map(|(list, index)| list.slot_list()[index].1.store_id())
result
.map(|(list, index)| list.slot_list()[index].1.store_id())
.flatten()
}

pub fn alive_account_count_in_slot(&self, slot: Slot) -> usize {
Expand Down Expand Up @@ -10425,7 +10428,7 @@ pub mod tests {

if let Some(index) = add_to_index {
let account_info = AccountInfo::new(
StorageLocation::AppendVec(storage.append_vec_id(), offsets[0]),
StorageLocation::AppendVec(Some(storage.append_vec_id()), offsets[0]),
account.lamports(),
);
index.upsert(
Expand Down Expand Up @@ -11225,11 +11228,13 @@ pub mod tests {

//slot is still there, since gc is lazy
assert_eq!(
accounts
.storage
.get_slot_storage_entry(0)
.unwrap()
.append_vec_id(),
Some(
accounts
.storage
.get_slot_storage_entry(0)
.unwrap()
.append_vec_id()
),
id
);

Expand Down Expand Up @@ -13569,10 +13574,10 @@ pub mod tests {
let key0 = Pubkey::new_from_array([0u8; 32]);
let key1 = Pubkey::new_from_array([1u8; 32]);
let key2 = Pubkey::new_from_array([2u8; 32]);
let info0 = AccountInfo::new(StorageLocation::AppendVec(0, 0), 0);
let info1 = AccountInfo::new(StorageLocation::AppendVec(1, 0), 0);
let info2 = AccountInfo::new(StorageLocation::AppendVec(2, 0), 0);
let info3 = AccountInfo::new(StorageLocation::AppendVec(3, 0), 0);
let info0 = AccountInfo::new(StorageLocation::AppendVec(None, 0), 0);
let info1 = AccountInfo::new(StorageLocation::AppendVec(None, 0), 0);
let info2 = AccountInfo::new(StorageLocation::AppendVec(None, 0), 0);
let info3 = AccountInfo::new(StorageLocation::AppendVec(None, 0), 0);
let mut reclaims = vec![];
accounts_index.upsert(
0,
Expand Down Expand Up @@ -16020,7 +16025,7 @@ pub mod tests {
}

let do_test = |test_params: TestParameters| {
let account_info = AccountInfo::new(StorageLocation::AppendVec(42, 128), 0);
let account_info = AccountInfo::new(StorageLocation::AppendVec(None, 128), 0);
let pubkey = solana_sdk::pubkey::new_rand();
let mut key_set = HashSet::default();
key_set.insert(pubkey);
Expand Down Expand Up @@ -17592,7 +17597,7 @@ pub mod tests {
if let Some(storage) = db.get_storage_for_slot(slot) {
storage.accounts.account_iter().for_each(|account| {
let info = AccountInfo::new(
StorageLocation::AppendVec(storage.append_vec_id(), account.offset),
StorageLocation::AppendVec(None, account.offset),
account.lamports(),
);
db.accounts_index.upsert(
Expand Down

0 comments on commit aec466e

Please sign in to comment.