Skip to content

Commit

Permalink
Fix review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
sambley committed Feb 26, 2019
1 parent f7d4a15 commit 68b6e6c
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 56 deletions.
4 changes: 3 additions & 1 deletion fullnode/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,9 @@ fn main() {
let (keypair, gossip) = parse_identity(&matches);
let ledger_path = matches.value_of("ledger").unwrap();
if let Some(paths) = matches.value_of("accounts") {
fullnode_config.account_paths = paths.to_string();
fullnode_config.account_paths = Some(paths.to_string());
} else {
fullnode_config.account_paths = None;
}
let cluster_entrypoint = matches
.value_of("network")
Expand Down
44 changes: 23 additions & 21 deletions runtime/src/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,17 +254,19 @@ impl AccountsDB {
.unwrap()
.iter()
.for_each(|p| {
if let Some(entry) = self.index_info.index.read().unwrap().get(p) {
let forks = entry.0.read().unwrap();
if let Some((id, index)) = forks.get(&fork) {
accounts.push(
self.storage.read().unwrap()[*id]
.appendvec
.read()
.unwrap()
.get_account(*index)
.unwrap(),
);
if let Some(map) = self.index_info.index.read().unwrap().get(p) {
let forks = map.0.read().unwrap();
if let Some((id, offset)) = forks.get(&fork) {
accounts.push(self.get_account(*id, *offset));
} else {
let fork_info = self.fork_info.read().unwrap();
if let Some(info) = fork_info.get(&fork) {
for parent_fork in info.parents.iter() {
if let Some((id, offset)) = forks.get(&parent_fork) {
accounts.push(self.get_account(*id, *offset));
}
}
}
}
}
});
Expand Down Expand Up @@ -305,10 +307,10 @@ impl AccountsDB {
Some(hash(&serialize(&ordered_accounts).unwrap()))
}

fn get_account(&self, id: AppendVecId, offset: u64) -> Option<Account> {
fn get_account(&self, id: AppendVecId, offset: u64) -> Account {
let appendvec = &self.storage.read().unwrap()[id].appendvec;
let av = appendvec.read().unwrap();
Some(av.get_account(offset).unwrap())
av.get_account(offset).unwrap()
}

fn load(&self, fork: Fork, pubkey: &Pubkey, walk_back: bool) -> Option<Account> {
Expand All @@ -317,7 +319,7 @@ impl AccountsDB {
let forks = map.0.read().unwrap();
// find most recent fork that is an ancestor of current_fork
if let Some((id, offset)) = forks.get(&fork) {
return self.get_account(*id, *offset);
return Some(self.get_account(*id, *offset));
} else {
if !walk_back {
return None;
Expand All @@ -326,7 +328,7 @@ impl AccountsDB {
if let Some(info) = fork_info.get(&fork) {
for parent_fork in info.parents.iter() {
if let Some((id, offset)) = forks.get(&parent_fork) {
return self.get_account(*id, *offset);
return Some(self.get_account(*id, *offset));
}
}
}
Expand Down Expand Up @@ -689,7 +691,7 @@ impl AccountsDB {
keys.push(pubkey.clone());
}
}
let account = self.get_account(id, offset).unwrap();
let account = self.get_account(id, offset);
if account.tokens == 0 {
if self.remove_account_entries(&[fork], &map) {
keys.push(pubkey.clone());
Expand Down Expand Up @@ -737,11 +739,11 @@ impl Accounts {
paths
}

pub fn new(fork: Fork, in_paths: &str) -> Self {
let paths = if !in_paths.is_empty() {
in_paths.to_string()
} else {
pub fn new(fork: Fork, in_paths: Option<String>) -> Self {
let paths = if in_paths.is_none() {
Self::make_default_paths()
} else {
in_paths.unwrap()
};
let accounts_db = AccountsDB::new(fork, &paths);
accounts_db.add_fork(fork, None);
Expand Down Expand Up @@ -932,7 +934,7 @@ mod tests {
ka: &Vec<(Pubkey, Account)>,
error_counters: &mut ErrorCounters,
) -> Vec<Result<(InstructionAccounts, InstructionLoaders)>> {
let accounts = Accounts::new(0, "");
let accounts = Accounts::new(0, None);
for ka in ka.iter() {
accounts.store_slow(0, true, &ka.0, &ka.1);
}
Expand Down
46 changes: 27 additions & 19 deletions runtime/src/appendvec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,35 +160,43 @@ where
}

let mut at = data_at as usize;
let data = &self.map[at..at + mem::size_of::<u64>()];
#[allow(clippy::cast_ptr_alignment)]
let ptr = data.as_ptr() as *mut u64;
unsafe {
let data = &self.map[at..at + mem::size_of::<u64>()];
#[allow(clippy::cast_ptr_alignment)]
let ptr = data.as_ptr() as *mut u64;
std::ptr::write_unaligned(ptr, len as u64);
at += mem::size_of::<u64>();
}
at += mem::size_of::<u64>();

let data = &self.map[at..at + mem::size_of::<u64>()];
#[allow(clippy::cast_ptr_alignment)]
let ptr = data.as_ptr() as *mut u64;
let data = &self.map[at..at + mem::size_of::<u64>()];
#[allow(clippy::cast_ptr_alignment)]
let ptr = data.as_ptr() as *mut u64;
unsafe {
std::ptr::write_unaligned(ptr, account.tokens);
at += mem::size_of::<u64>();
}
at += mem::size_of::<u64>();

let data = &self.map[at..at + account.userdata.len()];
let dst = data.as_ptr() as *mut u8;
let data = &account.userdata[0..account.userdata.len()];
let src = data.as_ptr();
let data = &self.map[at..at + account.userdata.len()];
let dst = data.as_ptr() as *mut u8;
let data = &account.userdata[0..account.userdata.len()];
let src = data.as_ptr();
unsafe {
std::ptr::copy_nonoverlapping(src, dst, account.userdata.len());
at += account.userdata.len();
}
at += account.userdata.len();

let data = &self.map[at..at + mem::size_of::<Pubkey>()];
let ptr = data.as_ptr() as *mut Pubkey;
let data = &self.map[at..at + mem::size_of::<Pubkey>()];
let ptr = data.as_ptr() as *mut Pubkey;
unsafe {
std::ptr::write(ptr, account.owner);
at += mem::size_of::<Pubkey>();
}
at += mem::size_of::<Pubkey>();

let data = &self.map[at..at + mem::size_of::<bool>()];
let ptr = data.as_ptr() as *mut bool;
let data = &self.map[at..at + mem::size_of::<bool>()];
let ptr = data.as_ptr() as *mut bool;
unsafe {
std::ptr::write(ptr, account.executable);
};
}

self.current_offset
.fetch_add(len + SIZEOF_U64, Ordering::Relaxed);
Expand Down
6 changes: 3 additions & 3 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,12 @@ pub struct Bank {

impl Bank {
pub fn new(genesis_block: &GenesisBlock) -> Self {
Self::new_with_paths(&genesis_block, "")
Self::new_with_paths(&genesis_block, None)
}

pub fn new_with_paths(genesis_block: &GenesisBlock, paths: &str) -> Self {
pub fn new_with_paths(genesis_block: &GenesisBlock, paths: Option<String>) -> Self {
let mut bank = Self::default();
bank.accounts = Some(Arc::new(Accounts::new(bank.id, &paths)));
bank.accounts = Some(Arc::new(Accounts::new(bank.id, paths)));
bank.process_genesis_block(genesis_block);
bank.add_builtin_programs();
bank
Expand Down
6 changes: 3 additions & 3 deletions src/blocktree_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ pub fn process_blocktree(
genesis_block: &GenesisBlock,
blocktree: &Blocktree,
leader_scheduler: &Arc<RwLock<LeaderScheduler>>,
account_paths: &str,
account_paths: Option<String>,
) -> Result<(BankForks, Vec<BankForksInfo>)> {
let now = Instant::now();
info!("processing ledger...");
Expand Down Expand Up @@ -327,7 +327,7 @@ mod tests {
info!("last_fork2_entry_id: {:?}", last_fork2_entry_id);

let (mut bank_forks, bank_forks_info) =
process_blocktree(&genesis_block, &blocktree, &leader_scheduler, "").unwrap();
process_blocktree(&genesis_block, &blocktree, &leader_scheduler, None).unwrap();

assert_eq!(bank_forks_info.len(), 2); // There are two forks
assert_eq!(
Expand Down Expand Up @@ -455,7 +455,7 @@ mod tests {
entry_height += entries.len() as u64;

let (bank_forks, bank_forks_info) =
process_blocktree(&genesis_block, &blocktree, &leader_scheduler, "").unwrap();
process_blocktree(&genesis_block, &blocktree, &leader_scheduler, None).unwrap();

assert_eq!(bank_forks_info.len(), 1);
assert_eq!(
Expand Down
10 changes: 5 additions & 5 deletions src/fullnode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ pub struct FullnodeConfig {
pub storage_rotate_count: u64,
pub leader_scheduler_config: LeaderSchedulerConfig,
pub tick_config: PohServiceConfig,
pub account_paths: String,
pub account_paths: Option<String>,
}
impl Default for FullnodeConfig {
fn default() -> Self {
Expand All @@ -79,7 +79,7 @@ impl Default for FullnodeConfig {
storage_rotate_count: NUM_HASHES_FOR_STORAGE_ROTATE,
leader_scheduler_config: LeaderSchedulerConfig::default(),
tick_config: PohServiceConfig::default(),
account_paths: "".to_string(),
account_paths: None,
}
}
}
Expand Down Expand Up @@ -129,7 +129,7 @@ impl Fullnode {
let (bank_forks, bank_forks_info, blocktree, ledger_signal_receiver) =
new_banks_from_blocktree(
ledger_path,
&config.account_paths,
config.account_paths.clone(),
config.ticks_per_slot(),
&leader_scheduler,
);
Expand Down Expand Up @@ -384,7 +384,7 @@ impl Fullnode {

pub fn new_banks_from_blocktree(
blocktree_path: &str,
account_paths: &str,
account_paths: Option<String>,
ticks_per_slot: u64,
leader_scheduler: &Arc<RwLock<LeaderScheduler>>,
) -> (BankForks, Vec<BankForksInfo>, Blocktree, Receiver<bool>) {
Expand Down Expand Up @@ -726,7 +726,7 @@ mod tests {
let leader_scheduler = Arc::new(RwLock::new(LeaderScheduler::default()));
let (bank_forks, bank_forks_info, _, _) = new_banks_from_blocktree(
&validator_ledger_path,
"",
None,
DEFAULT_TICKS_PER_SLOT,
&leader_scheduler,
);
Expand Down
6 changes: 3 additions & 3 deletions src/replay_stage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ mod test {
{
// Set up the bank
let (bank_forks, bank_forks_info, blocktree, ledger_signal_receiver) =
new_banks_from_blocktree(&my_ledger_path, "", ticks_per_slot, &leader_scheduler);
new_banks_from_blocktree(&my_ledger_path, None, ticks_per_slot, &leader_scheduler);

// Set up the replay stage
let (rotation_sender, rotation_receiver) = channel();
Expand Down Expand Up @@ -636,7 +636,7 @@ mod test {
let leader_scheduler = Arc::new(RwLock::new(LeaderScheduler::default()));
let (bank_forks, bank_forks_info, blocktree, l_receiver) = new_banks_from_blocktree(
&my_ledger_path,
"",
None,
DEFAULT_TICKS_PER_SLOT,
&leader_scheduler,
);
Expand Down Expand Up @@ -745,7 +745,7 @@ mod test {
let exit = Arc::new(AtomicBool::new(false));
{
let (bank_forks, bank_forks_info, blocktree, l_receiver) =
new_banks_from_blocktree(&my_ledger_path, "", ticks_per_slot, &leader_scheduler);
new_banks_from_blocktree(&my_ledger_path, None, ticks_per_slot, &leader_scheduler);
let bank = bank_forks.working_bank();
let meta = blocktree
.meta(0)
Expand Down
2 changes: 1 addition & 1 deletion tests/multinode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -944,7 +944,7 @@ fn test_leader_to_validator_transition() {
info!("Check the ledger to make sure it's the right height...");
let bank_forks = new_banks_from_blocktree(
&leader_ledger_path,
"",
None,
DEFAULT_TICKS_PER_SLOT,
&Arc::new(RwLock::new(LeaderScheduler::default())),
)
Expand Down

0 comments on commit 68b6e6c

Please sign in to comment.