From d651409c403d26b1840cf19f70b146247fe81739 Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Wed, 28 Aug 2024 13:15:07 -0500 Subject: [PATCH] account_saver: Remove nested options (#2724) --- accounts-db/src/accounts.rs | 4 +- accounts-db/src/accounts_db.rs | 60 +++++++------------ .../src/accounts_db/geyser_plugin_utils.rs | 10 ++-- runtime/src/bank.rs | 7 ++- svm/src/account_saver.rs | 23 +++---- 5 files changed, 44 insertions(+), 60 deletions(-) diff --git a/accounts-db/src/accounts.rs b/accounts-db/src/accounts.rs index eb416c29e2f4e9..8de5431318a3a0 100644 --- a/accounts-db/src/accounts.rs +++ b/accounts-db/src/accounts.rs @@ -588,10 +588,10 @@ impl Accounts { pub fn store_cached<'a>( &self, accounts: impl StorableAccounts<'a>, - transactions: &'a [Option<&'a SanitizedTransaction>], + transactions: Option<&'a [&'a SanitizedTransaction]>, ) { self.accounts_db - .store_cached_inline_update_index(accounts, Some(transactions)); + .store_cached_inline_update_index(accounts, transactions); } pub fn store_accounts_cached<'a>(&self, accounts: impl StorableAccounts<'a>) { diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index 641c7d63f9f013..9f43360e4f1b71 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -90,6 +90,7 @@ use { hash::Hash, pubkey::Pubkey, rent_collector::RentCollector, + saturating_add_assign, timing::AtomicInterval, transaction::SanitizedTransaction, }, @@ -6817,27 +6818,20 @@ impl AccountsDb { &self, slot: Slot, accounts_and_meta_to_store: &impl StorableAccounts<'b>, - txn_iter: Box> + 'a>, + txs: Option<&[&SanitizedTransaction]>, ) -> Vec { - let mut write_version_producer: Box> = - if self.accounts_update_notifier.is_some() { - let mut current_version = self - .write_version - .fetch_add(accounts_and_meta_to_store.len() as u64, Ordering::AcqRel); - Box::new(std::iter::from_fn(move || { - let ret = current_version; - current_version += 1; - Some(ret) - })) - } else { - Box::new(std::iter::empty()) - }; + let mut current_write_version = if self.accounts_update_notifier.is_some() { + self.write_version + .fetch_add(accounts_and_meta_to_store.len() as u64, Ordering::AcqRel) + } else { + 0 + }; - let (account_infos, cached_accounts) = txn_iter - .enumerate() - .map(|(i, txn)| { + let (account_infos, cached_accounts) = (0..accounts_and_meta_to_store.len()) + .map(|index| { + let txn = txs.map(|txs| *txs.get(index).expect("txs must be present if provided")); let mut account_info = AccountInfo::default(); - accounts_and_meta_to_store.account_default_if_zero_lamport(i, |account| { + accounts_and_meta_to_store.account_default_if_zero_lamport(index, |account| { let account_shared_data = account.to_account_shared_data(); let pubkey = account.pubkey(); account_info = AccountInfo::new(StorageLocation::Cached, account.lamports()); @@ -6845,10 +6839,11 @@ impl AccountsDb { self.notify_account_at_accounts_update( slot, &account_shared_data, - txn, + &txn, pubkey, - &mut write_version_producer, + current_write_version, ); + saturating_add_assign!(current_write_version, 1); let cached_account = self.accounts_cache.store(slot, pubkey, account_shared_data); @@ -6872,7 +6867,7 @@ impl AccountsDb { &self, accounts: &'c impl StorableAccounts<'b>, store_to: &StoreTo, - transactions: Option<&[Option<&'a SanitizedTransaction>]>, + transactions: Option<&'a [&'a SanitizedTransaction]>, ) -> Vec { let mut calc_stored_meta_time = Measure::start("calc_stored_meta"); let slot = accounts.target_slot(); @@ -6895,18 +6890,7 @@ impl AccountsDb { .fetch_add(calc_stored_meta_time.as_us(), Ordering::Relaxed); match store_to { - StoreTo::Cache => { - let txn_iter: Box>> = - match transactions { - Some(transactions) => { - assert_eq!(transactions.len(), accounts.len()); - Box::new(transactions.iter()) - } - None => Box::new(std::iter::repeat(&None).take(accounts.len())), - }; - - self.write_accounts_to_cache(slot, accounts, txn_iter) - } + StoreTo::Cache => self.write_accounts_to_cache(slot, accounts, transactions), StoreTo::Storage(storage) => self.write_accounts_to_storage(slot, storage, accounts), } } @@ -8292,7 +8276,7 @@ impl AccountsDb { pub fn store_cached<'a>( &self, accounts: impl StorableAccounts<'a>, - transactions: Option<&'a [Option<&'a SanitizedTransaction>]>, + transactions: Option<&'a [&'a SanitizedTransaction]>, ) { self.store( accounts, @@ -8306,7 +8290,7 @@ impl AccountsDb { pub(crate) fn store_cached_inline_update_index<'a>( &self, accounts: impl StorableAccounts<'a>, - transactions: Option<&'a [Option<&'a SanitizedTransaction>]>, + transactions: Option<&'a [&'a SanitizedTransaction]>, ) { self.store( accounts, @@ -8334,7 +8318,7 @@ impl AccountsDb { &self, accounts: impl StorableAccounts<'a>, store_to: &StoreTo, - transactions: Option<&'a [Option<&'a SanitizedTransaction>]>, + transactions: Option<&'a [&'a SanitizedTransaction]>, reclaim: StoreReclaims, update_index_thread_selection: UpdateIndexThreadSelection, ) { @@ -8523,7 +8507,7 @@ impl AccountsDb { &self, accounts: impl StorableAccounts<'a>, store_to: &StoreTo, - transactions: Option<&'a [Option<&'a SanitizedTransaction>]>, + transactions: Option<&'a [&'a SanitizedTransaction]>, reclaim: StoreReclaims, update_index_thread_selection: UpdateIndexThreadSelection, ) { @@ -8569,7 +8553,7 @@ impl AccountsDb { accounts: impl StorableAccounts<'a>, store_to: &StoreTo, reset_accounts: bool, - transactions: Option<&[Option<&SanitizedTransaction>]>, + transactions: Option<&'a [&'a SanitizedTransaction]>, reclaim: StoreReclaims, update_index_thread_selection: UpdateIndexThreadSelection, ) -> StoreAccountsTiming { diff --git a/accounts-db/src/accounts_db/geyser_plugin_utils.rs b/accounts-db/src/accounts_db/geyser_plugin_utils.rs index 6f94f4300de675..cedebafa4f08e7 100644 --- a/accounts-db/src/accounts_db/geyser_plugin_utils.rs +++ b/accounts-db/src/accounts_db/geyser_plugin_utils.rs @@ -57,23 +57,21 @@ impl AccountsDb { notify_stats.report(); } - pub fn notify_account_at_accounts_update

( + pub fn notify_account_at_accounts_update( &self, slot: Slot, account: &AccountSharedData, txn: &Option<&SanitizedTransaction>, pubkey: &Pubkey, - write_version_producer: &mut P, - ) where - P: Iterator, - { + write_version: u64, + ) { if let Some(accounts_update_notifier) = &self.accounts_update_notifier { accounts_update_notifier.notify_account_update( slot, account, txn, pubkey, - write_version_producer.next().unwrap(), + write_version, ); } } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 95661ce5318c7a..ee138a1b516a3c 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -3806,9 +3806,10 @@ impl Bank { &durable_nonce, lamports_per_signature, ); - self.rc - .accounts - .store_cached((self.slot(), accounts_to_store.as_slice()), &transactions); + self.rc.accounts.store_cached( + (self.slot(), accounts_to_store.as_slice()), + transactions.as_deref(), + ); }); self.collect_rent(&processing_results); diff --git a/svm/src/account_saver.rs b/svm/src/account_saver.rs index 0ecbd181e6698f..ca3c45dbfd50f3 100644 --- a/svm/src/account_saver.rs +++ b/svm/src/account_saver.rs @@ -45,7 +45,7 @@ pub fn collect_accounts_to_store<'a, T: SVMMessage>( processing_results: &'a mut [TransactionProcessingResult], durable_nonce: &DurableNonce, lamports_per_signature: u64, -) -> (Vec<(&'a Pubkey, &'a AccountSharedData)>, Vec>) { +) -> (Vec<(&'a Pubkey, &'a AccountSharedData)>, Option>) { let collect_capacity = max_number_of_accounts_to_collect(txs, processing_results); let mut accounts = Vec::with_capacity(collect_capacity); let mut transactions = Vec::with_capacity(collect_capacity); @@ -87,12 +87,12 @@ pub fn collect_accounts_to_store<'a, T: SVMMessage>( } } } - (accounts, transactions) + (accounts, Some(transactions)) } fn collect_accounts_for_successful_tx<'a, T: SVMMessage>( collected_accounts: &mut Vec<(&'a Pubkey, &'a AccountSharedData)>, - collected_account_transactions: &mut Vec>, + collected_account_transactions: &mut Vec<&'a T>, transaction: &'a T, transaction_accounts: &'a [TransactionAccount], ) { @@ -109,13 +109,13 @@ fn collect_accounts_for_successful_tx<'a, T: SVMMessage>( }) { collected_accounts.push((address, account)); - collected_account_transactions.push(Some(transaction)); + collected_account_transactions.push(transaction); } } fn collect_accounts_for_failed_tx<'a, T: SVMMessage>( collected_accounts: &mut Vec<(&'a Pubkey, &'a AccountSharedData)>, - collected_account_transactions: &mut Vec>, + collected_account_transactions: &mut Vec<&'a T>, transaction: &'a T, rollback_accounts: &'a mut RollbackAccounts, durable_nonce: &DurableNonce, @@ -125,7 +125,7 @@ fn collect_accounts_for_failed_tx<'a, T: SVMMessage>( match rollback_accounts { RollbackAccounts::FeePayerOnly { fee_payer_account } => { collected_accounts.push((fee_payer_address, &*fee_payer_account)); - collected_account_transactions.push(Some(transaction)); + collected_account_transactions.push(transaction); } RollbackAccounts::SameNonceAndFeePayer { nonce } => { // Since we know we are dealing with a valid nonce account, @@ -134,14 +134,14 @@ fn collect_accounts_for_failed_tx<'a, T: SVMMessage>( .try_advance_nonce(*durable_nonce, lamports_per_signature) .unwrap(); collected_accounts.push((nonce.address(), nonce.account())); - collected_account_transactions.push(Some(transaction)); + collected_account_transactions.push(transaction); } RollbackAccounts::SeparateNonceAndFeePayer { nonce, fee_payer_account, } => { collected_accounts.push((fee_payer_address, &*fee_payer_account)); - collected_account_transactions.push(Some(transaction)); + collected_account_transactions.push(transaction); // Since we know we are dealing with a valid nonce account, // unwrap is safe here @@ -149,7 +149,7 @@ fn collect_accounts_for_failed_tx<'a, T: SVMMessage>( .try_advance_nonce(*durable_nonce, lamports_per_signature) .unwrap(); collected_accounts.push((nonce.address(), nonce.account())); - collected_account_transactions.push(Some(transaction)); + collected_account_transactions.push(transaction); } } } @@ -294,9 +294,10 @@ mod tests { .iter() .any(|(pubkey, _account)| *pubkey == &keypair1.pubkey())); + let transactions = transactions.unwrap(); assert_eq!(transactions.len(), 2); - assert!(transactions.iter().any(|txn| txn.unwrap().eq(&tx0))); - assert!(transactions.iter().any(|txn| txn.unwrap().eq(&tx1))); + assert!(transactions.iter().any(|txn| (*txn).eq(&tx0))); + assert!(transactions.iter().any(|txn| (*txn).eq(&tx1))); } #[test]