Skip to content

Commit

Permalink
removes unnecessary weight updates in WeightedShuffle::search (#4499)
Browse files Browse the repository at this point in the history
Below line is a left-over from the old code and is not needed any
more:
https://github.com/anza-xyz/agave/blob/1b9996194/gossip/src/weighted_shuffle.rs#L144

The commit also removes unnecessary trait constraints and adds bounds
check to WeightedShuffle::remove_index.
  • Loading branch information
behzadnouri authored Jan 16, 2025
1 parent 91d0d0c commit d3305e3
Showing 1 changed file with 10 additions and 8 deletions.
18 changes: 10 additions & 8 deletions gossip/src/weighted_shuffle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use {
distributions::uniform::{SampleUniform, UniformSampler},
Rng,
},
std::ops::{AddAssign, Sub, SubAssign},
std::ops::{AddAssign, SubAssign},
};

// Each internal tree node has FANOUT many child nodes with indices:
Expand Down Expand Up @@ -106,7 +106,7 @@ where

impl<T> WeightedShuffle<T>
where
T: Copy + ConstZero + PartialOrd + AddAssign + SubAssign + Sub<Output = T>,
T: Copy + ConstZero + PartialOrd + SubAssign,
{
// Removes given weight at index k.
fn remove(&mut self, k: usize, weight: T) {
Expand Down Expand Up @@ -140,8 +140,6 @@ where
index = (index << BIT_SHIFT) + j + 1;
break;
} else {
debug_assert!(weight >= node);
weight -= node;
val -= node;
}
}
Expand All @@ -153,10 +151,14 @@ where
let index = self.num_nodes + k; // leaf node
let offset = (index - 1) & BIT_MASK;
let index = (index - 1) >> BIT_SHIFT; // parent node
if self.tree[index][offset] == Self::ZERO {
let Some(weight) = self.tree.get(index).map(|node| node[offset]) else {
error!("WeightedShuffle::remove_index: Invalid index {k}");
return;
};
if weight == Self::ZERO {
self.remove_zero(k);
} else {
self.remove(k, self.tree[index][offset]);
self.remove(k, weight);
}
}

Expand All @@ -169,7 +171,7 @@ where

impl<T> WeightedShuffle<T>
where
T: Copy + ConstZero + PartialOrd + AddAssign + SampleUniform + SubAssign + Sub<Output = T>,
T: Copy + ConstZero + PartialOrd + SampleUniform + SubAssign,
{
// Equivalent to weighted_shuffle.shuffle(&mut rng).next()
pub fn first<R: Rng>(&self, rng: &mut R) -> Option<usize> {
Expand All @@ -188,7 +190,7 @@ where

impl<'a, T: 'a> WeightedShuffle<T>
where
T: Copy + ConstZero + PartialOrd + AddAssign + SampleUniform + SubAssign + Sub<Output = T>,
T: Copy + ConstZero + PartialOrd + SampleUniform + SubAssign,
{
pub fn shuffle<R: Rng>(mut self, rng: &'a mut R) -> impl Iterator<Item = usize> + 'a {
std::iter::from_fn(move || {
Expand Down

0 comments on commit d3305e3

Please sign in to comment.