diff --git a/Cargo.lock b/Cargo.lock index 53433b8a1..d61372835 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1725,11 +1725,6 @@ name = "mach_o_sys" version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" -[[package]] -name = "maplit" -version = "1.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" - [[package]] name = "matches" version = "0.1.8" @@ -3662,7 +3657,6 @@ dependencies = [ "libc 0.2.62 (registry+https://github.com/rust-lang/crates.io-index)", "libloading 0.5.2 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", - "maplit 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)", "memmap 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", "rand 0.6.5 (registry+https://github.com/rust-lang/crates.io-index)", "rayon 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -5424,7 +5418,6 @@ dependencies = [ "checksum log 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)" = "e19e8d5c34a3e0e2223db8e060f9e8264aeeb5c5fc64a4ee9965c062211c024b" "checksum log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)" = "14b6052be84e6b71ab17edffc2eeabf5c2c3ae1fdb464aae35ac50c67a44e1f7" "checksum mach_o_sys 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "3e854583a83f20cf329bb9283366335387f7db59d640d1412167e05fedb98826" -"checksum maplit 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "08cbb6b4fef96b6d77bfc40ec491b1690c779e77b05cd9f07f787ed376fd4c43" "checksum matches 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)" = "7ffc5c5338469d4d3ea17d269fa8ea3512ad247247c30bd2df69e68309ed0a08" "checksum memchr 0.1.11 (registry+https://github.com/rust-lang/crates.io-index)" = "d8b629fb514376c675b98c1421e80b151d3817ac42d7c667717d282761418d20" "checksum memchr 2.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "88579771288728879b57485cc7d6b07d648c9f0141eb955f8ab7f9d45394468e" diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index 32c25598b..d5eb9d778 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -18,7 +18,6 @@ lazy_static = "1.3.0" libc = "0.2.62" libloading = "0.5.2" log = "0.4.8" -maplit = "1.0.1" memmap = "0.6.2" rand = "0.6.5" rayon = "1.1.0" diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 18f1df587..6986c543c 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -315,9 +315,7 @@ impl Accounts { /// Slow because lock is held for 1 operation instead of many pub fn store_slow(&self, fork: Fork, pubkey: &Pubkey, account: &Account) { - let mut accounts = HashMap::new(); - accounts.insert(pubkey, account); - self.accounts_db.store(fork, &accounts); + self.accounts_db.store(fork, &[(pubkey, account)]); } fn get_read_access_credit_only<'a>( @@ -507,15 +505,7 @@ impl Accounts { res: &[Result<()>], loaded: &mut [Result<(InstructionAccounts, InstructionLoaders, InstructionCredits)>], ) { - let accounts = self.collect_accounts(txs, res, loaded); - // Only store credit-debit accounts immediately - let mut accounts_to_store: HashMap<&Pubkey, &Account> = HashMap::new(); - - for (pubkey, (account, is_debitable)) in accounts.iter() { - if *is_debitable { - accounts_to_store.insert(pubkey, account); - } - } + let accounts_to_store = self.collect_accounts_to_store(txs, res, loaded); self.accounts_db.store(fork, &accounts_to_store); } @@ -584,13 +574,13 @@ impl Accounts { } } - fn collect_accounts<'a>( + fn collect_accounts_to_store<'a>( &self, txs: &'a [Transaction], res: &'a [Result<()>], loaded: &'a mut [Result<(InstructionAccounts, InstructionLoaders, InstructionCredits)>], - ) -> HashMap<&'a Pubkey, (&'a Account, bool)> { - let mut accounts: HashMap<&Pubkey, (&Account, bool)> = HashMap::new(); + ) -> Vec<(&'a Pubkey, &'a Account)> { + let mut accounts = Vec::new(); for (i, raccs) in loaded.iter_mut().enumerate() { if res[i].is_err() || raccs.is_err() { continue; @@ -605,22 +595,20 @@ impl Accounts { .zip(acc.0.iter()) .zip(acc.2.iter()) { - if !accounts.contains_key(key) { - accounts.insert(key, (account, message.is_debitable(i))); + if message.is_debitable(i) { + accounts.push((key, account)); } if *credit > 0 { // Increment credit-only account balance Atomic - if accounts.get_mut(key).is_some() { - self.credit_only_account_locks - .read() - .unwrap() - .as_ref() - .expect("Collect accounts should only be called before a commit, and credit only account locks should exist before a commit") - .get(key) - .unwrap() - .credits - .fetch_add(*credit, Ordering::Relaxed); - } + self.credit_only_account_locks + .read() + .unwrap() + .as_ref() + .expect("Collect accounts should only be called before a commit, and credit only account locks should exist before a commit") + .get(key) + .unwrap() + .credits + .fetch_add(*credit, Ordering::Relaxed); } } } @@ -1453,7 +1441,7 @@ mod tests { } #[test] - fn test_collect_accounts() { + fn test_collect_accounts_to_store() { let keypair0 = Keypair::new(); let keypair1 = Keypair::new(); let pubkey = Pubkey::new_rand(); @@ -1519,19 +1507,17 @@ mod tests { }, ); } - let collected_accounts = accounts.collect_accounts(&txs, &loaders, &mut loaded); - assert_eq!(collected_accounts.len(), 3); - assert!(collected_accounts.contains_key(&keypair0.pubkey())); - assert!(collected_accounts.contains_key(&keypair1.pubkey())); - assert!(collected_accounts.contains_key(&pubkey)); + let collected_accounts = accounts.collect_accounts_to_store(&txs, &loaders, &mut loaded); + assert_eq!(collected_accounts.len(), 2); + assert!(collected_accounts + .iter() + .find(|(pubkey, _account)| *pubkey == &keypair0.pubkey()) + .is_some()); + assert!(collected_accounts + .iter() + .find(|(pubkey, _account)| *pubkey == &keypair1.pubkey()) + .is_some()); - let credit_debit_account0 = collected_accounts.get(&keypair0.pubkey()).unwrap(); - assert_eq!(credit_debit_account0.1, true); - let credit_debit_account1 = collected_accounts.get(&keypair1.pubkey()).unwrap(); - assert_eq!(credit_debit_account1.1, true); - - let credit_only_account = collected_accounts.get(&pubkey).unwrap(); - assert_eq!(credit_only_account.1, false); // Ensure credit_only_lock reflects credits from both accounts: 2 + 3 = 5 let credit_only_locks = accounts.credit_only_account_locks.read().unwrap(); let credit_only_locks = credit_only_locks.as_ref().unwrap(); diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index c3d7600e6..c771010b2 100644 --- a/runtime/src/accounts_db.rs +++ b/runtime/src/accounts_db.rs @@ -701,11 +701,7 @@ impl AccountsDB { } } - fn store_accounts( - &self, - fork_id: Fork, - accounts: &HashMap<&Pubkey, &Account>, - ) -> Vec { + fn store_accounts(&self, fork_id: Fork, accounts: &[(&Pubkey, &Account)]) -> Vec { let with_meta: Vec<(StorageMeta, &Account)> = accounts .iter() .map(|(pubkey, account)| { @@ -756,23 +752,23 @@ impl AccountsDB { &self, fork_id: Fork, infos: Vec, - accounts: &HashMap<&Pubkey, &Account>, + accounts: &[(&Pubkey, &Account)], ) -> (Vec<(Fork, AccountInfo)>, u64) { let mut reclaims: Vec<(Fork, AccountInfo)> = Vec::with_capacity(infos.len() * 2); let mut inserts = vec![]; let index = self.accounts_index.read().unwrap(); let mut update_index_work = Measure::start("update_index_work"); - for (info, account) in infos.into_iter().zip(accounts.iter()) { - let key = &account.0; - if let Some(info) = index.update(fork_id, key, info, &mut reclaims) { - inserts.push((account, info)); + for (info, pubkey_account) in infos.into_iter().zip(accounts.iter()) { + let pubkey = pubkey_account.0; + if let Some(info) = index.update(fork_id, pubkey, info, &mut reclaims) { + inserts.push((pubkey, info)); } } let last_root = index.last_root; drop(index); if !inserts.is_empty() { let mut index = self.accounts_index.write().unwrap(); - for ((pubkey, _account), info) in inserts { + for (pubkey, info) in inserts { index.insert(fork_id, pubkey, info, &mut reclaims); } } @@ -824,7 +820,7 @@ impl AccountsDB { } /// Store the account update. - pub fn store(&self, fork_id: Fork, accounts: &HashMap<&Pubkey, &Account>) { + pub fn store(&self, fork_id: Fork, accounts: &[(&Pubkey, &Account)]) { let mut store_accounts = Measure::start("store::store_accounts"); let infos = self.store_accounts(fork_id, accounts); store_accounts.stop(); @@ -927,7 +923,6 @@ pub mod tests { // TODO: all the bank tests are bank specific, issue: 2194 use super::*; use bincode::serialize_into; - use maplit::hashmap; use rand::{thread_rng, Rng}; use solana_sdk::account::Account; use std::fs; @@ -940,7 +935,7 @@ pub mod tests { let key = Pubkey::default(); let account0 = Account::new(1, 0, &key); - db.store(0, &hashmap!(&key => &account0)); + db.store(0, &[(&key, &account0)]); db.add_root(0); let ancestors = vec![(1, 1)].into_iter().collect(); assert_eq!(db.load_slow(&ancestors, &key), Some((account0, 0))); @@ -953,10 +948,10 @@ pub mod tests { let key = Pubkey::default(); let account0 = Account::new(1, 0, &key); - db.store(0, &hashmap!(&key => &account0)); + db.store(0, &[(&key, &account0)]); let account1 = Account::new(0, 0, &key); - db.store(1, &hashmap!(&key => &account1)); + db.store(1, &[(&key, &account1)]); let ancestors = vec![(1, 1)].into_iter().collect(); assert_eq!(&db.load_slow(&ancestors, &key).unwrap().0, &account1); @@ -980,10 +975,10 @@ pub mod tests { let key = Pubkey::default(); let account0 = Account::new(1, 0, &key); - db.store(0, &hashmap!(&key => &account0)); + db.store(0, &[(&key, &account0)]); let account1 = Account::new(0, 0, &key); - db.store(1, &hashmap!(&key => &account1)); + db.store(1, &[(&key, &account1)]); db.add_root(0); let ancestors = vec![(1, 1)].into_iter().collect(); @@ -1002,7 +997,7 @@ pub mod tests { let account0 = Account::new(1, 0, &key); // store value 1 in the "root", i.e. db zero - db.store(0, &hashmap!(&key => &account0)); + db.store(0, &[(&key, &account0)]); // now we have: // @@ -1015,7 +1010,7 @@ pub mod tests { // store value 0 in one child let account1 = Account::new(0, 0, &key); - db.store(1, &hashmap!(&key => &account1)); + db.store(1, &[(&key, &account1)]); // masking accounts is done at the Accounts level, at accountsDB we see // original account (but could also accept "None", which is implemented @@ -1077,8 +1072,8 @@ pub mod tests { let pubkey = Pubkey::new_rand(); let account = Account::new(1, DEFAULT_FILE_SIZE as usize / 3, &pubkey); - db.store(1, &hashmap!(&pubkey => &account)); - db.store(1, &hashmap!(&pubkeys[0] => &account)); + db.store(1, &[(&pubkey, &account)]); + db.store(1, &[(&pubkeys[0], &account)]); { let stores = db.storage.read().unwrap(); let fork_0_stores = &stores.0.get(&0).unwrap(); @@ -1107,11 +1102,11 @@ pub mod tests { // 1 token in the "root", i.e. db zero let db0 = AccountsDB::new(None); let account0 = Account::new(1, 0, &key); - db0.store(0, &hashmap!(&key => &account0)); + db0.store(0, &[(&key, &account0)]); // 0 lamports in the child let account1 = Account::new(0, 0, &key); - db0.store(1, &hashmap!(&key => &account1)); + db0.store(1, &[(&key, &account1)]); // masking accounts is done at the Accounts level, at accountsDB we see // original account @@ -1135,7 +1130,7 @@ pub mod tests { let account = Account::new((t + 1) as u64, space, &Account::default().owner); pubkeys.push(pubkey.clone()); assert!(accounts.load_slow(&ancestors, &pubkey).is_none()); - accounts.store(fork, &hashmap!(&pubkey => &account)); + accounts.store(fork, &[(&pubkey, &account)]); } for t in 0..num_vote { let pubkey = Pubkey::new_rand(); @@ -1143,7 +1138,7 @@ pub mod tests { pubkeys.push(pubkey.clone()); let ancestors = vec![(fork, 0)].into_iter().collect(); assert!(accounts.load_slow(&ancestors, &pubkey).is_none()); - accounts.store(fork, &hashmap!(&pubkey => &account)); + accounts.store(fork, &[(&pubkey, &account)]); } } @@ -1153,7 +1148,7 @@ pub mod tests { let ancestors = vec![(fork, 0)].into_iter().collect(); if let Some((mut account, _)) = accounts.load_slow(&ancestors, &pubkeys[idx]) { account.lamports = account.lamports + 1; - accounts.store(fork, &hashmap!(&pubkeys[idx] => &account)); + accounts.store(fork, &[(&pubkeys[idx], &account)]); if account.lamports == 0 { let ancestors = vec![(fork, 0)].into_iter().collect(); assert!(accounts.load_slow(&ancestors, &pubkeys[idx]).is_none()); @@ -1207,7 +1202,7 @@ pub mod tests { ) { for idx in 0..num { let account = Account::new((idx + count) as u64, 0, &Account::default().owner); - accounts.store(fork, &hashmap!(&pubkeys[idx] => &account)); + accounts.store(fork, &[(&pubkeys[idx], &account)]); } } @@ -1251,7 +1246,7 @@ pub mod tests { for i in 0..9 { let key = Pubkey::new_rand(); let account = Account::new(i + 1, size as usize / 4, &key); - accounts.store(0, &hashmap!(&key => &account)); + accounts.store(0, &[(&key, &account)]); keys.push(key); } for (i, key) in keys.iter().enumerate() { @@ -1286,7 +1281,7 @@ pub mod tests { let status = [AccountStorageStatus::Available, AccountStorageStatus::Full]; let pubkey1 = Pubkey::new_rand(); let account1 = Account::new(1, DEFAULT_FILE_SIZE as usize / 2, &pubkey1); - accounts.store(0, &hashmap!(&pubkey1 => &account1)); + accounts.store(0, &[(&pubkey1, &account1)]); { let stores = accounts.storage.read().unwrap(); assert_eq!(stores.0.len(), 1); @@ -1296,7 +1291,7 @@ pub mod tests { let pubkey2 = Pubkey::new_rand(); let account2 = Account::new(1, DEFAULT_FILE_SIZE as usize / 2, &pubkey2); - accounts.store(0, &hashmap!(&pubkey2 => &account2)); + accounts.store(0, &[(&pubkey2, &account2)]); { let stores = accounts.storage.read().unwrap(); assert_eq!(stores.0.len(), 1); @@ -1319,7 +1314,7 @@ pub mod tests { // lots of stores, but 3 storages should be enough for everything for i in 0..25 { let index = i % 2; - accounts.store(0, &hashmap!(&pubkey1 => &account1)); + accounts.store(0, &[(&pubkey1, &account1)]); { let stores = accounts.storage.read().unwrap(); assert_eq!(stores.0.len(), 1); @@ -1374,7 +1369,7 @@ pub mod tests { let pubkey = Pubkey::new_rand(); let account = Account::new(1, 0, &Account::default().owner); //store an account - accounts.store(0, &hashmap!(&pubkey => &account)); + accounts.store(0, &[(&pubkey, &account)]); let ancestors = vec![(0, 0)].into_iter().collect(); let id = { let index = accounts.accounts_index.read().unwrap(); @@ -1389,7 +1384,7 @@ pub mod tests { assert!(accounts.storage.read().unwrap().0[&0].get(&id).is_some()); //store causes cleanup - accounts.store(1, &hashmap!(&pubkey => &account)); + accounts.store(1, &[(&pubkey, &account)]); //fork is gone assert!(accounts.storage.read().unwrap().0.get(&0).is_none()); @@ -1470,7 +1465,7 @@ pub mod tests { loop { let account_bal = thread_rng().gen_range(1, 99); account.lamports = account_bal; - db.store(fork_id, &hashmap!(&pubkey => &account)); + db.store(fork_id, &[(&pubkey, &account)]); let (account, fork) = db.load_slow(&HashMap::new(), &pubkey).expect( &format!("Could not fetch stored account {}, iter {}", pubkey, i), ); @@ -1496,11 +1491,11 @@ pub mod tests { let key0 = Pubkey::new_rand(); let account0 = Account::new(1, 0, &key); - db.store(0, &hashmap!(&key0 => &account0)); + db.store(0, &[(&key0, &account0)]); let key1 = Pubkey::new_rand(); let account1 = Account::new(2, 0, &key); - db.store(1, &hashmap!(&key1 => &account1)); + db.store(1, &[(&key1, &account1)]); let ancestors = vec![(0, 0)].into_iter().collect(); let accounts: Vec = @@ -1530,7 +1525,7 @@ pub mod tests { let data_len = DEFAULT_FILE_SIZE as usize + 7; let account = Account::new(1, data_len, &key); - db.store(0, &hashmap!(&key => &account)); + db.store(0, &[(&key, &account)]); let ancestors = vec![(0, 0)].into_iter().collect(); let ret = db.load_slow(&ancestors, &key).unwrap();