Switch accounts storage lock to DashMap (#12126)

Co-authored-by: Carl Lin <[email protected]>
solana-labs · Oct 14, 2020 · f8d338c · f8d338c
1 parent 1f1eb9f
commit f8d338c
Show file tree

Hide file tree

Showing 7 changed files with 416 additions and 241 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/programs/bpf/Cargo.lock b/programs/bpf/Cargo.lock
diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml
@@ -14,6 +14,7 @@ blake3 = "0.3.6"
 bv = { version = "0.11.1", features = ["serde"] }
 byteorder = "1.3.4"
 bzip2 = "0.3.3"
+dashmap = "3.11.10"
 crossbeam-channel = "0.4"
 dir-diff = "0.3.2"
 flate2 = "1.0.14"

diff --git a/runtime/benches/accounts.rs b/runtime/benches/accounts.rs
@@ -2,6 +2,7 @@
 
 extern crate test;
 
+use rand::Rng;
 use solana_runtime::{
     accounts::{create_test_accounts, Accounts},
     bank::*,
@@ -11,7 +12,7 @@ use solana_sdk::{
     genesis_config::{create_genesis_config, ClusterType},
     pubkey::Pubkey,
 };
-use std::{path::PathBuf, sync::Arc};
+use std::{collections::HashMap, path::PathBuf, sync::Arc, thread::Builder};
 use test::Bencher;
 
 fn deposit_many(bank: &Bank, pubkeys: &mut Vec<Pubkey>, num: usize) {
@@ -141,3 +142,57 @@ fn bench_delete_dependencies(bencher: &mut Bencher) {
         accounts.accounts_db.clean_accounts(None);
     });
 }
+
+#[bench]
+#[ignore]
+fn bench_concurrent_read_write(bencher: &mut Bencher) {
+    let num_readers = 5;
+    let accounts = Arc::new(Accounts::new(
+        vec![
+            PathBuf::from(std::env::var("FARF_DIR").unwrap_or_else(|_| "farf".to_string()))
+                .join("concurrent_read_write"),
+        ],
+        &ClusterType::Development,
+    ));
+    let num_keys = 1000;
+    let slot = 0;
+    accounts.add_root(slot);
+    let pubkeys: Arc<Vec<_>> = Arc::new(
+        (0..num_keys)
+            .map(|_| {
+                let pubkey = Pubkey::new_rand();
+                let account = Account::new(1, 0, &Account::default().owner);
+                accounts.store_slow(slot, &pubkey, &account);
+                pubkey
+            })
+            .collect(),
+    );
+
+    for _ in 0..num_readers {
+        let accounts = accounts.clone();
+        let pubkeys = pubkeys.clone();
+        Builder::new()
+            .name("readers".to_string())
+            .spawn(move || {
+                let mut rng = rand::thread_rng();
+                loop {
+                    let i = rng.gen_range(0, num_keys);
+                    test::black_box(accounts.load_slow(&HashMap::new(), &pubkeys[i]).unwrap());
+                }
+            })
+            .unwrap();
+    }
+
+    let num_new_keys = 1000;
+    let new_accounts: Vec<_> = (0..num_new_keys)
+        .map(|_| Account::new(1, 0, &Account::default().owner))
+        .collect();
+    bencher.iter(|| {
+        for account in &new_accounts {
+            // Write to a different slot than the one being read from. Because
+            // there's a new account pubkey being written to every time, will
+            // compete for the accounts index lock on every store
+            accounts.store_slow(slot + 1, &Pubkey::new_rand(), &account);
+        }
+    })
+}
diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs
@@ -303,7 +303,6 @@ impl Accounts {
         //PERF: hold the lock to scan for the references, but not to clone the accounts
         //TODO: two locks usually leads to deadlocks, should this be one structure?
         let accounts_index = self.accounts_db.accounts_index.read().unwrap();
-        let storage = self.accounts_db.storage.read().unwrap();
 
         let fee_config = FeeConfig {
             secp256k1_program_enabled: feature_set
@@ -328,7 +327,7 @@ impl Accounts {
                     };
 
                     let load_res = self.load_tx_accounts(
-                        &storage,
+                        &self.accounts_db.storage,
                         ancestors,
                         &accounts_index,
                         tx,
@@ -343,7 +342,7 @@ impl Accounts {
                     };
 
                     let load_res = Self::load_loaders(
-                        &storage,
+                        &self.accounts_db.storage,
                         ancestors,
                         &accounts_index,
                         tx,
@@ -1507,10 +1506,9 @@ mod tests {
         let ancestors = vec![(0, 0)].into_iter().collect();
 
         let accounts_index = accounts.accounts_db.accounts_index.read().unwrap();
-        let storage = accounts.accounts_db.storage.read().unwrap();
         assert_eq!(
             Accounts::load_executable_accounts(
-                &storage,
+                &accounts.accounts_db.storage,
                 &ancestors,
                 &accounts_index,
                 &Pubkey::new_rand(),