Skip to content

Commit

Permalink
Don't hold dashmap write lock in store create (#13007)
Browse files Browse the repository at this point in the history
Co-authored-by: Carl Lin <carl@solana.com>
  • Loading branch information
carllin and carllin authored Oct 21, 2020
1 parent 0776fa0 commit c8fc0a6
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 10 deletions.
58 changes: 57 additions & 1 deletion runtime/benches/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

extern crate test;

use dashmap::DashMap;
use rand::Rng;
use solana_runtime::{
accounts::{create_test_accounts, Accounts},
Expand All @@ -12,7 +13,12 @@ use solana_sdk::{
genesis_config::{create_genesis_config, ClusterType},
pubkey::Pubkey,
};
use std::{collections::HashMap, path::PathBuf, sync::Arc, thread::Builder};
use std::{
collections::HashMap,
path::PathBuf,
sync::{Arc, RwLock},
thread::Builder,
};
use test::Bencher;

fn deposit_many(bank: &Bank, pubkeys: &mut Vec<Pubkey>, num: usize) {
Expand Down Expand Up @@ -196,3 +202,53 @@ fn bench_concurrent_read_write(bencher: &mut Bencher) {
}
})
}

#[bench]
#[ignore]
fn bench_dashmap_single_reader_with_n_writers(bencher: &mut Bencher) {
let num_readers = 5;
let num_keys = 10000;
let map = Arc::new(DashMap::new());
for i in 0..num_keys {
map.insert(i, i);
}
for _ in 0..num_readers {
let map = map.clone();
Builder::new()
.name("readers".to_string())
.spawn(move || loop {
test::black_box(map.entry(5).or_insert(2));
})
.unwrap();
}
bencher.iter(|| {
for _ in 0..num_keys {
test::black_box(map.get(&5).unwrap().value());
}
})
}

#[bench]
#[ignore]
fn bench_rwlock_hashmap_single_reader_with_n_writers(bencher: &mut Bencher) {
let num_readers = 5;
let num_keys = 10000;
let map = Arc::new(RwLock::new(HashMap::new()));
for i in 0..num_keys {
map.write().unwrap().insert(i, i);
}
for _ in 0..num_readers {
let map = map.clone();
Builder::new()
.name("readers".to_string())
.spawn(move || loop {
test::black_box(map.write().unwrap().get(&5));
})
.unwrap();
}
bencher.iter(|| {
for _ in 0..num_keys {
test::black_box(map.read().unwrap().get(&5));
}
})
}
24 changes: 15 additions & 9 deletions runtime/src/accounts_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,8 @@ impl AccountStorage {
slot: Slot,
store_id: AppendVecId,
) -> Option<Arc<AccountStorageEntry>> {
self.0
.get(&slot)
.and_then(|storage_map| storage_map.value().read().unwrap().get(&store_id).cloned())
self.get_slot_stores(slot)
.and_then(|storage_map| storage_map.read().unwrap().get(&store_id).cloned())
}

fn get_slot_stores(&self, slot: Slot) -> Option<SlotStores> {
Expand Down Expand Up @@ -1410,12 +1409,19 @@ impl AccountsDB {
let store =
Arc::new(self.new_storage_entry(slot, &Path::new(&self.paths[path_index]), size));
let store_for_index = store.clone();
let slot_storage = self
.storage
.0
.entry(slot)
.or_insert(Arc::new(RwLock::new(HashMap::new())));
slot_storage

let slot_storages: SlotStores = self.storage.get_slot_stores(slot).unwrap_or_else(||
// DashMap entry.or_insert() returns a RefMut, essentially a write lock,
// which is dropped after this block ends, minimizing time held by the lock.
// However, we still want to persist the reference to the `SlotStores` behind
// the lock, hence we clone it out, (`SlotStores` is an Arc so is cheap to clone).
self.storage
.0
.entry(slot)
.or_insert(Arc::new(RwLock::new(HashMap::new())))
.clone());

slot_storages
.write()
.unwrap()
.insert(store.id, store_for_index);
Expand Down

0 comments on commit c8fc0a6

Please sign in to comment.