Skip to content

Commit

Permalink
Removes write version from StorableAccounts (#542)
Browse files Browse the repository at this point in the history
  • Loading branch information
brooksprumo authored Apr 3, 2024
1 parent 57572d5 commit ce1f41e
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 35 deletions.
11 changes: 4 additions & 7 deletions accounts-db/src/account_storage/meta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl<
{
/// used when accounts contains hash and write version already
pub fn new(accounts: &'b U) -> Self {
assert!(accounts.has_hash_and_write_version());
assert!(accounts.has_hash());
Self {
accounts,
hashes_and_write_versions: None,
Expand All @@ -66,7 +66,7 @@ impl<
hashes: Vec<V>,
write_versions: Vec<StoredMetaWriteVersion>,
) -> Self {
assert!(!accounts.has_hash_and_write_version());
assert!(!accounts.has_hash());
assert_eq!(accounts.len(), hashes.len());
assert_eq!(write_versions.len(), hashes.len());
Self {
Expand All @@ -80,11 +80,8 @@ impl<
pub fn get(&self, index: usize) -> (Option<&T>, &Pubkey, &AccountHash, StoredMetaWriteVersion) {
let account = self.accounts.account_default_if_zero_lamport(index);
let pubkey = self.accounts.pubkey(index);
let (hash, write_version) = if self.accounts.has_hash_and_write_version() {
(
self.accounts.hash(index),
self.accounts.write_version(index),
)
let (hash, write_version) = if self.accounts.has_hash() {
(self.accounts.hash(index), StoredMetaWriteVersion::default())
} else {
let item = self.hashes_and_write_versions.as_ref().unwrap();
(item.0[index].borrow(), item.1[index])
Expand Down
2 changes: 1 addition & 1 deletion accounts-db/src/accounts_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6487,7 +6487,7 @@ impl AccountsDb {
self.write_accounts_to_cache(slot, accounts, txn_iter)
}
StoreTo::Storage(storage) => {
if accounts.has_hash_and_write_version() {
if accounts.has_hash() {
self.write_accounts_to_storage(
slot,
storage,
Expand Down
2 changes: 1 addition & 1 deletion accounts-db/src/append_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -878,7 +878,7 @@ pub mod tests {
static_assertions::assert_eq_align!(u64, StoredMeta, AccountMeta);

#[test]
#[should_panic(expected = "accounts.has_hash_and_write_version()")]
#[should_panic(expected = "accounts.has_hash()")]
fn test_storable_accounts_with_hashes_and_write_versions_new() {
let account = AccountSharedData::default();
// for (Slot, &'a [(&'a Pubkey, &'a T)])
Expand Down
35 changes: 9 additions & 26 deletions accounts-db/src/storable_accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,23 +35,16 @@ pub trait StorableAccounts<'a, T: ReadableAccount + Sync>: Sync {
false
}

/// true iff the impl can provide hash and write_version
/// Otherwise, hash and write_version have to be provided separately to store functions.
fn has_hash_and_write_version(&self) -> bool {
/// true iff the impl can provide hash
/// Otherwise, hash has to be provided separately to store functions.
fn has_hash(&self) -> bool {
false
}

/// return hash for account at 'index'
/// Should only be called if 'has_hash_and_write_version' = true
/// Should only be called if 'has_hash' = true
fn hash(&self, _index: usize) -> &AccountHash {
// this should never be called if has_hash_and_write_version returns false
unimplemented!();
}

/// return write_version for account at 'index'
/// Should only be called if 'has_hash_and_write_version' = true
fn write_version(&self, _index: usize) -> u64 {
// this should never be called if has_hash_and_write_version returns false
// this should never be called if has_hash returns false
unimplemented!();
}
}
Expand Down Expand Up @@ -142,15 +135,12 @@ impl<'a> StorableAccounts<'a, StoredAccountMeta<'a>> for (Slot, &'a [&'a StoredA
fn len(&self) -> usize {
self.1.len()
}
fn has_hash_and_write_version(&self) -> bool {
fn has_hash(&self) -> bool {
true
}
fn hash(&self, index: usize) -> &AccountHash {
self.account(index).hash()
}
fn write_version(&self, index: usize) -> u64 {
self.account(index).write_version()
}
}

/// holds slices of accounts being moved FROM a common source slot to 'target_slot'
Expand Down Expand Up @@ -237,15 +227,12 @@ impl<'a> StorableAccounts<'a, StoredAccountMeta<'a>> for StorableAccountsBySlot<
fn contains_multiple_slots(&self) -> bool {
self.contains_multiple_slots
}
fn has_hash_and_write_version(&self) -> bool {
fn has_hash(&self) -> bool {
true
}
fn hash(&self, index: usize) -> &AccountHash {
self.account(index).hash()
}
fn write_version(&self, index: usize) -> u64 {
self.account(index).write_version()
}
}

/// this tuple contains a single different source slot that applies to all accounts
Expand All @@ -269,15 +256,12 @@ impl<'a> StorableAccounts<'a, StoredAccountMeta<'a>>
fn len(&self) -> usize {
self.1.len()
}
fn has_hash_and_write_version(&self) -> bool {
fn has_hash(&self) -> bool {
true
}
fn hash(&self, index: usize) -> &AccountHash {
self.account(index).hash()
}
fn write_version(&self, index: usize) -> u64 {
self.account(index).write_version()
}
}

#[cfg(test)]
Expand Down Expand Up @@ -525,7 +509,7 @@ pub mod tests {
})
.collect::<Vec<_>>();
let storable = StorableAccountsBySlot::new(99, &slots_and_accounts[..]);
assert!(storable.has_hash_and_write_version());
assert!(storable.has_hash());
assert_eq!(99, storable.target_slot());
assert_eq!(entries0 != entries, storable.contains_multiple_slots());
(0..entries).for_each(|index| {
Expand All @@ -534,7 +518,6 @@ pub mod tests {
assert_eq!(storable.pubkey(index), raw2[index].pubkey());
assert_eq!(storable.hash(index), raw2[index].hash());
assert_eq!(storable.slot(index), expected_slots[index]);
assert_eq!(storable.write_version(index), raw2[index].write_version());
})
}
}
Expand Down

0 comments on commit ce1f41e

Please sign in to comment.