[TieredStorage] Avoid AccountHash copy in AccountMetaOptionalFields (#34969)
#### Problem Using non-reference type of AccountHash in AccountMetaOptionalFields causes an unnecessary copy as mentioned in #34948. #### Summary of Changes Uses &AccountHash in AccountMetaOptionalFields to avoid copying. #### Test Plan Existing unit tests. Fixes #34948
This commit is contained in:
parent
93271d91b0
commit
7138f8767e
|
@ -96,7 +96,7 @@ impl ByteBlockWriter {
|
|||
size += self.write_pod(&rent_epoch)?;
|
||||
}
|
||||
if let Some(hash) = opt_fields.account_hash {
|
||||
size += self.write_pod(&hash)?;
|
||||
size += self.write_pod(hash)?;
|
||||
}
|
||||
|
||||
debug_assert_eq!(size, opt_fields.size());
|
||||
|
@ -352,11 +352,12 @@ mod tests {
|
|||
let mut writer = ByteBlockWriter::new(format);
|
||||
let mut opt_fields_vec = vec![];
|
||||
let mut some_count = 0;
|
||||
let acc_hash = AccountHash(Hash::new_unique());
|
||||
|
||||
// prepare a vector of optional fields that contains all combinations
|
||||
// of Some and None.
|
||||
for rent_epoch in [None, Some(test_epoch)] {
|
||||
for account_hash in [None, Some(AccountHash(Hash::new_unique()))] {
|
||||
for account_hash in [None, Some(&acc_hash)] {
|
||||
some_count += rent_epoch.iter().count() + account_hash.iter().count();
|
||||
|
||||
opt_fields_vec.push(AccountMetaOptionalFields {
|
||||
|
@ -397,7 +398,7 @@ mod tests {
|
|||
}
|
||||
if let Some(expected_hash) = opt_fields.account_hash {
|
||||
let hash = read_pod::<AccountHash>(&decoded_buffer, offset).unwrap();
|
||||
assert_eq!(hash, &expected_hash);
|
||||
assert_eq!(hash, expected_hash);
|
||||
verified_count += 1;
|
||||
offset += std::mem::size_of::<AccountHash>();
|
||||
}
|
||||
|
|
|
@ -469,7 +469,7 @@ fn write_optional_fields(
|
|||
size += file.write_pod(&rent_epoch)?;
|
||||
}
|
||||
if let Some(hash) = opt_fields.account_hash {
|
||||
size += file.write_pod(&hash)?;
|
||||
size += file.write_pod(hash)?;
|
||||
}
|
||||
|
||||
debug_assert_eq!(size, opt_fields.size());
|
||||
|
@ -500,7 +500,7 @@ impl HotStorageWriter {
|
|||
account_data: &[u8],
|
||||
executable: bool,
|
||||
rent_epoch: Option<Epoch>,
|
||||
account_hash: Option<AccountHash>,
|
||||
account_hash: Option<&AccountHash>,
|
||||
) -> TieredStorageResult<usize> {
|
||||
let optional_fields = AccountMetaOptionalFields {
|
||||
rent_epoch,
|
||||
|
@ -567,9 +567,9 @@ impl HotStorageWriter {
|
|||
acc.owner(),
|
||||
acc.data(),
|
||||
acc.executable(),
|
||||
// only persist rent_epoch for those non-rent-exempt accounts
|
||||
// only persist rent_epoch for those rent-paying accounts
|
||||
(acc.rent_epoch() != RENT_EXEMPT_RENT_EPOCH).then_some(acc.rent_epoch()),
|
||||
Some(*account_hash),
|
||||
Some(account_hash),
|
||||
)
|
||||
})
|
||||
.unwrap_or((0, &OWNER_NO_OWNER, &[], false, None, None));
|
||||
|
@ -722,10 +722,11 @@ pub mod tests {
|
|||
const TEST_PADDING: u8 = 5;
|
||||
const TEST_OWNER_OFFSET: OwnerOffset = OwnerOffset(0x1fef_1234);
|
||||
const TEST_RENT_EPOCH: Epoch = 7;
|
||||
let acc_hash = AccountHash(Hash::new_unique());
|
||||
|
||||
let optional_fields = AccountMetaOptionalFields {
|
||||
rent_epoch: Some(TEST_RENT_EPOCH),
|
||||
account_hash: Some(AccountHash(Hash::new_unique())),
|
||||
account_hash: Some(&acc_hash),
|
||||
};
|
||||
|
||||
let flags = AccountMetaFlags::new_from(&optional_fields);
|
||||
|
@ -745,6 +746,7 @@ pub mod tests {
|
|||
fn test_hot_account_meta_full() {
|
||||
let account_data = [11u8; 83];
|
||||
let padding = [0u8; 5];
|
||||
let acc_hash = AccountHash(Hash::new_unique());
|
||||
|
||||
const TEST_LAMPORT: u64 = 2314232137;
|
||||
const OWNER_OFFSET: u32 = 0x1fef_1234;
|
||||
|
@ -752,7 +754,7 @@ pub mod tests {
|
|||
|
||||
let optional_fields = AccountMetaOptionalFields {
|
||||
rent_epoch: Some(TEST_RENT_EPOCH),
|
||||
account_hash: Some(AccountHash(Hash::new_unique())),
|
||||
account_hash: Some(&acc_hash),
|
||||
};
|
||||
|
||||
let flags = AccountMetaFlags::new_from(&optional_fields);
|
||||
|
@ -789,7 +791,7 @@ pub mod tests {
|
|||
assert_eq!(account_data, meta.account_data(account_block));
|
||||
assert_eq!(meta.rent_epoch(account_block), optional_fields.rent_epoch);
|
||||
assert_eq!(
|
||||
*(meta.account_hash(account_block).unwrap()),
|
||||
(meta.account_hash(account_block).unwrap()),
|
||||
optional_fields.account_hash.unwrap()
|
||||
);
|
||||
}
|
||||
|
@ -1339,7 +1341,7 @@ pub mod tests {
|
|||
acc.owner(),
|
||||
acc.data(),
|
||||
acc.executable(),
|
||||
// only persist rent_epoch for those non-rent-exempt accounts
|
||||
// only persist rent_epoch for those rent-paying accounts
|
||||
Some(*account_hash),
|
||||
)
|
||||
})
|
||||
|
|
|
@ -102,14 +102,14 @@ impl AccountMetaFlags {
|
|||
/// Note that the storage representation of the optional fields might be
|
||||
/// different from its in-memory representation.
|
||||
#[derive(Debug, PartialEq, Eq, Clone)]
|
||||
pub struct AccountMetaOptionalFields {
|
||||
pub struct AccountMetaOptionalFields<'a> {
|
||||
/// the epoch at which its associated account will next owe rent
|
||||
pub rent_epoch: Option<Epoch>,
|
||||
/// the hash of its associated account
|
||||
pub account_hash: Option<AccountHash>,
|
||||
pub account_hash: Option<&'a AccountHash>,
|
||||
}
|
||||
|
||||
impl AccountMetaOptionalFields {
|
||||
impl<'a> AccountMetaOptionalFields<'a> {
|
||||
/// The size of the optional fields in bytes (excluding the boolean flags).
|
||||
pub fn size(&self) -> usize {
|
||||
self.rent_epoch.map_or(0, |_| std::mem::size_of::<Epoch>())
|
||||
|
@ -210,9 +210,10 @@ pub mod tests {
|
|||
#[test]
|
||||
fn test_optional_fields_update_flags() {
|
||||
let test_epoch = 5432312;
|
||||
let acc_hash = AccountHash(Hash::new_unique());
|
||||
|
||||
for rent_epoch in [None, Some(test_epoch)] {
|
||||
for account_hash in [None, Some(AccountHash(Hash::new_unique()))] {
|
||||
for account_hash in [None, Some(&acc_hash)] {
|
||||
update_and_verify_flags(&AccountMetaOptionalFields {
|
||||
rent_epoch,
|
||||
account_hash,
|
||||
|
@ -224,9 +225,10 @@ pub mod tests {
|
|||
#[test]
|
||||
fn test_optional_fields_size() {
|
||||
let test_epoch = 5432312;
|
||||
let acc_hash = AccountHash(Hash::new_unique());
|
||||
|
||||
for rent_epoch in [None, Some(test_epoch)] {
|
||||
for account_hash in [None, Some(AccountHash(Hash::new_unique()))] {
|
||||
for account_hash in [None, Some(&acc_hash)] {
|
||||
let opt_fields = AccountMetaOptionalFields {
|
||||
rent_epoch,
|
||||
account_hash,
|
||||
|
@ -249,16 +251,17 @@ pub mod tests {
|
|||
#[test]
|
||||
fn test_optional_fields_offset() {
|
||||
let test_epoch = 5432312;
|
||||
let acc_hash = AccountHash(Hash::new_unique());
|
||||
|
||||
for rent_epoch in [None, Some(test_epoch)] {
|
||||
for account_hash in [None, Some(AccountHash(Hash::new_unique()))] {
|
||||
for account_hash in [None, Some(&acc_hash)] {
|
||||
let rent_epoch_offset = 0;
|
||||
let account_hash_offset =
|
||||
rent_epoch_offset + rent_epoch.as_ref().map(std::mem::size_of_val).unwrap_or(0);
|
||||
let derived_size = account_hash_offset
|
||||
+ account_hash
|
||||
.as_ref()
|
||||
.map(std::mem::size_of_val)
|
||||
.map(|acc_hash| std::mem::size_of_val(*acc_hash))
|
||||
.unwrap_or(0);
|
||||
let opt_fields = AccountMetaOptionalFields {
|
||||
rent_epoch,
|
||||
|
|
Loading…
Reference in New Issue