Skip to content

Commit ddec9ae

Browse files
committed
replace some btreemaps with hashmaps
This allows us to use `with_capacity` which hopefully prevents allocations
1 parent bbe27dd commit ddec9ae

File tree

3 files changed

+51
-26
lines changed

3 files changed

+51
-26
lines changed

core/src/bls_sigverify/bls_sigverifier.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,10 @@ impl BLSSigVerifier {
256256
let mut guard = self.consensus_rewards.write();
257257
let root_slot = root_bank.slot();
258258
for v in rewards_votes {
259-
guard.add_vote_message(root_slot, v);
259+
if let Some(rank_map) = get_key_to_rank_map(&root_bank, v.vote.slot()) {
260+
let max_validators = rank_map.len();
261+
guard.add_vote(root_slot, max_validators, v);
262+
}
260263
}
261264
}
262265

@@ -306,7 +309,7 @@ impl BLSSigVerifier {
306309
.filter_map(|vote| {
307310
guard
308311
.wants_vote(root_slot, &vote.vote_message)
309-
.then_some(vote.vote_message.clone())
312+
.then_some(vote.vote_message)
310313
})
311314
.collect()
312315
};

votor/src/consensus_rewards.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ impl ConsensusRewards {
5858
}
5959

6060
/// Adds received [`VoteMessage`]s from other nodes.
61-
pub fn add_vote_message(&mut self, root_slot: Slot, vote: VoteMessage) {
61+
pub fn add_vote(&mut self, root_slot: Slot, max_validators: usize, vote: VoteMessage) {
6262
// drop old state no longer needed
6363
self.votes = self.votes.split_off(
6464
&(root_slot
@@ -72,7 +72,7 @@ impl ConsensusRewards {
7272
}
7373
self.votes
7474
.entry(vote.vote.slot())
75-
.or_default()
75+
.or_insert(Entry::new(max_validators))
7676
.add_vote(vote);
7777
}
7878

votor/src/consensus_rewards/entry.rs

Lines changed: 44 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,14 @@ use {
99
rewards_certificate::{NotarRewardCertificate, SkipRewardCertificate},
1010
vote::Vote,
1111
},
12-
std::collections::BTreeMap,
12+
std::collections::HashMap,
1313
};
1414

1515
/// Builds a signature and bitmap suitable for creating a rewards certificate.
16-
fn build_sig_bitmap(votes: &BTreeMap<u16, VoteMessage>) -> Option<(BLSSignature, Vec<u8>)> {
17-
let max_rank = votes.last_key_value().map(|(rank, _)| rank).cloned()?;
16+
fn build_sig_bitmap(
17+
votes: &HashMap<u16, VoteMessage>,
18+
max_rank: u16,
19+
) -> Option<(BLSSignature, Vec<u8>)> {
1820
let mut bitmap = BitVec::repeat(false, max_rank as usize);
1921
for vote in votes.keys() {
2022
bitmap.set(*vote as usize, true);
@@ -29,20 +31,35 @@ fn build_sig_bitmap(votes: &BTreeMap<u16, VoteMessage>) -> Option<(BLSSignature,
2931
}
3032

3133
/// Per slot container for storing notar and skip votes for creating rewards certificates.
32-
#[derive(Default)]
3334
pub(super) struct Entry {
34-
/// map from validator rank to the skip vote.
35-
skip: BTreeMap<u16, VoteMessage>,
36-
/// notar votes are indexed by block id as different validators may vote for different blocks.
37-
notar: BTreeMap<Hash, BTreeMap<u16, VoteMessage>>,
35+
/// Map from validator rank to the skip vote.
36+
skip: HashMap<u16, VoteMessage>,
37+
/// Largest ranked validator that voted skip.
38+
skip_max_rank: u16,
39+
/// Notar votes are indexed by block id as different validators may vote for different blocks.
40+
/// Per block id, store a map from validator rank to notar vote and also the largest ranked validator that voted notar.
41+
notar: HashMap<Hash, (HashMap<u16, VoteMessage>, u16)>,
42+
/// Maximum number of validators for the slot this entry is working on.
43+
max_validators: usize,
3844
}
3945

4046
impl Entry {
47+
/// Creates a new instance of [`Entry`].
48+
pub(super) fn new(max_validators: usize) -> Self {
49+
Self {
50+
skip: HashMap::with_capacity(max_validators),
51+
skip_max_rank: 0,
52+
// under normal operations, all validators should vote for a single block id, still allocate space for a few more to hopefully avoid allocations.
53+
notar: HashMap::with_capacity(5),
54+
max_validators,
55+
}
56+
}
57+
4158
pub(super) fn wants_vote(&self, vote: &VoteMessage) -> bool {
4259
match vote.vote {
4360
Vote::Skip(_) => !self.skip.contains_key(&vote.rank),
4461
Vote::Notarize(notar) => {
45-
let Some(notar) = self.notar.get(&notar.block_id) else {
62+
let Some((notar, _)) = self.notar.get(&notar.block_id) else {
4663
return true;
4764
};
4865
!notar.contains_key(&vote.rank)
@@ -57,13 +74,16 @@ impl Entry {
5774
pub(super) fn add_vote(&mut self, vote: VoteMessage) {
5875
match vote.vote {
5976
Vote::Notarize(notar) => {
60-
self.notar
77+
let (map, max_rank) = self
78+
.notar
6179
.entry(notar.block_id)
62-
.or_default()
63-
.insert(vote.rank, vote);
80+
.or_insert((HashMap::with_capacity(self.max_validators), 0));
81+
map.insert(vote.rank, vote);
82+
*max_rank = (*max_rank).max(vote.rank);
6483
}
6584
Vote::Skip(_) => {
6685
self.skip.insert(vote.rank, vote);
86+
self.skip_max_rank = self.skip_max_rank.max(vote.rank);
6787
}
6888
_ => (),
6989
}
@@ -76,29 +96,31 @@ impl Entry {
7696
Option<SkipRewardCertificate>,
7797
Option<NotarRewardCertificate>,
7898
) {
79-
let skip = build_sig_bitmap(&self.skip).map(|(signature, bitmap)| SkipRewardCertificate {
80-
slot,
81-
signature,
82-
bitmap,
99+
let skip = build_sig_bitmap(&self.skip, self.skip_max_rank).map(|(signature, bitmap)| {
100+
SkipRewardCertificate {
101+
slot,
102+
signature,
103+
bitmap,
104+
}
83105
});
84106

85107
// we can only submit one notar rewards certificate but different validators may vote for different blocks and we cannot combine notar votes for different blocks together in one cert.
86108
// pick the block_id with most votes.
87109
// in practice, all validators should have voted for the same block otherwise, we have evidence of leader equivocation.
88110
// TODO: collect metrics for when equivocation is detected.
89111
let mut notar = None;
90-
for (block_id, map) in &self.notar {
112+
for (block_id, (map, max_rank)) in &self.notar {
91113
match notar {
92-
None => notar = Some((block_id, map)),
93-
Some((_, notar_map)) => {
114+
None => notar = Some((block_id, map, max_rank)),
115+
Some((_, notar_map, _)) => {
94116
if map.len() > notar_map.len() {
95-
notar = Some((block_id, map));
117+
notar = Some((block_id, map, max_rank));
96118
}
97119
}
98120
}
99121
}
100-
let notar = notar.and_then(|(block_id, votes)| {
101-
build_sig_bitmap(votes).map(|(signature, bitmap)| NotarRewardCertificate {
122+
let notar = notar.and_then(|(block_id, votes, max_rank)| {
123+
build_sig_bitmap(votes, *max_rank).map(|(signature, bitmap)| NotarRewardCertificate {
102124
slot,
103125
block_id: *block_id,
104126
signature,

0 commit comments

Comments
 (0)