optimize store_accounts (#5557)

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"

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"

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();
+        self.accounts_db.store(fork, &[(pubkey, account)]);
-        accounts.insert(pubkey, account);
-        self.accounts_db.store(fork, &accounts);
     }
 
     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);
+        let accounts_to_store = self.collect_accounts_to_store(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);
-            }
-        }
         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)> {
+    ) -> Vec<(&'a Pubkey, &'a Account)> {
-        let mut accounts: HashMap<&Pubkey, (&Account, bool)> = HashMap::new();
+        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) {
+                if message.is_debitable(i) {
-                    accounts.insert(key, (account, 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
-                        self.credit_only_account_locks
+                        .read()
-                            .read()
+                        .unwrap()
-                            .unwrap()
+                        .as_ref()
-                            .as_ref()
+                        .expect("Collect accounts should only be called before a commit, and credit only account locks should exist before a commit")
-                            .expect("Collect accounts should only be called before a commit, and credit only account locks should exist before a commit")
+                        .get(key)
-                            .get(key)
+                        .unwrap()
-                            .unwrap()
+                        .credits
-                            .credits
+                        .fetch_add(*credit, Ordering::Relaxed);
-                            .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);
+        let collected_accounts = accounts.collect_accounts_to_store(&txs, &loaders, &mut loaded);
-        assert_eq!(collected_accounts.len(), 3);
+        assert_eq!(collected_accounts.len(), 2);
-        assert!(collected_accounts.contains_key(&keypair0.pubkey()));
+        assert!(collected_accounts
-        assert!(collected_accounts.contains_key(&keypair1.pubkey()));
+            .iter()
-        assert!(collected_accounts.contains_key(&pubkey));
+            .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();

runtime/src/accounts_db.rs

@@ -701,11 +701,7 @@ impl AccountsDB {
         }
     }
 
-    fn store_accounts(
+    fn store_accounts(&self, fork_id: Fork, accounts: &[(&Pubkey, &Account)]) -> Vec<AccountInfo> {
-        &self,
-        fork_id: Fork,
-        accounts: &HashMap<&Pubkey, &Account>,
-    ) -> Vec<AccountInfo> {
         let with_meta: Vec<(StorageMeta, &Account)> = accounts
             .iter()
             .map(|(pubkey, account)| {
@@ -756,23 +752,23 @@ impl AccountsDB {
         &self,
         fork_id: Fork,
         infos: Vec<AccountInfo>,
-        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()) {
+        for (info, pubkey_account) in infos.into_iter().zip(accounts.iter()) {
-            let key = &account.0;
+            let pubkey = pubkey_account.0;
-            if let Some(info) = index.update(fork_id, key, info, &mut reclaims) {
+            if let Some(info) = index.update(fork_id, pubkey, info, &mut reclaims) {
-                inserts.push((account, info));
+                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, &[(&pubkey, &account)]);
-        db.store(1, &hashmap!(&pubkeys[0] => &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<Account> =
@@ -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();