From 5861430d12fc2a669a1866d88db40889a3c4e34a Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Fri, 23 Aug 2024 14:10:26 -0500 Subject: [PATCH 1/6] Remove nested options --- accounts-db/src/accounts.rs | 2 +- accounts-db/src/accounts_db.rs | 37 +++++++++++++++++----------------- svm/src/account_saver.rs | 20 +++++++++--------- 3 files changed, 29 insertions(+), 30 deletions(-) diff --git a/accounts-db/src/accounts.rs b/accounts-db/src/accounts.rs index eb416c29e2f4e9..3b8477890bdebd 100644 --- a/accounts-db/src/accounts.rs +++ b/accounts-db/src/accounts.rs @@ -588,7 +588,7 @@ impl Accounts { pub fn store_cached<'a>( &self, accounts: impl StorableAccounts<'a>, - transactions: &'a [Option<&'a SanitizedTransaction>], + transactions: &'a [&'a SanitizedTransaction], ) { self.accounts_db .store_cached_inline_update_index(accounts, Some(transactions)); diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index 9095a29b19cc50..58745535a68873 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -6809,7 +6809,7 @@ impl AccountsDb { &self, slot: Slot, accounts_and_meta_to_store: &impl StorableAccounts<'b>, - txn_iter: Box> + 'a>, + mut txn_iter: impl Iterator, ) -> Vec { let mut write_version_producer: Box> = if self.accounts_update_notifier.is_some() { @@ -6825,11 +6825,11 @@ impl AccountsDb { Box::new(std::iter::empty()) }; - 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 = &txn_iter.next(); // if empty, then None 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()); @@ -6864,7 +6864,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(); @@ -6888,14 +6888,13 @@ impl AccountsDb { 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())), - }; + let txn_iter = match transactions { + Some(transactions) => { + assert_eq!(transactions.len(), accounts.len()); + transactions.iter().copied() + } + None => [].iter().copied(), + }; self.write_accounts_to_cache(slot, accounts, txn_iter) } @@ -8279,7 +8278,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, @@ -8293,7 +8292,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, @@ -8321,7 +8320,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, ) { @@ -8510,7 +8509,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, ) { @@ -8556,7 +8555,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/svm/src/account_saver.rs b/svm/src/account_saver.rs index 0ecbd181e6698f..379283399cd920 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)>, Vec<&'a T>) { 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); @@ -92,7 +92,7 @@ pub fn collect_accounts_to_store<'a, T: SVMMessage>( 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); } } } @@ -295,8 +295,8 @@ mod tests { .any(|(pubkey, _account)| *pubkey == &keypair1.pubkey())); 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] From a3b52fe1590e60d31300dd48ad4e40990a7d3134 Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Fri, 23 Aug 2024 14:16:18 -0500 Subject: [PATCH 2/6] remove box --- accounts-db/src/accounts_db.rs | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index 58745535a68873..6bffa2c65a4f44 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -6811,19 +6811,14 @@ impl AccountsDb { accounts_and_meta_to_store: &impl StorableAccounts<'b>, mut txn_iter: impl Iterator, ) -> 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 write_version_producer = if self.accounts_update_notifier.is_some() { + let current_version = self + .write_version + .fetch_add(accounts_and_meta_to_store.len() as u64, Ordering::AcqRel); + current_version..current_version + accounts_and_meta_to_store.len() as u64 + } else { + 0..0 // dummy empty iterator + }; let (account_infos, cached_accounts) = (0..accounts_and_meta_to_store.len()) .map(|index| { From 2ba61d06c5c8a20792b3b6ab7965ad18ebbe6bbd Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Fri, 23 Aug 2024 14:49:00 -0500 Subject: [PATCH 3/6] Option --- accounts-db/src/accounts.rs | 4 ++-- runtime/src/bank.rs | 7 ++++--- svm/src/account_saver.rs | 5 +++-- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/accounts-db/src/accounts.rs b/accounts-db/src/accounts.rs index 3b8477890bdebd..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 [&'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/runtime/src/bank.rs b/runtime/src/bank.rs index 34a1cd9d70b89c..4b17701df20efa 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -3801,9 +3801,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 379283399cd920..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<&'a T>) { +) -> (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,7 +87,7 @@ pub fn collect_accounts_to_store<'a, T: SVMMessage>( } } } - (accounts, transactions) + (accounts, Some(transactions)) } fn collect_accounts_for_successful_tx<'a, T: SVMMessage>( @@ -294,6 +294,7 @@ mod tests { .iter() .any(|(pubkey, _account)| *pubkey == &keypair1.pubkey())); + let transactions = transactions.unwrap(); assert_eq!(transactions.len(), 2); assert!(transactions.iter().any(|txn| (*txn).eq(&tx0))); assert!(transactions.iter().any(|txn| (*txn).eq(&tx1))); From 2557ee589b3dcbe5e61595335ae5dbe5b1e8c266 Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Tue, 27 Aug 2024 08:10:51 -0500 Subject: [PATCH 4/6] use u64 to track write-version, manual iteration --- accounts-db/src/accounts_db.rs | 15 ++++++--------- .../src/accounts_db/geyser_plugin_utils.rs | 10 ++++------ 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index 6bffa2c65a4f44..a5fd0a704be76f 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, }, @@ -6811,14 +6812,9 @@ impl AccountsDb { accounts_and_meta_to_store: &impl StorableAccounts<'b>, mut txn_iter: impl Iterator, ) -> Vec { - let mut write_version_producer = if self.accounts_update_notifier.is_some() { - let current_version = self - .write_version - .fetch_add(accounts_and_meta_to_store.len() as u64, Ordering::AcqRel); - current_version..current_version + accounts_and_meta_to_store.len() as u64 - } else { - 0..0 // dummy empty iterator - }; + let mut current_write_version = self + .write_version + .fetch_add(accounts_and_meta_to_store.len() as u64, Ordering::AcqRel); let (account_infos, cached_accounts) = (0..accounts_and_meta_to_store.len()) .map(|index| { @@ -6834,8 +6830,9 @@ impl AccountsDb { &account_shared_data, 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); 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, ); } } From de664e8f5f8238d2793b980f735857a807c70ce1 Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Tue, 27 Aug 2024 08:19:41 -0500 Subject: [PATCH 5/6] pass option slice directly --- accounts-db/src/accounts_db.rs | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index a5fd0a704be76f..cb53460684843f 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -6810,7 +6810,7 @@ impl AccountsDb { &self, slot: Slot, accounts_and_meta_to_store: &impl StorableAccounts<'b>, - mut txn_iter: impl Iterator, + txs: Option<&[&SanitizedTransaction]>, ) -> Vec { let mut current_write_version = self .write_version @@ -6818,7 +6818,7 @@ impl AccountsDb { let (account_infos, cached_accounts) = (0..accounts_and_meta_to_store.len()) .map(|index| { - let txn = &txn_iter.next(); // if empty, then None + 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(index, |account| { let account_shared_data = account.to_account_shared_data(); @@ -6828,7 +6828,7 @@ impl AccountsDb { self.notify_account_at_accounts_update( slot, &account_shared_data, - txn, + &txn, pubkey, current_write_version, ); @@ -6879,17 +6879,7 @@ impl AccountsDb { .fetch_add(calc_stored_meta_time.as_us(), Ordering::Relaxed); match store_to { - StoreTo::Cache => { - let txn_iter = match transactions { - Some(transactions) => { - assert_eq!(transactions.len(), accounts.len()); - transactions.iter().copied() - } - None => [].iter().copied(), - }; - - 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), } } From ae2284a6f83a264e3d009d012b4e1a3ab9e21b95 Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Wed, 28 Aug 2024 07:59:14 -0500 Subject: [PATCH 6/6] only update write_version if accounts notifier is some --- accounts-db/src/accounts_db.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index cb53460684843f..64c1f4f85f9bcc 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -6812,9 +6812,12 @@ impl AccountsDb { accounts_and_meta_to_store: &impl StorableAccounts<'b>, txs: Option<&[&SanitizedTransaction]>, ) -> Vec { - let mut current_write_version = self - .write_version - .fetch_add(accounts_and_meta_to_store.len() as u64, Ordering::AcqRel); + 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) = (0..accounts_and_meta_to_store.len()) .map(|index| {