Skip to content

Commit a829c44

Browse files
brooksprumobrooksprumo
authored
Allows scanning slot and getting stored account info without data (#5840)
1 parent 6a0c554 commit a829c44

File tree

4 files changed

+130
-35
lines changed

4 files changed

+130
-35
lines changed

Diff for: ‎accounts-db/src/account_storage/stored_account_info.rs

+63
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,30 @@ impl StoredAccountInfo<'_> {
1919
}
2020
}
2121

22+
impl<'storage> StoredAccountInfo<'storage> {
23+
/// Constructs a new StoredAccountInfo from a StoredAccountInfoWithoutData and data
24+
///
25+
/// Use this ctor when `other_stored_account` is going out of scope, *but not* the underlying
26+
/// `'storage`. This facilitates incremental improvements towards not reading account data
27+
/// unnecessarily, by changing out the front-end code separately from the back-end.
28+
pub fn new_from<'other>(
29+
other_stored_account: &'other StoredAccountInfoWithoutData<'storage>,
30+
data: &'storage [u8],
31+
) -> Self {
32+
// Note that we must use the pubkey/owner fields directly so that we can get the `'storage`
33+
// lifetime of `other_stored_account`, and *not* its `'other` lifetime.
34+
assert_eq!(other_stored_account.data_len, data.len());
35+
Self {
36+
pubkey: other_stored_account.pubkey,
37+
lamports: other_stored_account.lamports,
38+
owner: other_stored_account.owner,
39+
data,
40+
executable: other_stored_account.executable,
41+
rent_epoch: other_stored_account.rent_epoch,
42+
}
43+
}
44+
}
45+
2246
impl ReadableAccount for StoredAccountInfo<'_> {
2347
fn lamports(&self) -> u64 {
2448
self.lamports
@@ -36,3 +60,42 @@ impl ReadableAccount for StoredAccountInfo<'_> {
3660
self.rent_epoch
3761
}
3862
}
63+
64+
/// Account type with fields that reference into a storage, *without* data
65+
///
66+
/// Used then scanning the accounts of a single storage.
67+
#[derive(Debug, Clone)]
68+
pub struct StoredAccountInfoWithoutData<'storage> {
69+
pub pubkey: &'storage Pubkey,
70+
pub lamports: u64,
71+
pub owner: &'storage Pubkey,
72+
pub data_len: usize,
73+
pub executable: bool,
74+
pub rent_epoch: Epoch,
75+
}
76+
77+
impl StoredAccountInfoWithoutData<'_> {
78+
pub fn pubkey(&self) -> &Pubkey {
79+
self.pubkey
80+
}
81+
}
82+
83+
impl<'storage> StoredAccountInfoWithoutData<'storage> {
84+
/// Constructs a new StoredAccountInfoWithoutData from a StoredAccountInfo
85+
///
86+
/// Use this ctor when `other_stored_account` is going out of scope, *but not* the underlying
87+
/// `'storage`. This facilitates incremental improvements towards not reading account data
88+
/// unnecessarily, by changing out the front-end code separately from the back-end.
89+
pub fn new_from<'other>(other_stored_account: &'other StoredAccountInfo<'storage>) -> Self {
90+
// Note that we must use the pubkey/owner fields directly so that we can get the `'storage`
91+
// lifetime of `other_stored_account`, and *not* its `'other` lifetime.
92+
Self {
93+
pubkey: other_stored_account.pubkey,
94+
lamports: other_stored_account.lamports,
95+
owner: other_stored_account.owner,
96+
data_len: other_stored_account.data.len(),
97+
executable: other_stored_account.executable,
98+
rent_epoch: other_stored_account.rent_epoch,
99+
}
100+
}
101+
}

Diff for: ‎accounts-db/src/accounts.rs

+9-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use {
22
crate::{
33
account_locks::{validate_account_locks, AccountLocks},
4+
account_storage::stored_account_info::StoredAccountInfo,
45
accounts_db::{
56
AccountStorageEntry, AccountsAddRootTiming, AccountsDb, LoadHint, LoadedAccount,
67
ScanAccountStorageData, ScanStorageResult, VerifyAccountsHashAndLamportsConfig,
@@ -214,13 +215,18 @@ impl Accounts {
214215
// Cache only has one version per key, don't need to worry about versioning
215216
func(loaded_account)
216217
},
217-
|accum: &mut HashMap<Pubkey, B>, loaded_account: &LoadedAccount, _data| {
218+
|accum: &mut HashMap<Pubkey, B>, stored_account, data| {
219+
// SAFETY: We called scan_account_storage() with
220+
// ScanAccountStorageData::DataRefForStorage, so `data` must be Some.
221+
let data = data.unwrap();
222+
let loaded_account =
223+
LoadedAccount::Stored(StoredAccountInfo::new_from(stored_account, data));
218224
let loaded_account_pubkey = *loaded_account.pubkey();
219-
if let Some(val) = func(loaded_account) {
225+
if let Some(val) = func(&loaded_account) {
220226
accum.insert(loaded_account_pubkey, val);
221227
}
222228
},
223-
ScanAccountStorageData::NoData,
229+
ScanAccountStorageData::DataRefForStorage,
224230
);
225231

226232
match scan_result {

Diff for: ‎accounts-db/src/accounts_db.rs

+49-23
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,9 @@ use {
2929
crate::{
3030
account_info::{AccountInfo, Offset, StorageLocation},
3131
account_storage::{
32-
meta::StoredAccountMeta, stored_account_info::StoredAccountInfo, AccountStorage,
33-
AccountStorageStatus, ShrinkInProgress,
32+
meta::StoredAccountMeta,
33+
stored_account_info::{StoredAccountInfo, StoredAccountInfoWithoutData},
34+
AccountStorage, AccountStorageStatus, ShrinkInProgress,
3435
},
3536
accounts_cache::{AccountsCache, CachedAccount, SlotCache},
3637
accounts_db::stats::{
@@ -173,6 +174,8 @@ impl StoreTo<'_> {
173174
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
174175
pub(crate) enum ScanAccountStorageData {
175176
/// callback for accounts in storage will not include `data`
177+
// Note, currently only used in tests, but do not remove.
178+
#[cfg_attr(not(test), allow(dead_code))]
176179
NoData,
177180
/// return data (&[u8]) for each account.
178181
/// This can be expensive to get and is not necessary for many scan operations.
@@ -4924,20 +4927,32 @@ impl AccountsDb {
49244927
&self,
49254928
slot: Slot,
49264929
cache_map_func: impl Fn(&LoadedAccount) -> Option<R> + Sync,
4927-
storage_scan_func: impl Fn(&mut B, &LoadedAccount, Option<&[u8]>) + Sync,
4930+
storage_scan_func: impl for<'a, 'b, 'storage> Fn(
4931+
&'b mut B,
4932+
&'a StoredAccountInfoWithoutData<'storage>,
4933+
Option<&'storage [u8]>, // account data
4934+
) + Sync,
49284935
scan_account_storage_data: ScanAccountStorageData,
49294936
) -> ScanStorageResult<R, B>
49304937
where
49314938
R: Send,
49324939
B: Send + Default + Sync,
49334940
{
49344941
self.scan_cache_storage_fallback(slot, cache_map_func, |retval, storage| {
4935-
storage.scan_accounts(|account| {
4936-
let loaded_account = LoadedAccount::Stored(account);
4937-
let data = (scan_account_storage_data == ScanAccountStorageData::DataRefForStorage)
4938-
.then_some(loaded_account.data());
4939-
storage_scan_func(retval, &loaded_account, data)
4940-
});
4942+
match scan_account_storage_data {
4943+
ScanAccountStorageData::NoData => {
4944+
storage.scan_accounts(|account| {
4945+
let account_without_data = StoredAccountInfoWithoutData::new_from(&account);
4946+
storage_scan_func(retval, &account_without_data, None);
4947+
});
4948+
}
4949+
ScanAccountStorageData::DataRefForStorage => {
4950+
storage.scan_accounts(|account| {
4951+
let account_without_data = StoredAccountInfoWithoutData::new_from(&account);
4952+
storage_scan_func(retval, &account_without_data, Some(account.data));
4953+
});
4954+
}
4955+
};
49414956
})
49424957
}
49434958

@@ -7495,20 +7510,23 @@ impl AccountsDb {
74957510
let scan_result: ScanStorageResult<(Pubkey, AccountHash), HashMap<Pubkey, AccountHash>> =
74967511
self.scan_account_storage(
74977512
slot,
7498-
|loaded_account: &LoadedAccount| {
7513+
|loaded_account| {
74997514
// Cache only has one version per key, don't need to worry about versioning
75007515
Some((*loaded_account.pubkey(), loaded_account.loaded_hash()))
75017516
},
7502-
|accum: &mut HashMap<Pubkey, AccountHash>,
7503-
loaded_account: &LoadedAccount,
7504-
_data| {
7517+
|accum: &mut HashMap<_, _>, stored_account, data| {
7518+
// SAFETY: We called scan_account_storage() with
7519+
// ScanAccountStorageData::DataRefForStorage, so `data` must be Some.
7520+
let data = data.unwrap();
7521+
let loaded_account =
7522+
LoadedAccount::Stored(StoredAccountInfo::new_from(stored_account, data));
75057523
let mut loaded_hash = loaded_account.loaded_hash();
75067524
if loaded_hash == AccountHash(Hash::default()) {
7507-
loaded_hash = Self::hash_account(loaded_account, loaded_account.pubkey())
7525+
loaded_hash = Self::hash_account(&loaded_account, loaded_account.pubkey())
75087526
}
75097527
accum.insert(*loaded_account.pubkey(), loaded_hash);
75107528
},
7511-
ScanAccountStorageData::NoData,
7529+
ScanAccountStorageData::DataRefForStorage,
75127530
);
75137531
scan.stop();
75147532

@@ -7529,11 +7547,16 @@ impl AccountsDb {
75297547
// Cache only has one version per key, don't need to worry about versioning
75307548
Some((*loaded_account.pubkey(), loaded_account.take_account()))
75317549
},
7532-
|accum: &mut HashMap<_, _>, loaded_account, _data| {
7550+
|accum: &mut HashMap<_, _>, stored_account, data| {
7551+
// SAFETY: We called scan_account_storage() with
7552+
// ScanAccountStorageData::DataRefForStorage, so `data` must be Some.
7553+
let data = data.unwrap();
7554+
let loaded_account =
7555+
LoadedAccount::Stored(StoredAccountInfo::new_from(stored_account, data));
75337556
// Storage may have duplicates so only keep the latest version for each key
75347557
accum.insert(*loaded_account.pubkey(), loaded_account.take_account());
75357558
},
7536-
ScanAccountStorageData::NoData,
7559+
ScanAccountStorageData::DataRefForStorage,
75377560
);
75387561

75397562
match scan_result {
@@ -7548,27 +7571,30 @@ impl AccountsDb {
75487571
ScanStorageResult<PubkeyHashAccount, HashMap<Pubkey, (AccountHash, AccountSharedData)>>;
75497572
let scan_result: ScanResult = self.scan_account_storage(
75507573
slot,
7551-
|loaded_account: &LoadedAccount| {
7574+
|loaded_account| {
75527575
// Cache only has one version per key, don't need to worry about versioning
75537576
Some(PubkeyHashAccount {
75547577
pubkey: *loaded_account.pubkey(),
75557578
hash: loaded_account.loaded_hash(),
75567579
account: loaded_account.take_account(),
75577580
})
75587581
},
7559-
|accum: &mut HashMap<Pubkey, (AccountHash, AccountSharedData)>,
7560-
loaded_account: &LoadedAccount,
7561-
_data| {
7562-
// Storage may have duplicates so only keep the latest version for each key
7582+
|accum: &mut HashMap<_, _>, stored_account, data| {
7583+
// SAFETY: We called scan_account_storage() with
7584+
// ScanAccountStorageData::DataRefForStorage, so `data` must be Some.
7585+
let data = data.unwrap();
7586+
let loaded_account =
7587+
LoadedAccount::Stored(StoredAccountInfo::new_from(stored_account, data));
75637588
let mut loaded_hash = loaded_account.loaded_hash();
75647589
let key = *loaded_account.pubkey();
75657590
let account = loaded_account.take_account();
75667591
if loaded_hash == AccountHash(Hash::default()) {
75677592
loaded_hash = Self::hash_account(&account, &key)
75687593
}
7594+
// Storage may have duplicates so only keep the latest version for each key
75697595
accum.insert(key, (loaded_hash, account));
75707596
},
7571-
ScanAccountStorageData::NoData,
7597+
ScanAccountStorageData::DataRefForStorage,
75727598
);
75737599

75747600
match scan_result {

Diff for: ‎accounts-db/src/accounts_db/tests.rs

+9-9
Original file line numberDiff line numberDiff line change
@@ -4418,8 +4418,8 @@ fn test_accounts_db_cache_clean_dead_slots() {
44184418
if let ScanStorageResult::Stored(slot_accounts) = accounts_db.scan_account_storage(
44194419
*slot as Slot,
44204420
|_| Some(0),
4421-
|slot_accounts: &mut HashSet<Pubkey>, loaded_account: &LoadedAccount, _data| {
4422-
slot_accounts.insert(*loaded_account.pubkey());
4421+
|slot_accounts: &mut HashSet<Pubkey>, stored_account, _data| {
4422+
slot_accounts.insert(*stored_account.pubkey());
44234423
},
44244424
ScanAccountStorageData::NoData,
44254425
) {
@@ -4452,8 +4452,8 @@ fn test_accounts_db_cache_clean() {
44524452
if let ScanStorageResult::Stored(slot_account) = accounts_db.scan_account_storage(
44534453
*slot as Slot,
44544454
|_| Some(0),
4455-
|slot_account: &mut Pubkey, loaded_account: &LoadedAccount, _data| {
4456-
*slot_account = *loaded_account.pubkey();
4455+
|slot_account: &mut Pubkey, stored_account, _data| {
4456+
*slot_account = *stored_account.pubkey();
44574457
},
44584458
ScanAccountStorageData::NoData,
44594459
) {
@@ -4507,7 +4507,7 @@ fn run_test_accounts_db_cache_clean_max_root(
45074507
for slot in &slots {
45084508
let slot_accounts = accounts_db.scan_account_storage(
45094509
*slot as Slot,
4510-
|loaded_account: &LoadedAccount| {
4510+
|loaded_account| {
45114511
assert!(
45124512
!is_cache_at_limit,
45134513
"When cache is at limit, all roots should have been flushed to storage"
@@ -4517,8 +4517,8 @@ fn run_test_accounts_db_cache_clean_max_root(
45174517
assert!(*slot > requested_flush_root);
45184518
Some(*loaded_account.pubkey())
45194519
},
4520-
|slot_accounts: &mut HashSet<Pubkey>, loaded_account: &LoadedAccount, _data| {
4521-
slot_accounts.insert(*loaded_account.pubkey());
4520+
|slot_accounts: &mut HashSet<Pubkey>, stored_account, _data| {
4521+
slot_accounts.insert(*stored_account.pubkey());
45224522
if !is_cache_at_limit {
45234523
// Only true when the limit hasn't been reached and there are still
45244524
// slots left in the cache
@@ -4628,8 +4628,8 @@ fn run_flush_rooted_accounts_cache(should_clean: bool) {
46284628
let ScanStorageResult::Stored(slot_accounts) = accounts_db.scan_account_storage(
46294629
*slot as Slot,
46304630
|_| Some(0),
4631-
|slot_account: &mut HashSet<Pubkey>, loaded_account: &LoadedAccount, _data| {
4632-
slot_account.insert(*loaded_account.pubkey());
4631+
|slot_account: &mut HashSet<Pubkey>, stored_account, _data| {
4632+
slot_account.insert(*stored_account.pubkey());
46334633
},
46344634
ScanAccountStorageData::NoData,
46354635
) else {

0 commit comments

Comments
 (0)