Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions accounts-db/src/account_storage/stored_account_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,30 @@ impl StoredAccountInfo<'_> {
}
}

impl<'storage> StoredAccountInfo<'storage> {
/// Constructs a new StoredAccountInfo from a StoredAccountInfoWithoutData and data
///
/// Use this ctor when `other_stored_account` is going out of scope, *but not* the underlying
/// `'storage`. This facilitates incremental improvements towards not reading account data
/// unnecessarily, by changing out the front-end code separately from the back-end.
pub fn new_from<'other>(
other_stored_account: &'other StoredAccountInfoWithoutData<'storage>,
data: &'storage [u8],
) -> Self {
// Note that we must use the pubkey/owner fields directly so that we can get the `'storage`
// lifetime of `other_stored_account`, and *not* its `'other` lifetime.
assert_eq!(other_stored_account.data_len, data.len());
Self {
pubkey: other_stored_account.pubkey,
lamports: other_stored_account.lamports,
owner: other_stored_account.owner,
data,
executable: other_stored_account.executable,
rent_epoch: other_stored_account.rent_epoch,
}
}
}

impl ReadableAccount for StoredAccountInfo<'_> {
fn lamports(&self) -> u64 {
self.lamports
Expand All @@ -36,3 +60,42 @@ impl ReadableAccount for StoredAccountInfo<'_> {
self.rent_epoch
}
}

/// Account type with fields that reference into a storage, *without* data
///
/// Used then scanning the accounts of a single storage.
#[derive(Debug, Clone)]
pub struct StoredAccountInfoWithoutData<'storage> {
pub pubkey: &'storage Pubkey,
pub lamports: u64,
pub owner: &'storage Pubkey,
pub data_len: usize,
pub executable: bool,
pub rent_epoch: Epoch,
}

impl StoredAccountInfoWithoutData<'_> {
pub fn pubkey(&self) -> &Pubkey {
self.pubkey
}
}

impl<'storage> StoredAccountInfoWithoutData<'storage> {
/// Constructs a new StoredAccountInfoWithoutData from a StoredAccountInfo
///
/// Use this ctor when `other_stored_account` is going out of scope, *but not* the underlying
/// `'storage`. This facilitates incremental improvements towards not reading account data
/// unnecessarily, by changing out the front-end code separately from the back-end.
pub fn new_from<'other>(other_stored_account: &'other StoredAccountInfo<'storage>) -> Self {
// Note that we must use the pubkey/owner fields directly so that we can get the `'storage`
// lifetime of `other_stored_account`, and *not* its `'other` lifetime.
Self {
pubkey: other_stored_account.pubkey,
lamports: other_stored_account.lamports,
owner: other_stored_account.owner,
data_len: other_stored_account.data.len(),
executable: other_stored_account.executable,
rent_epoch: other_stored_account.rent_epoch,
}
}
}
12 changes: 9 additions & 3 deletions accounts-db/src/accounts.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use {
crate::{
account_locks::{validate_account_locks, AccountLocks},
account_storage::stored_account_info::StoredAccountInfo,
accounts_db::{
AccountStorageEntry, AccountsAddRootTiming, AccountsDb, LoadHint, LoadedAccount,
ScanAccountStorageData, ScanStorageResult, VerifyAccountsHashAndLamportsConfig,
Expand Down Expand Up @@ -214,13 +215,18 @@ impl Accounts {
// Cache only has one version per key, don't need to worry about versioning
func(loaded_account)
},
|accum: &mut HashMap<Pubkey, B>, loaded_account: &LoadedAccount, _data| {
|accum: &mut HashMap<Pubkey, B>, stored_account, data| {
// SAFETY: We called scan_account_storage() with
// ScanAccountStorageData::DataRefForStorage, so `data` must be Some.
let data = data.unwrap();
let loaded_account =
LoadedAccount::Stored(StoredAccountInfo::new_from(stored_account, data));
let loaded_account_pubkey = *loaded_account.pubkey();
if let Some(val) = func(loaded_account) {
if let Some(val) = func(&loaded_account) {
accum.insert(loaded_account_pubkey, val);
}
},
ScanAccountStorageData::NoData,
ScanAccountStorageData::DataRefForStorage,
);

match scan_result {
Expand Down
72 changes: 49 additions & 23 deletions accounts-db/src/accounts_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@ use {
crate::{
account_info::{AccountInfo, Offset, StorageLocation},
account_storage::{
meta::StoredAccountMeta, stored_account_info::StoredAccountInfo, AccountStorage,
AccountStorageStatus, ShrinkInProgress,
meta::StoredAccountMeta,
stored_account_info::{StoredAccountInfo, StoredAccountInfoWithoutData},
AccountStorage, AccountStorageStatus, ShrinkInProgress,
},
accounts_cache::{AccountsCache, CachedAccount, SlotCache},
accounts_db::stats::{
Expand Down Expand Up @@ -173,6 +174,8 @@ impl StoreTo<'_> {
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) enum ScanAccountStorageData {
/// callback for accounts in storage will not include `data`
// Note, currently only used in tests, but do not remove.
#[cfg_attr(not(test), allow(dead_code))]
NoData,
/// return data (&[u8]) for each account.
/// This can be expensive to get and is not necessary for many scan operations.
Expand Down Expand Up @@ -4924,20 +4927,32 @@ impl AccountsDb {
&self,
slot: Slot,
cache_map_func: impl Fn(&LoadedAccount) -> Option<R> + Sync,
storage_scan_func: impl Fn(&mut B, &LoadedAccount, Option<&[u8]>) + Sync,
storage_scan_func: impl for<'a, 'b, 'storage> Fn(
&'b mut B,
&'a StoredAccountInfoWithoutData<'storage>,
Option<&'storage [u8]>, // account data
) + Sync,
scan_account_storage_data: ScanAccountStorageData,
) -> ScanStorageResult<R, B>
where
R: Send,
B: Send + Default + Sync,
{
self.scan_cache_storage_fallback(slot, cache_map_func, |retval, storage| {
storage.scan_accounts(|account| {
let loaded_account = LoadedAccount::Stored(account);
let data = (scan_account_storage_data == ScanAccountStorageData::DataRefForStorage)
.then_some(loaded_account.data());
storage_scan_func(retval, &loaded_account, data)
});
match scan_account_storage_data {
ScanAccountStorageData::NoData => {
storage.scan_accounts(|account| {
let account_without_data = StoredAccountInfoWithoutData::new_from(&account);
storage_scan_func(retval, &account_without_data, None);
});
}
Comment on lines +4943 to +4948
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the next PR we can add an AccountsFile::scan_accounts_without_data() function, so we'll actually not load the account data here.

ScanAccountStorageData::DataRefForStorage => {
storage.scan_accounts(|account| {
let account_without_data = StoredAccountInfoWithoutData::new_from(&account);
storage_scan_func(retval, &account_without_data, Some(account.data));
});
}
};
})
}

Expand Down Expand Up @@ -7495,20 +7510,23 @@ impl AccountsDb {
let scan_result: ScanStorageResult<(Pubkey, AccountHash), HashMap<Pubkey, AccountHash>> =
self.scan_account_storage(
slot,
|loaded_account: &LoadedAccount| {
|loaded_account| {
// Cache only has one version per key, don't need to worry about versioning
Some((*loaded_account.pubkey(), loaded_account.loaded_hash()))
},
|accum: &mut HashMap<Pubkey, AccountHash>,
loaded_account: &LoadedAccount,
_data| {
|accum: &mut HashMap<_, _>, stored_account, data| {
// SAFETY: We called scan_account_storage() with
// ScanAccountStorageData::DataRefForStorage, so `data` must be Some.
let data = data.unwrap();
let loaded_account =
LoadedAccount::Stored(StoredAccountInfo::new_from(stored_account, data));
let mut loaded_hash = loaded_account.loaded_hash();
if loaded_hash == AccountHash(Hash::default()) {
loaded_hash = Self::hash_account(loaded_account, loaded_account.pubkey())
loaded_hash = Self::hash_account(&loaded_account, loaded_account.pubkey())
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're hashing the account, so we do need the account data.

}
accum.insert(*loaded_account.pubkey(), loaded_hash);
},
ScanAccountStorageData::NoData,
ScanAccountStorageData::DataRefForStorage,
);
scan.stop();

Expand All @@ -7529,11 +7547,16 @@ impl AccountsDb {
// Cache only has one version per key, don't need to worry about versioning
Some((*loaded_account.pubkey(), loaded_account.take_account()))
},
|accum: &mut HashMap<_, _>, loaded_account, _data| {
|accum: &mut HashMap<_, _>, stored_account, data| {
// SAFETY: We called scan_account_storage() with
// ScanAccountStorageData::DataRefForStorage, so `data` must be Some.
let data = data.unwrap();
let loaded_account =
LoadedAccount::Stored(StoredAccountInfo::new_from(stored_account, data));
// Storage may have duplicates so only keep the latest version for each key
accum.insert(*loaded_account.pubkey(), loaded_account.take_account());
},
ScanAccountStorageData::NoData,
ScanAccountStorageData::DataRefForStorage,
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're returning the account, so we do need the account data.

);

match scan_result {
Expand All @@ -7548,27 +7571,30 @@ impl AccountsDb {
ScanStorageResult<PubkeyHashAccount, HashMap<Pubkey, (AccountHash, AccountSharedData)>>;
let scan_result: ScanResult = self.scan_account_storage(
slot,
|loaded_account: &LoadedAccount| {
|loaded_account| {
// Cache only has one version per key, don't need to worry about versioning
Some(PubkeyHashAccount {
pubkey: *loaded_account.pubkey(),
hash: loaded_account.loaded_hash(),
account: loaded_account.take_account(),
})
},
|accum: &mut HashMap<Pubkey, (AccountHash, AccountSharedData)>,
loaded_account: &LoadedAccount,
_data| {
// Storage may have duplicates so only keep the latest version for each key
|accum: &mut HashMap<_, _>, stored_account, data| {
// SAFETY: We called scan_account_storage() with
// ScanAccountStorageData::DataRefForStorage, so `data` must be Some.
let data = data.unwrap();
let loaded_account =
LoadedAccount::Stored(StoredAccountInfo::new_from(stored_account, data));
let mut loaded_hash = loaded_account.loaded_hash();
let key = *loaded_account.pubkey();
let account = loaded_account.take_account();
if loaded_hash == AccountHash(Hash::default()) {
loaded_hash = Self::hash_account(&account, &key)
}
// Storage may have duplicates so only keep the latest version for each key
accum.insert(key, (loaded_hash, account));
},
ScanAccountStorageData::NoData,
ScanAccountStorageData::DataRefForStorage,
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing, we're hashing the account here.

);

match scan_result {
Expand Down
18 changes: 9 additions & 9 deletions accounts-db/src/accounts_db/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4418,8 +4418,8 @@ fn test_accounts_db_cache_clean_dead_slots() {
if let ScanStorageResult::Stored(slot_accounts) = accounts_db.scan_account_storage(
*slot as Slot,
|_| Some(0),
|slot_accounts: &mut HashSet<Pubkey>, loaded_account: &LoadedAccount, _data| {
slot_accounts.insert(*loaded_account.pubkey());
|slot_accounts: &mut HashSet<Pubkey>, stored_account, _data| {
slot_accounts.insert(*stored_account.pubkey());
},
ScanAccountStorageData::NoData,
) {
Expand Down Expand Up @@ -4452,8 +4452,8 @@ fn test_accounts_db_cache_clean() {
if let ScanStorageResult::Stored(slot_account) = accounts_db.scan_account_storage(
*slot as Slot,
|_| Some(0),
|slot_account: &mut Pubkey, loaded_account: &LoadedAccount, _data| {
*slot_account = *loaded_account.pubkey();
|slot_account: &mut Pubkey, stored_account, _data| {
*slot_account = *stored_account.pubkey();
},
ScanAccountStorageData::NoData,
) {
Expand Down Expand Up @@ -4507,7 +4507,7 @@ fn run_test_accounts_db_cache_clean_max_root(
for slot in &slots {
let slot_accounts = accounts_db.scan_account_storage(
*slot as Slot,
|loaded_account: &LoadedAccount| {
|loaded_account| {
assert!(
!is_cache_at_limit,
"When cache is at limit, all roots should have been flushed to storage"
Expand All @@ -4517,8 +4517,8 @@ fn run_test_accounts_db_cache_clean_max_root(
assert!(*slot > requested_flush_root);
Some(*loaded_account.pubkey())
},
|slot_accounts: &mut HashSet<Pubkey>, loaded_account: &LoadedAccount, _data| {
slot_accounts.insert(*loaded_account.pubkey());
|slot_accounts: &mut HashSet<Pubkey>, stored_account, _data| {
slot_accounts.insert(*stored_account.pubkey());
if !is_cache_at_limit {
// Only true when the limit hasn't been reached and there are still
// slots left in the cache
Expand Down Expand Up @@ -4628,8 +4628,8 @@ fn run_flush_rooted_accounts_cache(should_clean: bool) {
let ScanStorageResult::Stored(slot_accounts) = accounts_db.scan_account_storage(
*slot as Slot,
|_| Some(0),
|slot_account: &mut HashSet<Pubkey>, loaded_account: &LoadedAccount, _data| {
slot_account.insert(*loaded_account.pubkey());
|slot_account: &mut HashSet<Pubkey>, stored_account, _data| {
slot_account.insert(*stored_account.pubkey());
},
ScanAccountStorageData::NoData,
) else {
Expand Down
Loading