From 3152090a6628a8caa6feab70ff897516d3bfee65 Mon Sep 17 00:00:00 2001
From: Sathish Ambley <sathish@solana.com>
Date: Sun, 31 Mar 2019 21:31:19 -0700
Subject: [PATCH] update with review comments

---
 runtime/benches/accounts.rs | 51 +++++++++++++------------------------
 runtime/src/accounts.rs     | 19 +++-----------
 2 files changed, 22 insertions(+), 48 deletions(-)

diff --git a/runtime/benches/accounts.rs b/runtime/benches/accounts.rs
index 7efb0409c1..ee3bb62ad3 100644
--- a/runtime/benches/accounts.rs
+++ b/runtime/benches/accounts.rs
@@ -6,33 +6,17 @@ use solana_runtime::bank::*;
 use solana_sdk::account::Account;
 use solana_sdk::genesis_block::GenesisBlock;
 use solana_sdk::pubkey::Pubkey;
-use solana_sdk::signature::{Keypair, KeypairUtil};
 use std::sync::Arc;
 use test::Bencher;
 
-fn compare_account(account1: &Account, account2: &Account) -> bool {
-    if account1.data != account2.data
-        || account1.owner != account2.owner
-        || account1.executable != account2.executable
-        || account1.lamports != account2.lamports
-    {
-        return false;
-    }
-    true
-}
-
-fn create_account(bank: &Bank, pubkeys: &mut Vec<Pubkey>, num: usize) {
+fn deposit_many(bank: &Bank, pubkeys: &mut Vec<Pubkey>, num: usize) {
     for t in 0..num {
-        let pubkey = Keypair::new().pubkey();
-        let mut default_account = Account::default();
+        let pubkey = Pubkey::new_rand();
+        let account = Account::new((t + 1) as u64, 0, &Account::default().owner);
         pubkeys.push(pubkey.clone());
-        default_account.lamports = (t + 1) as u64;
         assert!(bank.get_account(&pubkey).is_none());
         bank.deposit(&pubkey, (t + 1) as u64);
-        assert_eq!(
-            compare_account(&bank.get_account(&pubkey).unwrap(), &default_account),
-            true
-        );
+        assert_eq!(bank.get_account(&pubkey).unwrap(), account);
     }
 }
 
@@ -42,32 +26,33 @@ fn test_accounts_create(bencher: &mut Bencher) {
     let bank0 = Bank::new_with_paths(&genesis_block, Some("bench_a0".to_string()));
     bencher.iter(|| {
         let mut pubkeys: Vec<Pubkey> = vec![];
-        create_account(&bank0, &mut pubkeys, 1000);
+        deposit_many(&bank0, &mut pubkeys, 1000);
     });
 }
 
 #[bench]
 fn test_accounts_squash(bencher: &mut Bencher) {
     let (genesis_block, _) = GenesisBlock::new(100_000);
-    let mut banks: Vec<Arc<Bank>> = Vec::with_capacity(50);
+    let mut banks: Vec<Arc<Bank>> = Vec::with_capacity(10);
     banks.push(Arc::new(Bank::new_with_paths(
         &genesis_block,
         Some("bench_a1".to_string()),
     )));
     let mut pubkeys: Vec<Pubkey> = vec![];
-    create_account(&banks[0], &mut pubkeys, 250000);
+    deposit_many(&banks[0], &mut pubkeys, 250000);
     banks[0].freeze();
+    // Measures the performance of the squash operation merging the accounts
+    // with the majority of the accounts present in the parent bank that is
+    // moved over to this bank.
     bencher.iter(|| {
-        for index in 1..10 {
-            banks.push(Arc::new(Bank::new_from_parent(
-                &banks[index - 1],
-                &Pubkey::default(),
-                index as u64,
-            )));
-            for accounts in 0..10000 {
-                banks[index].deposit(&pubkeys[accounts], (accounts + 1) as u64);
-            }
-            banks[index].squash();
+        banks.push(Arc::new(Bank::new_from_parent(
+            &banks[0],
+            &Pubkey::default(),
+            1u64,
+        )));
+        for accounts in 0..10000 {
+            banks[1].deposit(&pubkeys[accounts], (accounts + 1) as u64);
         }
+        banks[1].squash();
     });
 }
diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs
index 0b13d69ae2..c7224deee3 100644
--- a/runtime/src/accounts.rs
+++ b/runtime/src/accounts.rs
@@ -1532,7 +1532,7 @@ mod tests {
             let account = db.load(0, &pubkeys[idx], true).unwrap();
             let mut default_account = Account::default();
             default_account.lamports = (idx + 1) as u64;
-            assert_eq!(compare_account(&default_account, &account), true);
+            assert_eq!(default_account, account);
         }
         db.add_fork(1, Some(0));
 
@@ -1629,23 +1629,12 @@ mod tests {
                 } else {
                     let mut default_account = Account::default();
                     default_account.lamports = account.lamports;
-                    assert_eq!(compare_account(&default_account, &account), true);
+                    assert_eq!(default_account, account);
                 }
             }
         }
     }
 
-    fn compare_account(account1: &Account, account2: &Account) -> bool {
-        if account1.data != account2.data
-            || account1.owner != account2.owner
-            || account1.executable != account2.executable
-            || account1.lamports != account2.lamports
-        {
-            return false;
-        }
-        true
-    }
-
     fn check_storage(accounts: &AccountsDB, count: usize) -> bool {
         let stores = accounts.storage.read().unwrap();
         assert_eq!(stores.len(), 1);
@@ -1662,7 +1651,7 @@ mod tests {
             let account = accounts.load(fork, &pubkeys[idx], true).unwrap();
             let mut default_account = Account::default();
             default_account.lamports = (idx + 1) as u64;
-            assert_eq!(compare_account(&default_account, &account), true);
+            assert_eq!(default_account, account);
         }
     }
 
@@ -1682,7 +1671,7 @@ mod tests {
         let account = accounts.load(0, &pubkeys[0], true).unwrap();
         let mut default_account = Account::default();
         default_account.lamports = 1;
-        assert_eq!(compare_account(&default_account, &account), true);
+        assert_eq!(default_account, account);
     }
 
     #[test]