From 824415ee2422875a21f34a4653268f6c29b3b880 Mon Sep 17 00:00:00 2001 From: jeff washington Date: Mon, 18 Dec 2023 17:27:29 -0600 Subject: [PATCH 1/6] wip: accumulate hash of all accounts --- accounts-db/src/accounts_db.rs | 30 +++++++++++++++++++++++++++--- runtime/src/bank.rs | 22 +++++++++++++++++++++- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index 570ff8c26a415c..67b5dcf2d31194 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -7916,10 +7916,33 @@ impl AccountsDb { /// /// As part of calculating the accounts delta hash, get a list of accounts modified this slot /// (aka dirty pubkeys) and add them to `self.uncleaned_pubkeys` for future cleaning. - pub fn calculate_accounts_delta_hash(&self, slot: Slot) -> AccountsDeltaHash { + pub fn calculate_accounts_delta_hash(&self, slot: Slot) -> (AccountsDeltaHash, Vec<(Pubkey, AccountHash)>) { self.calculate_accounts_delta_hash_internal(slot, None, HashMap::default()) } + pub fn accumulate_accounts_hash( + &self, + slot: Slot, + mut ancestors: Ancestors, + accumulated_accounts_hash: &mut Hash, + pubkey_hash: Vec<(Pubkey, AccountHash)>, + ) { + // we are assuming it was easy to lookup a hash for everything written in `slot` when we were calculating the delta hash. So, caller passes in `pubkey_hash` + // note we don't need rewrites in `pubkey_hash`. these accounts had the same hash before and after. So, we only have to consider what was written that changed. + ancestors.remove(&slot); + // if we want to look it up ourselves: let (hashes, _scan_us, _accumulate) = self.get_pubkey_hash_for_slot(slot); + let old = pubkey_hash.iter().map(|(k, _)| { + self.load_with_fixed_root(&ancestors, k).map(|(account, _)| Self::hash_account(&account, k)) + }).collect::>(); + pubkey_hash.into_iter().zip(old.into_iter()).for_each(|((k, new_hash), old_hash)| { + if let Some(old) = old_hash { + // todo if old == new, then we can avoid this update altogether + // todo subtract accumulated_accounts_hash -= old_hash + } + // todo add accumulated_accounts_hash += new_hash + }); + } + /// Calculate accounts delta hash for `slot` /// /// As part of calculating the accounts delta hash, get a list of accounts modified this slot @@ -7929,8 +7952,9 @@ impl AccountsDb { slot: Slot, ignore: Option, mut skipped_rewrites: HashMap, - ) -> AccountsDeltaHash { + ) -> (AccountsDeltaHash, Vec<(Pubkey, AccountHash)>) { let (mut hashes, scan_us, mut accumulate) = self.get_pubkey_hash_for_slot(slot); + let original_pubkey_hash = hashes.clone(); let dirty_keys = hashes.iter().map(|(pubkey, _hash)| *pubkey).collect(); hashes.iter().for_each(|(k, _h)| { @@ -7969,7 +7993,7 @@ impl AccountsDb { .skipped_rewrites_num .fetch_add(num_skipped_rewrites, Ordering::Relaxed); - accounts_delta_hash + (accounts_delta_hash, original_pubkey_hash) } /// Set the accounts delta hash for `slot` in the `accounts_delta_hashes` map diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index b865c86d2b35d3..3fd704a525127f 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -511,6 +511,7 @@ impl PartialEq for Bank { return true; } let Self { + accumulated_accounts_hash: _, skipped_rewrites: _, rc: _, status_cache: _, @@ -833,6 +834,8 @@ pub struct Bank { pub check_program_modification_slot: bool, + pub accumulated_accounts_hash: RwLock>, + epoch_reward_status: EpochRewardStatus, } @@ -976,6 +979,7 @@ impl Bank { fn default_with_accounts(accounts: Accounts) -> Self { let mut bank = Self { + accumulated_accounts_hash: RwLock::default(), skipped_rewrites: Mutex::default(), incremental_snapshot_persistence: None, rc: BankRc::new(accounts, Slot::default()), @@ -1312,6 +1316,7 @@ impl Bank { let accounts_data_size_initial = parent.load_accounts_data_size(); let mut new = Self { + accumulated_accounts_hash: RwLock::default(), skipped_rewrites: Mutex::default(), incremental_snapshot_persistence: None, rc, @@ -1827,6 +1832,7 @@ impl Bank { ); let stakes_accounts_load_duration = now.elapsed(); let mut bank = Self { + accumulated_accounts_hash: RwLock::default(), skipped_rewrites: Mutex::default(), incremental_snapshot_persistence: fields.incremental_snapshot_persistence, rc: bank_rc, @@ -6984,7 +6990,7 @@ impl Bank { && self.get_reward_calculation_num_blocks() == 0 && self.partitioned_rewards_stake_account_stores_per_block() == u64::MAX)) .then_some(sysvar::epoch_rewards::id()); - let accounts_delta_hash = self + let (accounts_delta_hash, pubkey_hash) = self .rc .accounts .accounts_db @@ -6994,6 +7000,20 @@ impl Bank { self.skipped_rewrites.lock().unwrap().clone(), ); + let mut accumulated = self.parent().map(|bank| bank.accumulated_accounts_hash.read().unwrap().unwrap_or_default()).unwrap_or_default(); // todo probably not default here + self + .rc + .accounts + .accounts_db + .accumulate_accounts_hash( + slot, + self.ancestors.clone(), + &mut accumulated, + pubkey_hash, + ); + + *self.accumulated_accounts_hash.write().unwrap() = Some(accumulated); + let mut signature_count_buf = [0u8; 8]; LittleEndian::write_u64(&mut signature_count_buf[..], self.signature_count()); From 8503bbd1fdcc4e468d22f1a4ab5981b5ac22db45 Mon Sep 17 00:00:00 2001 From: jeff washington Date: Mon, 18 Dec 2023 17:38:48 -0600 Subject: [PATCH 2/6] fix up storage version of accounts hash --- accounts-db/src/accounts_hash.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/accounts-db/src/accounts_hash.rs b/accounts-db/src/accounts_hash.rs index 72f74be6f130d1..1c1f2e5f65db70 100644 --- a/accounts-db/src/accounts_hash.rs +++ b/accounts-db/src/accounts_hash.rs @@ -1162,6 +1162,8 @@ impl<'a> AccountsHasher<'a> { overall_sum = Self::checked_cast_for_capitalization( item.lamports as u128 + overall_sum as u128, ); + // note that we DO have to dedup and avoid zero lamport hashes... + // todo: probably we could accumulate here instead of writing every hash here and accumulating each hash later (in a map/fold reduce) hashes.write(&item.hash.0); } else { // if lamports == 0, check if they should be included @@ -1170,6 +1172,7 @@ impl<'a> AccountsHasher<'a> { // the hash of its pubkey let hash = blake3::hash(bytemuck::bytes_of(&item.pubkey)); let hash = Hash::new_from_array(hash.into()); + // todo: same as above hashes.write(&hash); } } @@ -1204,6 +1207,16 @@ impl<'a> AccountsHasher<'a> { let _guard = self.active_stats.activate(ActiveStatItem::HashMerkleTree); let mut hash_time = Measure::start("hash"); + let mut accumulated = Hash::default(); + let mut i = 0; + while i < cumulative.total_count() { + let slice = cumulative.get_slice(i); + slice.iter().for_each(|hash| { + // todo: accumulate here if we weren't able to do it earlier + // accumulated += hash + }); + i += slice.len(); + } let (hash, _) = Self::compute_merkle_root_from_slices( cumulative.total_count(), MERKLE_FANOUT, From 649308ea3aa3d034081215535ac4a398ebc960aa Mon Sep 17 00:00:00 2001 From: jeff washington Date: Mon, 18 Dec 2023 17:48:31 -0600 Subject: [PATCH 3/6] add some comments --- runtime/src/bank.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 3fd704a525127f..3092f0890e5b43 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -7000,6 +7000,14 @@ impl Bank { self.skipped_rewrites.lock().unwrap().clone(), ); + // todo: note that this could be slow. My theory was we'd want to run this in the bg and have it ready later. + // It could be ready for the snapshot, accounts hash abs loop, publishing to gossip later, or even to include in the bank hash NEXT slot. + // having it ready THIS slot is a lot of work and adds loads into the dependency chain or makes every load more expensive (especially considering an account written multiple times in the same slot). + // We'd have to add infrastructure to keep hash values in the read only cache, for example. + // We could also build a HashMap)> which is populated the FIRST time a writable account is loaded in this slot. a bg thread could hash the contents. Then, we would have the hash of each written account prior to the first load. + // We could even leave the RwLock out of it and just re-hash inside `accumulate_accounts_hash`. Depends on how much time loading from prior slots, cloning the account, hashing the account, and looking up the hash (in hashmap) take relative to each other. + // I think we want to avoid passing the hash value (or some type of Arc> all around to everyone at every load just for this feature). We need (impl ReadableAccount) + pubkey in order to calculate a hash, so we need something like &AccountSharedData, which we'd like to avoid cloning just so we can hash it later. + // All of this concern can get mitigated if we just wait a single slot and do this hash in the bg. let mut accumulated = self.parent().map(|bank| bank.accumulated_accounts_hash.read().unwrap().unwrap_or_default()).unwrap_or_default(); // todo probably not default here self .rc From d0e50e61d2f784687500f7405b6fc662d2a79e86 Mon Sep 17 00:00:00 2001 From: jeff washington Date: Tue, 19 Dec 2023 17:31:30 -0600 Subject: [PATCH 4/6] use old_written_accounts to keep track of old accounts and their hash --- accounts-db/src/accounts_db.rs | 9 ++++++++- runtime/src/accounts/mod.rs | 12 ++++++++++++ runtime/src/bank.rs | 8 ++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index 67b5dcf2d31194..79d8178331403e 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -7926,13 +7926,20 @@ impl AccountsDb { mut ancestors: Ancestors, accumulated_accounts_hash: &mut Hash, pubkey_hash: Vec<(Pubkey, AccountHash)>, + old_written_accounts: &RwLock, Option)>> ) { // we are assuming it was easy to lookup a hash for everything written in `slot` when we were calculating the delta hash. So, caller passes in `pubkey_hash` // note we don't need rewrites in `pubkey_hash`. these accounts had the same hash before and after. So, we only have to consider what was written that changed. ancestors.remove(&slot); + let old_written_accounts = old_written_accounts.read().unwrap(); // if we want to look it up ourselves: let (hashes, _scan_us, _accumulate) = self.get_pubkey_hash_for_slot(slot); let old = pubkey_hash.iter().map(|(k, _)| { - self.load_with_fixed_root(&ancestors, k).map(|(account, _)| Self::hash_account(&account, k)) + if let Some((account, hash)) = old_written_accounts.get(k) { + Some(hash.unwrap()) // todo on demand calculate, calculate in bg + } + else { + self.load_with_fixed_root(&ancestors, k).map(|(account, _)| Self::hash_account(&account, k)) + } }).collect::>(); pubkey_hash.into_iter().zip(old.into_iter()).for_each(|((k, new_hash), old_hash)| { if let Some(old) = old_hash { diff --git a/runtime/src/accounts/mod.rs b/runtime/src/accounts/mod.rs index 249168de7a9a03..cb06f0cc854905 100644 --- a/runtime/src/accounts/mod.rs +++ b/runtime/src/accounts/mod.rs @@ -46,6 +46,8 @@ use { solana_system_program::{get_system_account_kind, SystemAccountKind}, std::{collections::HashMap, num::NonZeroUsize}, }; +use solana_accounts_db::accounts_hash::AccountHash; +use std::sync::RwLock; #[allow(clippy::too_many_arguments)] pub(super) fn load_accounts( @@ -63,6 +65,7 @@ pub(super) fn load_accounts( program_accounts: &HashMap, loaded_programs: &LoadedProgramsForTxBatch, should_collect_rent: bool, + old_written_accounts: &RwLock, Option)>>, ) -> Vec { txs.iter() .zip(lock_results) @@ -106,6 +109,7 @@ pub(super) fn load_accounts( program_accounts, loaded_programs, should_collect_rent, + old_written_accounts, ) { Ok(loaded_transaction) => loaded_transaction, Err(e) => return (Err(e), None), @@ -147,6 +151,7 @@ fn load_transaction_accounts( program_accounts: &HashMap, loaded_programs: &LoadedProgramsForTxBatch, should_collect_rent: bool, + old_written_accounts: &RwLock, Option)>>, ) -> Result { let in_reward_interval = reward_interval == RewardInterval::InsideInterval; @@ -215,6 +220,13 @@ fn load_transaction_accounts( .load_with_fixed_root(ancestors, key) .map(|(mut account, _)| { if message.is_writable(i) { + { + // todo: find all the places this has to happen + let mut old_written_accounts= old_written_accounts.write().unwrap(); + if !old_written_accounts.contains_key(key) { + old_written_accounts.insert(*key, (Some(account.clone()), None)); + } + } if should_collect_rent { let rent_due = rent_collector .collect_from_existing_account( diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 3092f0890e5b43..1bd2b97e0d5849 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -512,6 +512,7 @@ impl PartialEq for Bank { } let Self { accumulated_accounts_hash: _, + old_written_accounts: _, skipped_rewrites: _, rc: _, status_cache: _, @@ -835,6 +836,8 @@ pub struct Bank { pub check_program_modification_slot: bool, pub accumulated_accounts_hash: RwLock>, + /// the pubkey, (account, hash) pairs that were loaded in this slot in order to be written. Ideally, when the bank is done, this contains a calculated hash value for all accounts which are written in THIS slot. + pub old_written_accounts: RwLock, Option)>>, epoch_reward_status: EpochRewardStatus, } @@ -980,6 +983,7 @@ impl Bank { fn default_with_accounts(accounts: Accounts) -> Self { let mut bank = Self { accumulated_accounts_hash: RwLock::default(), + old_written_accounts: RwLock::default(), skipped_rewrites: Mutex::default(), incremental_snapshot_persistence: None, rc: BankRc::new(accounts, Slot::default()), @@ -1317,6 +1321,7 @@ impl Bank { let accounts_data_size_initial = parent.load_accounts_data_size(); let mut new = Self { accumulated_accounts_hash: RwLock::default(), + old_written_accounts: RwLock::default(), skipped_rewrites: Mutex::default(), incremental_snapshot_persistence: None, rc, @@ -1832,6 +1837,7 @@ impl Bank { ); let stakes_accounts_load_duration = now.elapsed(); let mut bank = Self { + old_written_accounts: RwLock::default(), accumulated_accounts_hash: RwLock::default(), skipped_rewrites: Mutex::default(), incremental_snapshot_persistence: fields.incremental_snapshot_persistence, @@ -5221,6 +5227,7 @@ impl Bank { &program_accounts_map, &programs_loaded_for_tx_batch.borrow(), self.should_collect_rent(), + &self.old_written_accounts, ); load_time.stop(); @@ -7018,6 +7025,7 @@ impl Bank { self.ancestors.clone(), &mut accumulated, pubkey_hash, + &self.old_written_accounts, ); *self.accumulated_accounts_hash.write().unwrap() = Some(accumulated); From 30334857a1da6de6db236bbbda81ccac65ff226b Mon Sep 17 00:00:00 2001 From: jeff washington Date: Tue, 19 Dec 2023 17:41:43 -0600 Subject: [PATCH 5/6] seed next slot with old_written_accounts_from_last_slot --- runtime/src/bank.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 1bd2b97e0d5849..c500a2b12cb6c2 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -512,6 +512,7 @@ impl PartialEq for Bank { } let Self { accumulated_accounts_hash: _, + old_written_accounts_from_last_slot: _, old_written_accounts: _, skipped_rewrites: _, rc: _, @@ -838,6 +839,8 @@ pub struct Bank { pub accumulated_accounts_hash: RwLock>, /// the pubkey, (account, hash) pairs that were loaded in this slot in order to be written. Ideally, when the bank is done, this contains a calculated hash value for all accounts which are written in THIS slot. pub old_written_accounts: RwLock, Option)>>, + /// the pubkey, hash pairs that were written by the last slot. MANY accounts are written EVERY slot. This avoids a re-hashing. + pub old_written_accounts_from_last_slot: RwLock, Option)>>, epoch_reward_status: EpochRewardStatus, } @@ -983,6 +986,7 @@ impl Bank { fn default_with_accounts(accounts: Accounts) -> Self { let mut bank = Self { accumulated_accounts_hash: RwLock::default(), + old_written_accounts_from_last_slot: RwLock::default(), old_written_accounts: RwLock::default(), skipped_rewrites: Mutex::default(), incremental_snapshot_persistence: None, @@ -1321,7 +1325,9 @@ impl Bank { let accounts_data_size_initial = parent.load_accounts_data_size(); let mut new = Self { accumulated_accounts_hash: RwLock::default(), - old_written_accounts: RwLock::default(), + // start this slot's old written accounts with the accounts written in the last slot. many accounts are written every slot (like votes) + old_written_accounts: RwLock::new(std::mem::take(&mut parent.old_written_accounts_from_last_slot.write().unwrap())), + old_written_accounts_from_last_slot: RwLock::default(), skipped_rewrites: Mutex::default(), incremental_snapshot_persistence: None, rc, @@ -1838,6 +1844,7 @@ impl Bank { let stakes_accounts_load_duration = now.elapsed(); let mut bank = Self { old_written_accounts: RwLock::default(), + old_written_accounts_from_last_slot: RwLock::default(), accumulated_accounts_hash: RwLock::default(), skipped_rewrites: Mutex::default(), incremental_snapshot_persistence: fields.incremental_snapshot_persistence, @@ -7015,6 +7022,17 @@ impl Bank { // We could even leave the RwLock out of it and just re-hash inside `accumulate_accounts_hash`. Depends on how much time loading from prior slots, cloning the account, hashing the account, and looking up the hash (in hashmap) take relative to each other. // I think we want to avoid passing the hash value (or some type of Arc> all around to everyone at every load just for this feature). We need (impl ReadableAccount) + pubkey in order to calculate a hash, so we need something like &AccountSharedData, which we'd like to avoid cloning just so we can hash it later. // All of this concern can get mitigated if we just wait a single slot and do this hash in the bg. + + + { + // every account that was written and hashed in this slot is likely going to be written again in the next slot. So, remember the pubkey and hash values as of the end of this slot. + // This will become the initial expected (pubkey, hash) pairs needed to subtract out these hashes from the accumulated hash next time. + let mut old_written_accounts_from_last_slot = self.old_written_accounts_from_last_slot.write().unwrap(); + pubkey_hash.iter().for_each(|(k, h)| { + old_written_accounts_from_last_slot.insert(*k, (None, Some(*h))); + }) + } + let mut accumulated = self.parent().map(|bank| bank.accumulated_accounts_hash.read().unwrap().unwrap_or_default()).unwrap_or_default(); // todo probably not default here self .rc From 02e253dcfc411630323f3adc5c75bc1f57fca4ec Mon Sep 17 00:00:00 2001 From: jeff washington Date: Tue, 19 Dec 2023 17:47:26 -0600 Subject: [PATCH 6/6] persistence --- runtime/src/bank.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index c500a2b12cb6c2..83e72562eec543 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -1845,6 +1845,7 @@ impl Bank { let mut bank = Self { old_written_accounts: RwLock::default(), old_written_accounts_from_last_slot: RwLock::default(), + // todo: this has to be saved and loaded in bank persistence somehow accumulated_accounts_hash: RwLock::default(), skipped_rewrites: Mutex::default(), incremental_snapshot_persistence: fields.incremental_snapshot_persistence,