From dfa62383427df8f23d32c856ca22f7cd2e20d354 Mon Sep 17 00:00:00 2001 From: carllin Date: Wed, 4 Sep 2019 01:49:42 -0700 Subject: [PATCH] Remove unnecessary construction of descendants (#5742) --- core/src/consensus.rs | 57 +++++++++++++++++++++++----------------- core/src/replay_stage.rs | 3 +-- 2 files changed, 34 insertions(+), 26 deletions(-) diff --git a/core/src/consensus.rs b/core/src/consensus.rs index c369863e23..3891be2e1d 100644 --- a/core/src/consensus.rs +++ b/core/src/consensus.rs @@ -259,22 +259,26 @@ impl Tower { false } - pub fn is_locked_out(&self, slot: u64, descendants: &HashMap>) -> bool { + pub fn is_locked_out(&self, slot: u64, ancestors: &HashMap>) -> bool { + assert!(ancestors.contains_key(&slot)); + let mut lockouts = self.lockouts.clone(); lockouts.process_slot_vote_unchecked(slot); for vote in &lockouts.votes { if vote.slot == slot { continue; } - if !descendants[&vote.slot].contains(&slot) { + if !ancestors[&slot].contains(&vote.slot) { return true; } } if let Some(root) = lockouts.root_slot { - !descendants[&root].contains(&slot) - } else { - false + // This case should never happen because bank forks purges all + // non-descendants of the root every time root is set + assert!(ancestors[&slot].contains(&root)); } + + false } pub fn check_vote_stake_threshold( @@ -605,28 +609,29 @@ mod test { #[test] fn test_is_locked_out_empty() { let tower = Tower::new_for_tests(0, 0.67); - let descendants = HashMap::new(); - assert!(!tower.is_locked_out(0, &descendants)); + let ancestors = vec![(0, HashSet::new())].into_iter().collect(); + assert!(!tower.is_locked_out(0, &ancestors)); } #[test] fn test_is_locked_out_root_slot_child_pass() { let mut tower = Tower::new_for_tests(0, 0.67); - let descendants = vec![(0, vec![1].into_iter().collect())] + let ancestors = vec![(1, vec![0].into_iter().collect())] .into_iter() .collect(); tower.lockouts.root_slot = Some(0); - assert!(!tower.is_locked_out(1, &descendants)); + assert!(!tower.is_locked_out(1, &ancestors)); } #[test] fn test_is_locked_out_root_slot_sibling_fail() { let mut tower = Tower::new_for_tests(0, 0.67); - let descendants = vec![(0, vec![1].into_iter().collect())] + let ancestors = vec![(2, vec![0].into_iter().collect())] .into_iter() .collect(); tower.lockouts.root_slot = Some(0); - assert!(tower.is_locked_out(2, &descendants)); + tower.record_vote(1, Hash::default()); + assert!(tower.is_locked_out(2, &ancestors)); } #[test] @@ -640,48 +645,52 @@ mod test { #[test] fn test_is_locked_out_double_vote() { let mut tower = Tower::new_for_tests(0, 0.67); - let descendants = vec![(0, vec![1].into_iter().collect()), (1, HashSet::new())] + let ancestors = vec![(1, vec![0].into_iter().collect()), (0, HashSet::new())] .into_iter() .collect(); tower.record_vote(0, Hash::default()); tower.record_vote(1, Hash::default()); - assert!(tower.is_locked_out(0, &descendants)); + assert!(tower.is_locked_out(0, &ancestors)); } #[test] fn test_is_locked_out_child() { let mut tower = Tower::new_for_tests(0, 0.67); - let descendants = vec![(0, vec![1].into_iter().collect())] + let ancestors = vec![(1, vec![0].into_iter().collect())] .into_iter() .collect(); tower.record_vote(0, Hash::default()); - assert!(!tower.is_locked_out(1, &descendants)); + assert!(!tower.is_locked_out(1, &ancestors)); } #[test] fn test_is_locked_out_sibling() { let mut tower = Tower::new_for_tests(0, 0.67); - let descendants = vec![ - (0, vec![1, 2].into_iter().collect()), - (1, HashSet::new()), - (2, HashSet::new()), + let ancestors = vec![ + (0, HashSet::new()), + (1, vec![0].into_iter().collect()), + (2, vec![0].into_iter().collect()), ] .into_iter() .collect(); tower.record_vote(0, Hash::default()); tower.record_vote(1, Hash::default()); - assert!(tower.is_locked_out(2, &descendants)); + assert!(tower.is_locked_out(2, &ancestors)); } #[test] fn test_is_locked_out_last_vote_expired() { let mut tower = Tower::new_for_tests(0, 0.67); - let descendants = vec![(0, vec![1, 4].into_iter().collect()), (1, HashSet::new())] - .into_iter() - .collect(); + let ancestors = vec![ + (0, HashSet::new()), + (1, vec![0].into_iter().collect()), + (4, vec![0].into_iter().collect()), + ] + .into_iter() + .collect(); tower.record_vote(0, Hash::default()); tower.record_vote(1, Hash::default()); - assert!(!tower.is_locked_out(4, &descendants)); + assert!(!tower.is_locked_out(4, &ancestors)); tower.record_vote(4, Hash::default()); assert_eq!(tower.lockouts.votes[0].slot, 0); assert_eq!(tower.lockouts.votes[0].confirmation_count, 2); diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index 4d2128f147..05215424c9 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -573,7 +573,6 @@ impl ReplayStage { ) -> Vec<(u128, Arc, HashMap, u64)> { let tower_start = Instant::now(); // Tower voting - let descendants = bank_forks.read().unwrap().descendants(); let ancestors = bank_forks.read().unwrap().ancestors(); let frozen_banks = bank_forks.read().unwrap().frozen_banks(); @@ -591,7 +590,7 @@ impl ReplayStage { !has_voted }) .filter(|b| { - let is_locked_out = tower.is_locked_out(b.slot(), &descendants); + let is_locked_out = tower.is_locked_out(b.slot(), &ancestors); trace!("bank is is_locked_out: {} {}", b.slot(), is_locked_out); !is_locked_out })