Finer grained AccountsIndex locking (#12787)

Co-authored-by: Carl Lin <carl@solana.com> (cherry picked from commit e6b821c) # Conflicts: # runtime/src/accounts.rs # runtime/src/accounts_db.rs
solana-labs · Oct 28, 2020 · 42228bc · 42228bc
1 parent c1a55bf
commit 42228bc
Show file tree

Hide file tree

Showing 9 changed files with 898 additions and 442 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
@@ -29,6 +29,7 @@ memmap = "0.7.0"
 num-derive = { version = "0.3" }
 num-traits = { version = "0.2" }
 num_cpus = "1.13.0"
+ouroboros = "0.4.0"
 rand = "0.7.0"
 rayon = "1.4.1"
 regex = "1.3.9"

diff --git a/runtime/benches/accounts.rs b/runtime/benches/accounts.rs
@@ -148,14 +148,18 @@ fn bench_delete_dependencies(bencher: &mut Bencher) {
     });
 }
 
-#[bench]
-#[ignore]
-fn bench_concurrent_read_write(bencher: &mut Bencher) {
+fn store_accounts_with_possible_contention<F: 'static>(
+    bench_name: &str,
+    bencher: &mut Bencher,
+    reader_f: F,
+) where
+    F: Fn(&Accounts, &[Pubkey]) + Send + Copy,
+{
     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"),
+                .join(bench_name),
         ],
         &ClusterType::Development,
     ));
@@ -179,11 +183,7 @@ fn bench_concurrent_read_write(bencher: &mut Bencher) {
         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());
-                }
+                reader_f(&accounts, &pubkeys);
             })
             .unwrap();
     }
@@ -202,6 +202,30 @@ fn bench_concurrent_read_write(bencher: &mut Bencher) {
     })
 }
 
+#[bench]
+#[ignore]
+fn bench_concurrent_read_write(bencher: &mut Bencher) {
+    store_accounts_with_possible_contention(
+        "concurrent_read_write",
+        bencher,
+        |accounts, pubkeys| {
+            let mut rng = rand::thread_rng();
+            loop {
+                let i = rng.gen_range(0, pubkeys.len());
+                test::black_box(accounts.load_slow(&HashMap::new(), &pubkeys[i]).unwrap());
+            }
+        },
+    )
+}
+
+#[bench]
+#[ignore]
+fn bench_concurrent_scan_write(bencher: &mut Bencher) {
+    store_accounts_with_possible_contention("concurrent_scan_write", bencher, |accounts, _| loop {
+        test::black_box(accounts.load_by_program(&HashMap::new(), &Account::default().owner));
+    })
+}
+
 #[bench]
 #[ignore]
 fn bench_dashmap_single_reader_with_n_writers(bencher: &mut Bencher) {

diff --git a/runtime/benches/accounts_index.rs b/runtime/benches/accounts_index.rs
@@ -15,10 +15,10 @@ fn bench_accounts_index(bencher: &mut Bencher) {
     const NUM_FORKS: u64 = 16;
 
     let mut reclaims = vec![];
-    let mut index = AccountsIndex::<AccountInfo>::default();
+    let index = AccountsIndex::<AccountInfo>::default();
     for f in 0..NUM_FORKS {
         for pubkey in pubkeys.iter().take(NUM_PUBKEYS) {
-            index.insert(f, pubkey, AccountInfo::default(), &mut reclaims);
+            index.upsert(f, pubkey, AccountInfo::default(), &mut reclaims);
         }
     }
 
@@ -27,7 +27,7 @@ fn bench_accounts_index(bencher: &mut Bencher) {
     bencher.iter(|| {
         for _p in 0..NUM_PUBKEYS {
             let pubkey = thread_rng().gen_range(0, NUM_PUBKEYS);
-            index.insert(
+            index.upsert(
                 fork,
                 &pubkeys[pubkey],
                 AccountInfo::default(),

diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs
@@ -306,9 +306,7 @@ impl Accounts {
         rent_collector: &RentCollector,
         feature_set: &FeatureSet,
     ) -> Vec<(Result<TransactionLoadResult>, Option<HashAgeKind>)> {
-        //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 accounts_index = &self.accounts_db.accounts_index;
 
         let fee_config = FeeConfig {
             secp256k1_program_enabled: feature_set
@@ -335,7 +333,7 @@ impl Accounts {
                     let load_res = self.load_tx_accounts(
                         &self.accounts_db.storage,
                         ancestors,
-                        &accounts_index,
+                        accounts_index,
                         tx,
                         fee,
                         error_counters,
@@ -350,7 +348,7 @@ impl Accounts {
                     let load_res = Self::load_loaders(
                         &self.accounts_db.storage,
                         ancestors,
-                        &accounts_index,
+                        accounts_index,
                         tx,
                         error_counters,
                     );
@@ -1519,13 +1517,17 @@ mod tests {
         let mut error_counters = ErrorCounters::default();
         let ancestors = vec![(0, 0)].into_iter().collect();
 
-        let accounts_index = accounts.accounts_db.accounts_index.read().unwrap();
         assert_eq!(
             Accounts::load_executable_accounts(
                 &accounts.accounts_db.storage,
                 &ancestors,
+<<<<<<< HEAD
                 &accounts_index,
                 &solana_sdk::pubkey::new_rand(),
+=======
+                &accounts.accounts_db.accounts_index,
+                &Pubkey::new_rand(),
+>>>>>>> e6b821c39... Finer grained AccountsIndex locking (#12787)
                 &mut error_counters
             ),
             Err(TransactionError::ProgramAccountNotFound)