From 56878f06e10804ba135b53fea7474d7d8b17d327 Mon Sep 17 00:00:00 2001 From: Anatoly Yakovenko Date: Tue, 16 Apr 2019 05:55:39 -0700 Subject: [PATCH 1/9] storage account changes --- runtime/src/append_vec.rs | 145 ++++++++++++++++++++------------------ 1 file changed, 77 insertions(+), 68 deletions(-) diff --git a/runtime/src/append_vec.rs b/runtime/src/append_vec.rs index aa5dfa91f1905d..7544aece9fd3da 100644 --- a/runtime/src/append_vec.rs +++ b/runtime/src/append_vec.rs @@ -15,17 +15,31 @@ macro_rules! align_up { ($addr + ($align - 1)) & !($align - 1) }; } - -//TODO: This structure should contain references -/// StoredAccount contains enough context to recover the index from storage itself -#[derive(Clone, PartialEq, Debug)] -pub struct StoredAccount { +pub struct StorageMeta { /// global write version pub write_version: u64, /// key for the account pub pubkey: Pubkey, + pub data_len: u64, +} +//TODO: This structure should contain references +/// StoredAccount contains enough context to recover the index from storage itself +#[derive(Clone, PartialEq, Debug)] +pub struct StoredAccount<'a> { + pub meta: &'a StorageMeta, /// account data - pub account: Account, + pub account: &'a Account, + pub data: &'a mut [u8], +} + +impl<'a> StoredAccount<'a> { + pub fn clone_account(&self) -> Account { + let mut account = self.account.clone(); + let data = unsafe { Vec::from_raw_parts(self.data, self.data.len(), self.data.len()) }; + std::mem::swap(&mut account.data, &mut data); + std::mem::forget(data); + account + } } pub struct AppendVec { @@ -81,17 +95,24 @@ impl AppendVec { self.file_size } - // The reason for the `mut` is to allow the account data pointer to be fixed up after - // the structure is loaded + // The reason for the `mut` is to allow Vec::from_raw_parts #[allow(clippy::mut_from_ref)] - fn get_slice(&self, offset: usize, size: usize) -> &mut [u8] { + fn get_slice(&self, offset: usize, size: usize) -> Option<(&mut [u8], usize)> { let len = self.len(); - assert!(len >= offset + size); - let data = &self.map[offset..offset + size]; - unsafe { - let dst = data.as_ptr() as *mut u8; - std::slice::from_raw_parts_mut(dst, size) + if len < offset + size { + return None; } + let data = &self.map[offset..offset + size]; + //Data is aligned at the next 64 byte offset. Without alignment loading the memory may + //crash on some architectures. + let next = align_up!(offset + size, mem::size_of::()); + Some(( + unsafe { + let dst = data.as_ptr() as *mut u8; + std::slice::from_raw_parts_mut(dst, size) + }, + next, + )) } fn append_ptr(&self, offset: &mut usize, src: *const u8, len: usize) { @@ -132,67 +153,54 @@ impl AppendVec { Some(pos) } - //TODO: Make this safer - //StoredAccount should be a struct of references with the same lifetime as &self - //The structure should have a method to clone the account out - #[allow(clippy::transmute_ptr_to_ptr)] - pub fn get_account(&self, offset: usize) -> &StoredAccount { - let account: *mut StoredAccount = { - let data = self.get_slice(offset, mem::size_of::()); - unsafe { std::mem::transmute::<*const u8, *mut StoredAccount>(data.as_ptr()) } - }; - //Data is aligned at the next 64 byte offset. Without alignment loading the memory may - //crash on some architectures. - let data_at = align_up!( - offset + mem::size_of::(), - mem::size_of::() - ); - let account_ref: &mut StoredAccount = unsafe { &mut *account }; - let data = self.get_slice(data_at, account_ref.account.data.len()); - unsafe { - let mut new_data = Vec::from_raw_parts(data.as_mut_ptr(), data.len(), data.len()); - std::mem::swap(&mut account_ref.account.data, &mut new_data); - std::mem::forget(new_data); - }; - account_ref + fn get_type(&self, offset: usize) -> Option<(&T, usize)> { + let (data, next) = self.get_slice(offset, mem::size_of::())?; + let ptr: *const T = data.as_ptr() as *const T; + Some((unsafe { &*ptr }, next)) + } + + pub fn get_account<'a>(&'a self, offset: usize) -> Option<(StoredAccount<'a>, usize)> { + let (storage_meta, next): (&'a StorageMeta, _) = self.get_type(offset)?; + let (account, next): (&'a Account, _) = self.get_type(next)?; + let (data, next) = self.get_slice(next, storage_meta.data_len)?; + Some(( + StoredAccount { + storage_meta, + account, + data, + }, + next, + )) } - pub fn accounts(&self, mut start: usize) -> Vec<&StoredAccount> { + pub fn accounts<'a>(&'a self, mut start: usize) -> Vec> { let mut accounts = vec![]; loop { - //Data is aligned at the next 64 byte offset. Without alignment loading the memory may - //crash on some architectures. - let end = align_up!( - start + mem::size_of::(), - mem::size_of::() - ); - if end > self.len() { + if let Some((account, next)) = self.get_account(start) { + accounts.push(account); + start = next; + } else { break; } - let first = self.get_account(start); - accounts.push(first); - //Data is aligned at the next 64 byte offset. Without alignment loading the memory may - //crash on some architectures. - let data_at = align_up!( - start + mem::size_of::(), - mem::size_of::() - ); - let next = align_up!(data_at + first.account.data.len(), mem::size_of::()); - start = next; } accounts } - pub fn append_account(&self, account: &StoredAccount) -> Option { - let acc_ptr = account as *const StoredAccount; - let data_len = account.account.data.len(); - let data_ptr = account.account.data.as_ptr(); + pub fn append_account(&self, storage_meta: StorageMeta, account: &Account) -> Option { + let meta_ptr = &storage_meta as *const StorageMeta; + let account_ptr = account as *const Account; + let data_len = account.data.len(); + let data_ptr = account.data.as_ptr(); let ptrs = [ - (acc_ptr as *const u8, mem::size_of::()), + (meta_ptr as *const u8, mem::size_of::()), + (account_ptr as *const u8, mem::size_of::()), (data_ptr, data_len), ]; self.append_ptrs(&ptrs) } + pub fn append_account_test(&self, data: &(storage_meta: StorageMeta, account: Account)) -> Option { + self.append_account(data.storage_meta, &data.account) + } } pub mod test_utils { @@ -226,15 +234,16 @@ pub mod test_utils { TempFile { path: buf } } - pub fn create_test_account(sample: usize) -> StoredAccount { + pub fn create_test_account(sample: usize) -> (StorageMeta, Account) { let data_len = sample % 256; let mut account = Account::new(sample as u64, 0, &Pubkey::default()); account.data = (0..data_len).map(|_| data_len as u8).collect(); - StoredAccount { + let storage_meta = StorageMeta { write_version: 0, pubkey: Pubkey::default(), - account, - } + data_len, + }; + (storage_meta, account) } } @@ -252,7 +261,7 @@ pub mod tests { let path = get_append_vec_path("test_append"); let av = AppendVec::new(&path.path, true, 1024 * 1024); let account = create_test_account(0); - let index = av.append_account(&account).unwrap(); + let index = av.append_account_test(&account).unwrap(); assert_eq!(*av.get_account(index), account); } @@ -261,10 +270,10 @@ pub mod tests { let path = get_append_vec_path("test_append_data"); let av = AppendVec::new(&path.path, true, 1024 * 1024); let account = create_test_account(5); - let index = av.append_account(&account).unwrap(); + let index = av.append_account_test(&account).unwrap(); assert_eq!(*av.get_account(index), account); let account1 = create_test_account(6); - let index1 = av.append_account(&account1).unwrap(); + let index1 = av.append_account_test(&account1).unwrap(); assert_eq!(*av.get_account(index), account); assert_eq!(*av.get_account(index1), account1); } @@ -278,7 +287,7 @@ pub mod tests { let now = Instant::now(); for sample in 0..size { let account = create_test_account(sample); - let pos = av.append_account(&account).unwrap(); + let pos = av.append_account_test(&account).unwrap(); assert_eq!(*av.get_account(pos), account); indexes.push(pos) } From 35d8a25aa46fe355dba6db1e4345410eff5bb820 Mon Sep 17 00:00:00 2001 From: Anatoly Yakovenko Date: Tue, 16 Apr 2019 06:54:51 -0700 Subject: [PATCH 2/9] cleanup --- runtime/src/accounts.rs | 51 ++++++++++++++++++++++++--------------- runtime/src/append_vec.rs | 20 ++++++++------- 2 files changed, 42 insertions(+), 29 deletions(-) diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index c62346cccf693b..6dfe88aca9054e 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -19,7 +19,7 @@ //! commit for each fork entry would be indexed. use crate::accounts_index::{AccountsIndex, Fork}; -use crate::append_vec::{AppendVec, StoredAccount}; +use crate::append_vec::{AppendVec, StorageMeta, StoredAccount}; use crate::message_processor::has_duplicates; use bincode::serialize; use hashbrown::{HashMap, HashSet}; @@ -29,7 +29,7 @@ use rayon::prelude::*; use solana_metrics::counter::Counter; use solana_sdk::account::Account; use solana_sdk::fee_calculator::FeeCalculator; -use solana_sdk::hash::{hash, Hash, Hasher}; +use solana_sdk::hash::{Hash, Hasher}; use solana_sdk::native_loader; use solana_sdk::pubkey::Pubkey; use solana_sdk::signature::{Keypair, KeypairUtil}; @@ -278,14 +278,23 @@ impl AccountsDB { .collect() } + fn hash_account(stored_account: &StoredAccount) -> Hash { + let mut hasher = Hasher::default(); + hasher.hash(&serialize(&stored_account.account.lamports).unwrap()); + hasher.hash(&serialize(&stored_account.account.owner).unwrap()); + hasher.hash(&serialize(&stored_account.account.executable).unwrap()); + hasher.hash(&serialize(&stored_account.data).unwrap()); + hasher.result() + } + pub fn hash_internal_state(&self, fork_id: Fork) -> Option { let accumulator: Vec> = self.scan_account_storage( fork_id, |stored_account: &StoredAccount, accum: &mut Vec<(Pubkey, u64, Hash)>| { accum.push(( - stored_account.pubkey, - stored_account.write_version, - hash(&serialize(&stored_account.account).unwrap()), + stored_account.meta.pubkey, + stored_account.meta.write_version, + Self::hash_account(stored_account), )); }, ); @@ -313,7 +322,7 @@ impl AccountsDB { //TODO: thread this as a ref storage .get(info.id) - .map(|store| store.accounts.get_account(info.offset).account.clone()) + .and_then(|store| Some(store.accounts.get_account(info.offset)?.0.clone_account())) } fn load_tx_accounts( @@ -530,13 +539,12 @@ impl AccountsDB { { let accounts = &self.storage.read().unwrap()[id]; let write_version = self.write_version.fetch_add(1, Ordering::Relaxed) as u64; - let stored_account = StoredAccount { + let meta = StorageMeta { write_version, pubkey: *pubkey, - //TODO: fix all this copy - account: account.clone(), + data_len: account.data.len() as u64, }; - result = accounts.accounts.append_account(&stored_account); + result = accounts.accounts.append_account(meta, account); accounts.add_account(); } if let Some(val) = result { @@ -691,21 +699,24 @@ impl Accounts { } pub fn load_by_program(&self, fork: Fork, program_id: &Pubkey) -> Vec<(Pubkey, Account)> { - let accumulator: Vec> = self.accounts_db.scan_account_storage( + let accumulator: Vec> = self.accounts_db.scan_account_storage( fork, - |stored_account: &StoredAccount, accum: &mut Vec| { + |stored_account: &StoredAccount, accum: &mut Vec<(Pubkey, u64, Account)>| { if stored_account.account.owner == *program_id { - accum.push(stored_account.clone()) + let val = ( + stored_account.meta.pubkey, + stored_account.meta.write_version, + stored_account.clone_account(), + ); + accum.push(val) } }, ); - let mut versions: Vec = accumulator.into_iter().flat_map(|x| x).collect(); - versions.sort_by_key(|s| (s.pubkey, (s.write_version as i64).neg())); - versions.dedup_by_key(|s| s.pubkey); - versions - .into_iter() - .map(|s| (s.pubkey, s.account)) - .collect() + let mut versions: Vec<(Pubkey, u64, Account)> = + accumulator.into_iter().flat_map(|x| x).collect(); + versions.sort_by_key(|s| (s.0, (s.1 as i64).neg())); + versions.dedup_by_key(|s| s.0); + versions.into_iter().map(|s| (s.0, s.2)).collect() } /// Slow because lock is held for 1 operation instead of many diff --git a/runtime/src/append_vec.rs b/runtime/src/append_vec.rs index 7544aece9fd3da..8d2624533be066 100644 --- a/runtime/src/append_vec.rs +++ b/runtime/src/append_vec.rs @@ -15,6 +15,7 @@ macro_rules! align_up { ($addr + ($align - 1)) & !($align - 1) }; } +#[derive(Clone, PartialEq, Debug)] pub struct StorageMeta { /// global write version pub write_version: u64, @@ -24,7 +25,6 @@ pub struct StorageMeta { } //TODO: This structure should contain references /// StoredAccount contains enough context to recover the index from storage itself -#[derive(Clone, PartialEq, Debug)] pub struct StoredAccount<'a> { pub meta: &'a StorageMeta, /// account data @@ -35,7 +35,9 @@ pub struct StoredAccount<'a> { impl<'a> StoredAccount<'a> { pub fn clone_account(&self) -> Account { let mut account = self.account.clone(); - let data = unsafe { Vec::from_raw_parts(self.data, self.data.len(), self.data.len()) }; + let mut data: Vec = unsafe { + Vec::from_raw_parts(self.data.as_mut_ptr(), self.data.len(), self.data.len()) + }; std::mem::swap(&mut account.data, &mut data); std::mem::forget(data); account @@ -160,12 +162,12 @@ impl AppendVec { } pub fn get_account<'a>(&'a self, offset: usize) -> Option<(StoredAccount<'a>, usize)> { - let (storage_meta, next): (&'a StorageMeta, _) = self.get_type(offset)?; + let (meta, next): (&'a StorageMeta, _) = self.get_type(offset)?; let (account, next): (&'a Account, _) = self.get_type(next)?; - let (data, next) = self.get_slice(next, storage_meta.data_len)?; + let (data, next) = self.get_slice(next, meta.data_len as usize)?; Some(( StoredAccount { - storage_meta, + meta, account, data, }, @@ -198,13 +200,13 @@ impl AppendVec { ]; self.append_ptrs(&ptrs) } - pub fn append_account_test(&self, data: &(storage_meta: StorageMeta, account: Account)) -> Option { - self.append_account(data.storage_meta, &data.account) + pub fn append_account_test(&self, data: &(StorageMeta, Account)) -> Option { + self.append_account(data.0.clone(), &data.1) } } pub mod test_utils { - use super::StoredAccount; + use super::StorageMeta; use rand::distributions::Alphanumeric; use rand::{thread_rng, Rng}; use solana_sdk::account::Account; @@ -241,7 +243,7 @@ pub mod test_utils { let storage_meta = StorageMeta { write_version: 0, pubkey: Pubkey::default(), - data_len, + data_len: data_len as u64, }; (storage_meta, account) } From 2b784ab31c0deb3aa0c6bdbe056f3a968396b187 Mon Sep 17 00:00:00 2001 From: Anatoly Yakovenko Date: Tue, 16 Apr 2019 06:58:57 -0700 Subject: [PATCH 3/9] checks --- runtime/src/accounts.rs | 10 +++++----- runtime/src/append_vec.rs | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 6dfe88aca9054e..e02fddfa3e6bd2 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -253,7 +253,7 @@ impl AccountsDB { // PERF: Sequentially read each storage entry in parallel pub fn scan_account_storage(&self, fork_id: Fork, scan_func: F) -> Vec where - F: Fn(&StoredAccount, &mut B) -> (), + F: Fn(&mut StoredAccount, &mut B) -> (), F: Send + Sync, B: Send + Default, { @@ -268,10 +268,10 @@ impl AccountsDB { storage_maps .into_par_iter() .map(|storage| { - let accounts = storage.accounts.accounts(0); + let mut accounts = storage.accounts.accounts(0); let mut retval = B::default(); accounts - .iter() + .iter_mut() .for_each(|stored_account| scan_func(stored_account, &mut retval)); retval }) @@ -290,7 +290,7 @@ impl AccountsDB { pub fn hash_internal_state(&self, fork_id: Fork) -> Option { let accumulator: Vec> = self.scan_account_storage( fork_id, - |stored_account: &StoredAccount, accum: &mut Vec<(Pubkey, u64, Hash)>| { + |stored_account: &mut StoredAccount, accum: &mut Vec<(Pubkey, u64, Hash)>| { accum.push(( stored_account.meta.pubkey, stored_account.meta.write_version, @@ -701,7 +701,7 @@ impl Accounts { pub fn load_by_program(&self, fork: Fork, program_id: &Pubkey) -> Vec<(Pubkey, Account)> { let accumulator: Vec> = self.accounts_db.scan_account_storage( fork, - |stored_account: &StoredAccount, accum: &mut Vec<(Pubkey, u64, Account)>| { + |stored_account: &mut StoredAccount, accum: &mut Vec<(Pubkey, u64, Account)>| { if stored_account.account.owner == *program_id { let val = ( stored_account.meta.pubkey, diff --git a/runtime/src/append_vec.rs b/runtime/src/append_vec.rs index 8d2624533be066..88d738f62e093f 100644 --- a/runtime/src/append_vec.rs +++ b/runtime/src/append_vec.rs @@ -33,7 +33,7 @@ pub struct StoredAccount<'a> { } impl<'a> StoredAccount<'a> { - pub fn clone_account(&self) -> Account { + pub fn clone_account(&mut self) -> Account { let mut account = self.account.clone(); let mut data: Vec = unsafe { Vec::from_raw_parts(self.data.as_mut_ptr(), self.data.len(), self.data.len()) From a62912215ebce99da61634087921a96d0c27ce49 Mon Sep 17 00:00:00 2001 From: Anatoly Yakovenko Date: Tue, 16 Apr 2019 07:01:16 -0700 Subject: [PATCH 4/9] comments --- runtime/src/append_vec.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/runtime/src/append_vec.rs b/runtime/src/append_vec.rs index 88d738f62e093f..f8cec048e62022 100644 --- a/runtime/src/append_vec.rs +++ b/runtime/src/append_vec.rs @@ -15,6 +15,8 @@ macro_rules! align_up { ($addr + ($align - 1)) & !($align - 1) }; } + +/// StorageMeta contains enough context to recover the index from storage itself #[derive(Clone, PartialEq, Debug)] pub struct StorageMeta { /// global write version @@ -23,8 +25,9 @@ pub struct StorageMeta { pub pubkey: Pubkey, pub data_len: u64, } -//TODO: This structure should contain references -/// StoredAccount contains enough context to recover the index from storage itself + +/// References to Memory Mapped memory +/// The Account is stored separately from its data, so getting the actual account requires a clone pub struct StoredAccount<'a> { pub meta: &'a StorageMeta, /// account data From 901deb558e6b2ae022fb1471b4b225c4c651edfd Mon Sep 17 00:00:00 2001 From: Anatoly Yakovenko Date: Tue, 16 Apr 2019 07:02:31 -0700 Subject: [PATCH 5/9] clippy --- runtime/src/append_vec.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/runtime/src/append_vec.rs b/runtime/src/append_vec.rs index f8cec048e62022..841212136a1625 100644 --- a/runtime/src/append_vec.rs +++ b/runtime/src/append_vec.rs @@ -180,13 +180,9 @@ impl AppendVec { pub fn accounts<'a>(&'a self, mut start: usize) -> Vec> { let mut accounts = vec![]; - loop { - if let Some((account, next)) = self.get_account(start) { - accounts.push(account); - start = next; - } else { - break; - } + while let Some((account, next)) = self.get_account(start) { + accounts.push(account); + start = next; } accounts } From 2180315303ebe3630affdca08e1b81850ca9fc38 Mon Sep 17 00:00:00 2001 From: Anatoly Yakovenko Date: Tue, 16 Apr 2019 07:13:18 -0700 Subject: [PATCH 6/9] tests --- runtime/src/append_vec.rs | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/runtime/src/append_vec.rs b/runtime/src/append_vec.rs index 841212136a1625..8b5d7d6ffe49de 100644 --- a/runtime/src/append_vec.rs +++ b/runtime/src/append_vec.rs @@ -177,6 +177,11 @@ impl AppendVec { next, )) } + pub fn get_account_test(&self, offset: usize) -> Option<(StorageMeta, Account)> { + let mut stored = self.get_account(offset)?; + let meta = stored.0.meta.clone(); + Some((meta, stored.0.clone_account())) + } pub fn accounts<'a>(&'a self, mut start: usize) -> Vec> { let mut accounts = vec![]; @@ -263,7 +268,7 @@ pub mod tests { let av = AppendVec::new(&path.path, true, 1024 * 1024); let account = create_test_account(0); let index = av.append_account_test(&account).unwrap(); - assert_eq!(*av.get_account(index), account); + assert_eq!(av.get_account_test(index).unwrap(), account); } #[test] @@ -272,11 +277,11 @@ pub mod tests { let av = AppendVec::new(&path.path, true, 1024 * 1024); let account = create_test_account(5); let index = av.append_account_test(&account).unwrap(); - assert_eq!(*av.get_account(index), account); + assert_eq!(av.get_account_test(index).unwrap(), account); let account1 = create_test_account(6); let index1 = av.append_account_test(&account1).unwrap(); - assert_eq!(*av.get_account(index), account); - assert_eq!(*av.get_account(index1), account1); + assert_eq!(av.get_account_test(index).unwrap(), account); + assert_eq!(av.get_account_test(index1).unwrap(), account1); } #[test] @@ -289,7 +294,7 @@ pub mod tests { for sample in 0..size { let account = create_test_account(sample); let pos = av.append_account_test(&account).unwrap(); - assert_eq!(*av.get_account(pos), account); + assert_eq!(av.get_account_test(pos).unwrap(), account); indexes.push(pos) } trace!("append time: {} ms", duration_as_ms(&now.elapsed()),); @@ -298,18 +303,19 @@ pub mod tests { for _ in 0..size { let sample = thread_rng().gen_range(0, indexes.len()); let account = create_test_account(sample); - assert_eq!(*av.get_account(indexes[sample]), account); + assert_eq!(av.get_account_test(indexes[sample]).unwrap(), account); } trace!("random read time: {} ms", duration_as_ms(&now.elapsed()),); let now = Instant::now(); assert_eq!(indexes.len(), size); assert_eq!(indexes[0], 0); - let accounts = av.accounts(indexes[0]); + let mut accounts = av.accounts(indexes[0]); assert_eq!(accounts.len(), size); - for (sample, v) in accounts.iter().enumerate() { + for (sample, v) in accounts.iter_mut().enumerate() { let account = create_test_account(sample); - assert_eq!(**v, account) + let recovered = v.clone_account(); + assert_eq!(recovered, account.1) } trace!( "sequential read time: {} ms", From 4711e4b78dfdb15fe0ac6d246bd77aa9d2a36d88 Mon Sep 17 00:00:00 2001 From: Anatoly Yakovenko Date: Tue, 16 Apr 2019 07:33:50 -0700 Subject: [PATCH 7/9] woot! --- runtime/src/accounts.rs | 16 +++++------- runtime/src/append_vec.rs | 54 +++++++++++++++++++++++---------------- runtime/src/lib.rs | 3 +++ 3 files changed, 42 insertions(+), 31 deletions(-) diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index e02fddfa3e6bd2..c62a65c8592433 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -253,7 +253,7 @@ impl AccountsDB { // PERF: Sequentially read each storage entry in parallel pub fn scan_account_storage(&self, fork_id: Fork, scan_func: F) -> Vec where - F: Fn(&mut StoredAccount, &mut B) -> (), + F: Fn(&StoredAccount, &mut B) -> (), F: Send + Sync, B: Send + Default, { @@ -268,10 +268,10 @@ impl AccountsDB { storage_maps .into_par_iter() .map(|storage| { - let mut accounts = storage.accounts.accounts(0); + let accounts = storage.accounts.accounts(0); let mut retval = B::default(); accounts - .iter_mut() + .iter() .for_each(|stored_account| scan_func(stored_account, &mut retval)); retval }) @@ -280,9 +280,7 @@ impl AccountsDB { fn hash_account(stored_account: &StoredAccount) -> Hash { let mut hasher = Hasher::default(); - hasher.hash(&serialize(&stored_account.account.lamports).unwrap()); - hasher.hash(&serialize(&stored_account.account.owner).unwrap()); - hasher.hash(&serialize(&stored_account.account.executable).unwrap()); + hasher.hash(&serialize(&stored_account.balance).unwrap()); hasher.hash(&serialize(&stored_account.data).unwrap()); hasher.result() } @@ -290,7 +288,7 @@ impl AccountsDB { pub fn hash_internal_state(&self, fork_id: Fork) -> Option { let accumulator: Vec> = self.scan_account_storage( fork_id, - |stored_account: &mut StoredAccount, accum: &mut Vec<(Pubkey, u64, Hash)>| { + |stored_account: &StoredAccount, accum: &mut Vec<(Pubkey, u64, Hash)>| { accum.push(( stored_account.meta.pubkey, stored_account.meta.write_version, @@ -701,8 +699,8 @@ impl Accounts { pub fn load_by_program(&self, fork: Fork, program_id: &Pubkey) -> Vec<(Pubkey, Account)> { let accumulator: Vec> = self.accounts_db.scan_account_storage( fork, - |stored_account: &mut StoredAccount, accum: &mut Vec<(Pubkey, u64, Account)>| { - if stored_account.account.owner == *program_id { + |stored_account: &StoredAccount, accum: &mut Vec<(Pubkey, u64, Account)>| { + if stored_account.balance.owner == *program_id { let val = ( stored_account.meta.pubkey, stored_account.meta.write_version, diff --git a/runtime/src/append_vec.rs b/runtime/src/append_vec.rs index 8b5d7d6ffe49de..2c43c0ee3b298d 100644 --- a/runtime/src/append_vec.rs +++ b/runtime/src/append_vec.rs @@ -26,24 +26,33 @@ pub struct StorageMeta { pub data_len: u64, } +#[derive(Serialize, Deserialize, Clone, Default, Eq, PartialEq)] +pub struct AccountBalance { + /// lamports in the account + pub lamports: u64, + /// the program that owns this account. If executable, the program that loads this account. + pub owner: Pubkey, + /// this account's data contains a loaded program (and is now read-only) + pub executable: bool, +} + /// References to Memory Mapped memory /// The Account is stored separately from its data, so getting the actual account requires a clone pub struct StoredAccount<'a> { pub meta: &'a StorageMeta, /// account data - pub account: &'a Account, - pub data: &'a mut [u8], + pub balance: &'a AccountBalance, + pub data: &'a [u8], } impl<'a> StoredAccount<'a> { - pub fn clone_account(&mut self) -> Account { - let mut account = self.account.clone(); - let mut data: Vec = unsafe { - Vec::from_raw_parts(self.data.as_mut_ptr(), self.data.len(), self.data.len()) - }; - std::mem::swap(&mut account.data, &mut data); - std::mem::forget(data); - account + pub fn clone_account(&self) -> Account { + Account { + lamports: self.balance.lamports, + owner: self.balance.owner, + executable: self.balance.executable, + data: self.data.to_vec(), + } } } @@ -101,8 +110,7 @@ impl AppendVec { } // The reason for the `mut` is to allow Vec::from_raw_parts - #[allow(clippy::mut_from_ref)] - fn get_slice(&self, offset: usize, size: usize) -> Option<(&mut [u8], usize)> { + fn get_slice(&self, offset: usize, size: usize) -> Option<(&[u8], usize)> { let len = self.len(); if len < offset + size { return None; @@ -112,10 +120,7 @@ impl AppendVec { //crash on some architectures. let next = align_up!(offset + size, mem::size_of::()); Some(( - unsafe { - let dst = data.as_ptr() as *mut u8; - std::slice::from_raw_parts_mut(dst, size) - }, + unsafe { std::slice::from_raw_parts(data.as_ptr() as *const u8, size) }, next, )) } @@ -166,19 +171,19 @@ impl AppendVec { pub fn get_account<'a>(&'a self, offset: usize) -> Option<(StoredAccount<'a>, usize)> { let (meta, next): (&'a StorageMeta, _) = self.get_type(offset)?; - let (account, next): (&'a Account, _) = self.get_type(next)?; + let (balance, next): (&'a AccountBalance, _) = self.get_type(next)?; let (data, next) = self.get_slice(next, meta.data_len as usize)?; Some(( StoredAccount { meta, - account, + balance, data, }, next, )) } pub fn get_account_test(&self, offset: usize) -> Option<(StorageMeta, Account)> { - let mut stored = self.get_account(offset)?; + let stored = self.get_account(offset)?; let meta = stored.0.meta.clone(); Some((meta, stored.0.clone_account())) } @@ -194,12 +199,17 @@ impl AppendVec { pub fn append_account(&self, storage_meta: StorageMeta, account: &Account) -> Option { let meta_ptr = &storage_meta as *const StorageMeta; - let account_ptr = account as *const Account; + let balance = AccountBalance { + lamports: account.lamports, + owner: account.owner, + executable: account.executable, + }; + let balance_ptr = &balance as *const AccountBalance; let data_len = account.data.len(); let data_ptr = account.data.as_ptr(); let ptrs = [ (meta_ptr as *const u8, mem::size_of::()), - (account_ptr as *const u8, mem::size_of::()), + (balance_ptr as *const u8, mem::size_of::()), (data_ptr, data_len), ]; self.append_ptrs(&ptrs) @@ -263,7 +273,7 @@ pub mod tests { use std::time::Instant; #[test] - fn test_append_vec() { + fn test_append_vec_one() { let path = get_append_vec_path("test_append"); let av = AppendVec::new(&path.path, true, 1024 * 1024); let account = create_test_account(0); diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 60ed866e6d61d9..b7b14fcf5126d2 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -14,3 +14,6 @@ mod system_instruction_processor; #[macro_use] extern crate solana_metrics; + +#[macro_use] +extern crate serde_derive; From 11067f4235cf8d930c673cd4d53204de42f0934b Mon Sep 17 00:00:00 2001 From: Anatoly Yakovenko Date: Tue, 16 Apr 2019 07:36:57 -0700 Subject: [PATCH 8/9] comments --- runtime/src/append_vec.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/runtime/src/append_vec.rs b/runtime/src/append_vec.rs index 2c43c0ee3b298d..6013439ced330f 100644 --- a/runtime/src/append_vec.rs +++ b/runtime/src/append_vec.rs @@ -130,6 +130,7 @@ impl AppendVec { //crash on some architectures. let pos = align_up!(*offset as usize, mem::size_of::()); let data = &self.map[pos..(pos + len)]; + //this mut append is safe because only 1 thread can append at a time unsafe { let dst = data.as_ptr() as *mut u8; std::ptr::copy(src, dst, len); From 3c90d6f201e1cc9f92549a37d92deb14e936d298 Mon Sep 17 00:00:00 2001 From: Anatoly Yakovenko Date: Tue, 16 Apr 2019 08:10:10 -0700 Subject: [PATCH 9/9] benches --- runtime/benches/append_vec.rs | 40 +++++++++++++++++------------------ runtime/src/accounts.rs | 2 +- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/runtime/benches/append_vec.rs b/runtime/benches/append_vec.rs index ccd5017e146d4c..e55d8d9f71dadb 100644 --- a/runtime/benches/append_vec.rs +++ b/runtime/benches/append_vec.rs @@ -15,8 +15,8 @@ fn append_vec_append(bencher: &mut Bencher) { let path = get_append_vec_path("bench_append"); let vec = AppendVec::new(&path.path, true, 64 * 1024); bencher.iter(|| { - let account = create_test_account(0); - if vec.append_account(&account).is_none() { + let (meta, account) = create_test_account(0); + if vec.append_account(meta, &account).is_none() { vec.reset(); } }); @@ -26,8 +26,8 @@ fn add_test_accounts(vec: &AppendVec, size: usize) -> Vec<(usize, usize)> { (0..size) .into_iter() .filter_map(|sample| { - let account = create_test_account(sample); - vec.append_account(&account).map(|pos| (sample, pos)) + let (meta, account) = create_test_account(sample); + vec.append_account(meta, &account).map(|pos| (sample, pos)) }) .collect() } @@ -40,9 +40,9 @@ fn append_vec_sequential_read(bencher: &mut Bencher) { let mut indexes = add_test_accounts(&vec, size); bencher.iter(|| { let (sample, pos) = indexes.pop().unwrap(); - let account = vec.get_account(pos); - let test = create_test_account(sample); - assert_eq!(*account, test); + let (account, _next) = vec.get_account(pos).unwrap(); + let (_meta, test) = create_test_account(sample); + assert_eq!(account.data, test.data.as_slice()); indexes.push((sample, pos)); }); } @@ -55,9 +55,9 @@ fn append_vec_random_read(bencher: &mut Bencher) { bencher.iter(|| { let random_index: usize = thread_rng().gen_range(0, indexes.len()); let (sample, pos) = &indexes[random_index]; - let account = vec.get_account(*pos); - let test = create_test_account(*sample); - assert_eq!(*account, test); + let (account, _next) = vec.get_account(*pos).unwrap(); + let (_meta, test) = create_test_account(*sample); + assert_eq!(account.data, test.data.as_slice()); }); } @@ -70,8 +70,8 @@ fn append_vec_concurrent_append_read(bencher: &mut Bencher) { let indexes1 = indexes.clone(); spawn(move || loop { let sample = indexes1.lock().unwrap().len(); - let account = create_test_account(sample); - if let Some(pos) = vec1.append_account(&account) { + let (meta, account) = create_test_account(sample); + if let Some(pos) = vec1.append_account(meta, &account) { indexes1.lock().unwrap().push((sample, pos)) } else { break; @@ -84,9 +84,9 @@ fn append_vec_concurrent_append_read(bencher: &mut Bencher) { let len = indexes.lock().unwrap().len(); let random_index: usize = thread_rng().gen_range(0, len); let (sample, pos) = indexes.lock().unwrap().get(random_index).unwrap().clone(); - let account = vec.get_account(pos); - let test = create_test_account(sample); - assert_eq!(*account, test); + let (account, _next) = vec.get_account(pos).unwrap(); + let (_meta, test) = create_test_account(sample); + assert_eq!(account.data, test.data.as_slice()); }); } @@ -106,14 +106,14 @@ fn append_vec_concurrent_read_append(bencher: &mut Bencher) { .get(random_index % len) .unwrap() .clone(); - let account = vec1.get_account(pos); - let test = create_test_account(sample); - assert_eq!(*account, test); + let (account, _next) = vec1.get_account(pos).unwrap(); + let (_meta, test) = create_test_account(sample); + assert_eq!(account.data, test.data.as_slice()); }); bencher.iter(|| { let sample: usize = thread_rng().gen_range(0, 256); - let account = create_test_account(sample); - if let Some(pos) = vec.append_account(&account) { + let (meta, account) = create_test_account(sample); + if let Some(pos) = vec.append_account(meta, &account) { indexes.lock().unwrap().push((sample, pos)) } }); diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index c62a65c8592433..d296c760459170 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -281,7 +281,7 @@ impl AccountsDB { fn hash_account(stored_account: &StoredAccount) -> Hash { let mut hasher = Hasher::default(); hasher.hash(&serialize(&stored_account.balance).unwrap()); - hasher.hash(&serialize(&stored_account.data).unwrap()); + hasher.hash(stored_account.data); hasher.result() }