From 595017499e9a967b4102992462f9f0d5a72d7aae Mon Sep 17 00:00:00 2001 From: sakridge Date: Sat, 20 Jul 2019 17:58:39 -0700 Subject: [PATCH] accounts_index: RwLock per-account (#5198) * accounts_index: RwLock per-account Lots of lock contention on the accounts_index lock, only take write-lock on accounts_index if we need to insert/remove an account. For updates, take a read-lock and then write-lock on the individual account. * Remove unneeded enumerate and add comments. --- runtime/src/accounts_db.rs | 38 ++++++---- runtime/src/accounts_index.rs | 127 ++++++++++++++++++++++------------ 2 files changed, 107 insertions(+), 58 deletions(-) diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index c6eb2865c..9f82c2000 100644 --- a/runtime/src/accounts_db.rs +++ b/runtime/src/accounts_db.rs @@ -461,9 +461,11 @@ impl AccountsDB { accounts_index: &AccountsIndex, pubkey: &Pubkey, ) -> Option<(Account, Fork)> { - let (info, fork) = accounts_index.get(pubkey, ancestors)?; + let (lock, index) = accounts_index.get(pubkey, ancestors)?; + let fork = lock[index].0; //TODO: thread this as a ref if let Some(fork_storage) = storage.0.get(&fork) { + let info = &lock[index].1; fork_storage .get(&info.id) .and_then(|store| Some(store.accounts.get_account(info.offset)?.0.clone_account())) @@ -599,14 +601,25 @@ impl AccountsDB { accounts: &HashMap<&Pubkey, &Account>, ) -> (Vec<(Fork, AccountInfo)>, u64) { let mut reclaims: Vec<(Fork, AccountInfo)> = Vec::with_capacity(infos.len() * 2); - let mut index = self.accounts_index.write().unwrap(); + 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()) { let key = &account.0; - index.insert(fork_id, key, info, &mut reclaims); + if let Some(info) = index.update(fork_id, key, info, &mut reclaims) { + inserts.push((account, 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 { + index.insert(fork_id, pubkey, info, &mut reclaims); + } } update_index_work.stop(); - (reclaims, index.last_root) + (reclaims, last_root) } fn remove_dead_accounts(&self, reclaims: Vec<(Fork, AccountInfo)>) -> HashSet { @@ -1270,22 +1283,17 @@ mod tests { //store an account accounts.store(0, &hashmap!(&pubkey => &account)); let ancestors = vec![(0, 0)].into_iter().collect(); - let info = accounts - .accounts_index - .read() - .unwrap() - .get(&pubkey, &ancestors) - .unwrap() - .0 - .clone(); + let id = { + let index = accounts.accounts_index.read().unwrap(); + let (list, idx) = index.get(&pubkey, &ancestors).unwrap(); + list[idx].1.id + }; //fork 0 is behind root, but it is not root, therefore it is purged accounts.add_root(1); assert!(accounts.accounts_index.read().unwrap().is_purged(0)); //fork is still there, since gc is lazy - assert!(accounts.storage.read().unwrap().0[&0] - .get(&info.id) - .is_some()); + assert!(accounts.storage.read().unwrap().0[&0].get(&id).is_some()); //store causes cleanup accounts.store(1, &hashmap!(&pubkey => &account)); diff --git a/runtime/src/accounts_index.rs b/runtime/src/accounts_index.rs index a70490029..cd2ea42d3 100644 --- a/runtime/src/accounts_index.rs +++ b/runtime/src/accounts_index.rs @@ -1,12 +1,14 @@ use log::*; use solana_sdk::pubkey::Pubkey; use std::collections::{HashMap, HashSet}; +use std::sync::{RwLock, RwLockReadGuard}; pub type Fork = u64; +type ForkList = Vec<(Fork, T)>; #[derive(Debug, Default)] pub struct AccountsIndex { - pub account_maps: hashbrown::HashMap>, + pub account_maps: hashbrown::HashMap>>, pub roots: HashSet, @@ -21,24 +23,22 @@ impl AccountsIndex { F: FnMut(&Pubkey, (&T, Fork)) -> (), { for (pubkey, list) in self.account_maps.iter() { - if let Some(fork_info) = self.latest_fork(ancestors, list) { - func(pubkey, fork_info); + let list_r = list.read().unwrap(); + if let Some(index) = self.latest_fork(ancestors, &list_r) { + func(pubkey, (&list_r[index].1, list_r[index].0)); } } } // find the latest fork and T in a list for a given ancestor - fn latest_fork<'a>( - &self, - ancestors: &HashMap, - list: &'a [(Fork, T)], - ) -> Option<(&'a T, Fork)> { + // returns index into 'list' if found, None if not. + fn latest_fork(&self, ancestors: &HashMap, list: &[(Fork, T)]) -> Option { let mut max = 0; let mut rv = None; - for (fork, t) in list.iter().rev() { + for (i, (fork, _t)) in list.iter().rev().enumerate() { if *fork >= max && (ancestors.get(fork).is_some() || self.is_root(*fork)) { - trace!("GET {} {:?}", fork, ancestors); - rv = Some((t, *fork)); + trace!("GET {} {:?} i: {}", fork, ancestors, i); + rv = Some((list.len() - 1) - i); max = *fork; } } @@ -47,10 +47,19 @@ impl AccountsIndex { /// Get an account /// The latest account that appears in `ancestors` or `roots` is returned. - pub fn get(&self, pubkey: &Pubkey, ancestors: &HashMap) -> Option<(&T, Fork)> { - self.account_maps - .get(pubkey) - .and_then(|list| self.latest_fork(ancestors, list)) + pub fn get( + &self, + pubkey: &Pubkey, + ancestors: &HashMap, + ) -> Option<(RwLockReadGuard>, usize)> { + self.account_maps.get(pubkey).and_then(|list| { + let lock = list.read().unwrap(); + if let Some(found_index) = self.latest_fork(ancestors, &lock) { + Some((lock, found_index)) + } else { + None + } + }) } pub fn get_max_root(roots: &HashSet, fork_vec: &[(Fork, T)]) -> Fork { @@ -70,33 +79,56 @@ impl AccountsIndex { account_info: T, reclaims: &mut Vec<(Fork, T)>, ) { - let roots = &self.roots; - let fork_vec = self + let _fork_vec = self .account_maps .entry(*pubkey) - .or_insert_with(|| (Vec::with_capacity(32))); + .or_insert_with(|| RwLock::new(Vec::with_capacity(32))); + self.update(fork, pubkey, account_info, reclaims); + } - // filter out old entries - reclaims.extend(fork_vec.iter().filter(|(f, _)| *f == fork).cloned()); - fork_vec.retain(|(f, _)| *f != fork); + // Try to update an item in account_maps. If the account is not + // already present, then the function will return back Some(account_info) which + // the caller can then take the write lock and do an 'insert' with the item. + // It returns None if the item is already present and thus successfully updated. + pub fn update( + &self, + fork: Fork, + pubkey: &Pubkey, + account_info: T, + reclaims: &mut Vec<(Fork, T)>, + ) -> Option { + let roots = &self.roots; + if let Some(lock) = self.account_maps.get(pubkey) { + let mut fork_vec = lock.write().unwrap(); + // filter out old entries + reclaims.extend(fork_vec.iter().filter(|(f, _)| *f == fork).cloned()); + fork_vec.retain(|(f, _)| *f != fork); - // add the new entry - fork_vec.push((fork, account_info)); + // add the new entry + fork_vec.push((fork, account_info)); - let max_root = Self::get_max_root(roots, fork_vec); + let max_root = Self::get_max_root(roots, &fork_vec); - reclaims.extend( - fork_vec - .iter() - .filter(|(fork, _)| Self::can_purge(max_root, *fork)) - .cloned(), - ); - fork_vec.retain(|(fork, _)| !Self::can_purge(max_root, *fork)); + reclaims.extend( + fork_vec + .iter() + .filter(|(fork, _)| Self::can_purge(max_root, *fork)) + .cloned(), + ); + fork_vec.retain(|(fork, _)| !Self::can_purge(max_root, *fork)); + + return None; + } else { + return Some(account_info); + } } pub fn add_index(&mut self, fork: Fork, pubkey: &Pubkey, account_info: T) { - let entry = self.account_maps.entry(*pubkey).or_insert_with(|| vec![]); - entry.push((fork, account_info)); + let entry = self + .account_maps + .entry(*pubkey) + .or_insert_with(|| RwLock::new(vec![])); + entry.write().unwrap().push((fork, account_info)); } pub fn is_purged(&self, fork: Fork) -> bool { @@ -136,7 +168,7 @@ mod tests { let key = Keypair::new(); let index = AccountsIndex::::default(); let ancestors = HashMap::new(); - assert_eq!(index.get(&key.pubkey(), &ancestors), None); + assert!(index.get(&key.pubkey(), &ancestors).is_none()); let mut num = 0; index.scan_accounts(&ancestors, |_pubkey, _index| num += 1); @@ -152,7 +184,7 @@ mod tests { assert!(gc.is_empty()); let ancestors = HashMap::new(); - assert_eq!(index.get(&key.pubkey(), &ancestors), None); + assert!(index.get(&key.pubkey(), &ancestors).is_none()); let mut num = 0; index.scan_accounts(&ancestors, |_pubkey, _index| num += 1); @@ -168,7 +200,7 @@ mod tests { assert!(gc.is_empty()); let ancestors = vec![(1, 1)].into_iter().collect(); - assert_eq!(index.get(&key.pubkey(), &ancestors), None); + assert!(index.get(&key.pubkey(), &ancestors).is_none()); let mut num = 0; index.scan_accounts(&ancestors, |_pubkey, _index| num += 1); @@ -184,7 +216,8 @@ mod tests { assert!(gc.is_empty()); let ancestors = vec![(0, 0)].into_iter().collect(); - assert_eq!(index.get(&key.pubkey(), &ancestors), Some((&true, 0))); + let (list, idx) = index.get(&key.pubkey(), &ancestors).unwrap(); + assert_eq!(list[idx], (0, true)); let mut num = 0; let mut found_key = false; @@ -216,7 +249,8 @@ mod tests { let ancestors = vec![].into_iter().collect(); index.add_root(0); - assert_eq!(index.get(&key.pubkey(), &ancestors), Some((&true, 0))); + let (list, idx) = index.get(&key.pubkey(), &ancestors).unwrap(); + assert_eq!(list[idx], (0, true)); } #[test] @@ -271,16 +305,20 @@ mod tests { let mut gc = Vec::new(); index.insert(0, &key.pubkey(), true, &mut gc); assert!(gc.is_empty()); - assert_eq!(index.get(&key.pubkey(), &ancestors), Some((&true, 0))); + let (list, idx) = index.get(&key.pubkey(), &ancestors).unwrap(); + assert_eq!(list[idx], (0, true)); + drop(list); let mut gc = Vec::new(); index.insert(0, &key.pubkey(), false, &mut gc); assert_eq!(gc, vec![(0, true)]); - assert_eq!(index.get(&key.pubkey(), &ancestors), Some((&false, 0))); + let (list, idx) = index.get(&key.pubkey(), &ancestors).unwrap(); + assert_eq!(list[idx], (0, false)); } #[test] fn test_update_new_fork() { + solana_logger::setup(); let key = Keypair::new(); let mut index = AccountsIndex::::default(); let ancestors = vec![(0, 0)].into_iter().collect(); @@ -289,9 +327,11 @@ mod tests { assert!(gc.is_empty()); index.insert(1, &key.pubkey(), false, &mut gc); assert!(gc.is_empty()); - assert_eq!(index.get(&key.pubkey(), &ancestors), Some((&true, 0))); + let (list, idx) = index.get(&key.pubkey(), &ancestors).unwrap(); + assert_eq!(list[idx], (0, true)); let ancestors = vec![(1, 0)].into_iter().collect(); - assert_eq!(index.get(&key.pubkey(), &ancestors), Some((&false, 1))); + let (list, idx) = index.get(&key.pubkey(), &ancestors).unwrap(); + assert_eq!(list[idx], (1, false)); } #[test] @@ -310,7 +350,8 @@ mod tests { index.insert(4, &key.pubkey(), true, &mut gc); assert_eq!(gc, vec![(0, true), (1, false), (2, true)]); let ancestors = vec![].into_iter().collect(); - assert_eq!(index.get(&key.pubkey(), &ancestors), Some((&true, 3))); + let (list, idx) = index.get(&key.pubkey(), &ancestors).unwrap(); + assert_eq!(list[idx], (3, true)); let mut num = 0; let mut found_key = false;