From 1864bc2080287e61fbd7e8c20358d1ac11b0f7f9 Mon Sep 17 00:00:00 2001 From: "Jeff Washington (jwash)" <75863576+jeffwashington@users.noreply.github.com> Date: Wed, 28 Apr 2021 08:47:26 -0500 Subject: [PATCH] write Option (#16874) * write Option<&AccountSharedData> * add comment --- runtime/src/accounts_db.rs | 50 +++++++++++++++++++--------------- runtime/src/append_vec.rs | 55 +++++++++++++++++++++++++++++++++++--- 2 files changed, 81 insertions(+), 24 deletions(-) diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index b8a348d19b..301f1ac807 100644 --- a/runtime/src/accounts_db.rs +++ b/runtime/src/accounts_db.rs @@ -3323,7 +3323,7 @@ impl AccountsDb { slot: Slot, hashes: &[impl Borrow], mut storage_finder: F, - accounts_and_meta_to_store: &[(StoredMeta, &AccountSharedData)], + accounts_and_meta_to_store: &[(StoredMeta, Option<&AccountSharedData>)], ) -> Vec { assert_eq!(hashes.len(), accounts_and_meta_to_store.len()); let mut infos: Vec = Vec::with_capacity(accounts_and_meta_to_store.len()); @@ -3331,7 +3331,10 @@ impl AccountsDb { let mut total_storage_find_us = 0; while infos.len() < accounts_and_meta_to_store.len() { let mut storage_find = Measure::start("storage_finder"); - let data_len = accounts_and_meta_to_store[infos.len()].1.data().len(); + let data_len = accounts_and_meta_to_store[infos.len()] + .1 + .map(|account| account.data().len()) + .unwrap_or_default(); let storage = storage_finder(slot, data_len + STORE_META_OVERHEAD); storage_find.stop(); total_storage_find_us += storage_find.as_us(); @@ -3377,7 +3380,9 @@ impl AccountsDb { store_id: storage.append_vec_id(), offset: offsets[0], stored_size, - lamports: account.lamports(), + lamports: account + .map(|account| account.lamports()) + .unwrap_or_default(), }); } // restore the state to available @@ -3705,7 +3710,7 @@ impl AccountsDb { &self, slot: Slot, hashes: Option<&[impl Borrow]>, - accounts_and_meta_to_store: &[(StoredMeta, &AccountSharedData)], + accounts_and_meta_to_store: &[(StoredMeta, Option<&AccountSharedData>)], ) -> Vec { let len = accounts_and_meta_to_store.len(); let hashes = hashes.map(|hashes| { @@ -3718,9 +3723,17 @@ impl AccountsDb { .enumerate() .map(|(i, (meta, account))| { let hash = hashes.map(|hashes| hashes[i].borrow()); - let cached_account = - self.accounts_cache - .store(slot, &meta.pubkey, (*account).clone(), hash); + + let account = account.cloned().unwrap_or_default(); + + let account_info = AccountInfo { + store_id: CACHE_VIRTUAL_STORAGE_ID, + offset: CACHE_VIRTUAL_OFFSET, + stored_size: CACHE_VIRTUAL_STORED_SIZE, + lamports: account.lamports(), + }; + + let cached_account = self.accounts_cache.store(slot, &meta.pubkey, account, hash); // hash this account in the bg match &self.sender_bg_hasher { Some(ref sender) => { @@ -3728,13 +3741,7 @@ impl AccountsDb { } None => (), }; - - AccountInfo { - store_id: CACHE_VIRTUAL_STORAGE_ID, - offset: CACHE_VIRTUAL_OFFSET, - stored_size: CACHE_VIRTUAL_STORED_SIZE, - lamports: account.lamports(), - } + account_info }) .collect() } @@ -3751,17 +3758,18 @@ impl AccountsDb { mut write_version_producer: P, is_cached_store: bool, ) -> Vec { - let default_account = AccountSharedData::default(); - let accounts_and_meta_to_store: Vec<(StoredMeta, &AccountSharedData)> = accounts + let accounts_and_meta_to_store: Vec<(StoredMeta, Option<&AccountSharedData>)> = accounts .iter() .map(|(pubkey, account)| { self.read_only_accounts_cache.remove(pubkey, slot); - let account = if account.lamports() == 0 { - &default_account + // this is the source of Some(Account) or None. + // Some(Account) = store 'Account' + // None = store a default/empty account with 0 lamports + let (account, data_len) = if account.lamports() == 0 { + (None, 0) } else { - *account + (Some(*account), account.data().len() as u64) }; - let data_len = account.data().len() as u64; let meta = StoredMeta { write_version: write_version_producer.next().unwrap(), pubkey: **pubkey, @@ -5822,7 +5830,7 @@ pub mod tests { }; storages[0][0] .accounts - .append_accounts(&[(sm, &acc)], &[&Hash::default()]); + .append_accounts(&[(sm, Some(&acc))], &[&Hash::default()]); let calls = AtomicU64::new(0); let result = AccountsDb::scan_account_storage_no_bank( diff --git a/runtime/src/append_vec.rs b/runtime/src/append_vec.rs index 134238d2d1..54b27f73a8 100644 --- a/runtime/src/append_vec.rs +++ b/runtime/src/append_vec.rs @@ -69,6 +69,15 @@ impl<'a, T: ReadableAccount> From<&'a T> for AccountMeta { } } +impl<'a, T: ReadableAccount> From> for AccountMeta { + fn from(account: Option<&'a T>) -> Self { + match account { + Some(account) => AccountMeta::from(account), + None => AccountMeta::default(), + } + } +} + /// References to account data stored elsewhere. Getting an `Account` requires cloning /// (see `StoredAccountMeta::clone_account()`). #[derive(PartialEq, Debug)] @@ -453,7 +462,7 @@ impl AppendVec { /// and will be available to other threads. pub fn append_accounts( &self, - accounts: &[(StoredMeta, &AccountSharedData)], + accounts: &[(StoredMeta, Option<&AccountSharedData>)], hashes: &[impl Borrow], ) -> Vec { let _lock = self.append_lock.lock().unwrap(); @@ -464,7 +473,10 @@ impl AppendVec { let account_meta = AccountMeta::from(*account); let account_meta_ptr = &account_meta as *const AccountMeta; let data_len = stored_meta.data_len as usize; - let data_ptr = account.data().as_ptr(); + let data_ptr = account + .map(|account| account.data()) + .unwrap_or_default() + .as_ptr(); let hash_ptr = hash.borrow().as_ref().as_ptr(); let ptrs = [ (meta_ptr as *const u8, mem::size_of::()), @@ -495,7 +507,7 @@ impl AppendVec { account: &AccountSharedData, hash: Hash, ) -> Option { - let res = self.append_accounts(&[(storage_meta, account)], &[&hash]); + let res = self.append_accounts(&[(storage_meta, Some(account))], &[&hash]); if res.len() == 1 { None } else { @@ -592,6 +604,43 @@ pub mod tests { } } + #[test] + fn test_account_meta_default() { + let def1 = AccountMeta::default(); + let def2 = AccountMeta::from(&Account::default()); + assert_eq!(&def1, &def2); + let def2 = AccountMeta::from(&AccountSharedData::default()); + assert_eq!(&def1, &def2); + let def2 = AccountMeta::from(Some(&AccountSharedData::default())); + assert_eq!(&def1, &def2); + let none: Option<&AccountSharedData> = None; + let def2 = AccountMeta::from(none); + assert_eq!(&def1, &def2); + } + + #[test] + fn test_account_meta_non_default() { + let def1 = AccountMeta { + lamports: 1, + owner: Pubkey::new_unique(), + executable: true, + rent_epoch: 3, + }; + let def2_account = Account { + lamports: def1.lamports, + owner: def1.owner, + executable: def1.executable, + rent_epoch: def1.rent_epoch, + data: Vec::new(), + }; + let def2 = AccountMeta::from(&def2_account); + assert_eq!(&def1, &def2); + let def2 = AccountMeta::from(&AccountSharedData::from(def2_account.clone())); + assert_eq!(&def1, &def2); + let def2 = AccountMeta::from(Some(&AccountSharedData::from(def2_account))); + assert_eq!(&def1, &def2); + } + #[test] #[should_panic(expected = "too small file size 0 for AppendVec")] fn test_append_vec_new_bad_size() {