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

@@ -461,9 +461,11 @@ impl AccountsDB {
         accounts_index: &AccountsIndex<AccountInfo>,
         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<Fork> {
@@ -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
+        let id = {
-            .accounts_index
+            let index = accounts.accounts_index.read().unwrap();
-            .read()
+            let (list, idx) = index.get(&pubkey, &ancestors).unwrap();
-            .unwrap()
+            list[idx].1.id
-            .get(&pubkey, &ancestors)
+        };
-            .unwrap()
-            .0
-            .clone();
         //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]
+        assert!(accounts.storage.read().unwrap().0[&0].get(&id).is_some());
-            .get(&info.id)
-            .is_some());
 
         //store causes cleanup
         accounts.store(1, &hashmap!(&pubkey => &account));

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<T> = Vec<(Fork, T)>;
 
 #[derive(Debug, Default)]
 pub struct AccountsIndex<T> {
-    pub account_maps: hashbrown::HashMap<Pubkey, Vec<(Fork, T)>>,
+    pub account_maps: hashbrown::HashMap<Pubkey, RwLock<ForkList<T>>>,
 
     pub roots: HashSet<Fork>,
 
@@ -21,24 +23,22 @@ impl<T: Clone> AccountsIndex<T> {
         F: FnMut(&Pubkey, (&T, Fork)) -> (),
     {
         for (pubkey, list) in self.account_maps.iter() {
-            if let Some(fork_info) = self.latest_fork(ancestors, list) {
+            let list_r = list.read().unwrap();
-                func(pubkey, fork_info);
+            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>(
+    // returns index into 'list' if found, None if not.
-        &self,
+    fn latest_fork(&self, ancestors: &HashMap<Fork, usize>, list: &[(Fork, T)]) -> Option<usize> {
-        ancestors: &HashMap<Fork, usize>,
-        list: &'a [(Fork, T)],
-    ) -> Option<(&'a T, Fork)> {
         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);
+                trace!("GET {} {:?} i: {}", fork, ancestors, i);
-                rv = Some((t, *fork));
+                rv = Some((list.len() - 1) - i);
                 max = *fork;
             }
         }
@@ -47,10 +47,19 @@ impl<T: Clone> AccountsIndex<T> {
 
     /// Get an account
     /// The latest account that appears in `ancestors` or `roots` is returned.
-    pub fn get(&self, pubkey: &Pubkey, ancestors: &HashMap<Fork, usize>) -> Option<(&T, Fork)> {
+    pub fn get(
-        self.account_maps
+        &self,
-            .get(pubkey)
+        pubkey: &Pubkey,
-            .and_then(|list| self.latest_fork(ancestors, list))
+        ancestors: &HashMap<Fork, usize>,
+    ) -> Option<(RwLockReadGuard<ForkList<T>>, 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>, fork_vec: &[(Fork, T)]) -> Fork {
@@ -70,33 +79,56 @@ impl<T: Clone> AccountsIndex<T> {
         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
+    // Try to update an item in account_maps. If the account is not
-        reclaims.extend(fork_vec.iter().filter(|(f, _)| *f == fork).cloned());
+    // already present, then the function will return back Some(account_info) which
-        fork_vec.retain(|(f, _)| *f != fork);
+    // 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<T> {
+        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
+            // add the new entry
-        fork_vec.push((fork, account_info));
+            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(
+            reclaims.extend(
-            fork_vec
+                fork_vec
-                .iter()
+                    .iter()
-                .filter(|(fork, _)| Self::can_purge(max_root, *fork))
+                    .filter(|(fork, _)| Self::can_purge(max_root, *fork))
-                .cloned(),
+                    .cloned(),
-        );
+            );
-        fork_vec.retain(|(fork, _)| !Self::can_purge(max_root, *fork));
+            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![]);
+        let entry = self
-        entry.push((fork, account_info));
+            .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::<bool>::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::<bool>::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;