Skip to content

Commit

Permalink
perf: prevent unnecessary boxing in 'copath()' and use iterators
Browse files Browse the repository at this point in the history
  • Loading branch information
beltram committed Jan 8, 2024
1 parent 2863e79 commit 7ad3f91
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 21 deletions.
2 changes: 1 addition & 1 deletion openmls/src/binary_tree/array_representation/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ impl<'a, L: Clone + Debug + Default, P: Clone + Debug + Default> AbDiff<'a, L, P
}

/// Returns the copath of a leaf node.
pub(crate) fn copath(&self, leaf_index: LeafNodeIndex) -> Vec<TreeNodeIndex> {
pub(crate) fn copath(&self, leaf_index: LeafNodeIndex) -> impl Iterator<Item = TreeNodeIndex> {
copath(leaf_index, self.size())
}

Expand Down
37 changes: 20 additions & 17 deletions openmls/src/binary_tree/array_representation/treemath.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,10 @@ impl TreeSize {
self.0 = 0;
}
}

pub(crate) fn height(&self) -> usize {
log2(self.0)
}
}

#[test]
Expand Down Expand Up @@ -363,33 +367,32 @@ pub(crate) fn test_sibling(index: TreeNodeIndex) -> TreeNodeIndex {
pub(crate) fn direct_path(node_index: LeafNodeIndex, size: TreeSize) -> Vec<ParentNodeIndex> {
let r = root(size).u32();

let mut d = vec![];
let mut direct_path = Vec::with_capacity(size.height());
let mut x = node_index.to_tree_index();
while x != r {
let parent = parent(TreeNodeIndex::new(x));
d.push(parent);
direct_path.push(parent);
x = parent.to_tree_index();
}
d
direct_path
}

/// Copath of a leaf node.
pub(crate) fn copath(leaf_index: LeafNodeIndex, size: TreeSize) -> Vec<TreeNodeIndex> {
pub(crate) fn copath(
leaf_index: LeafNodeIndex,
size: TreeSize,
) -> impl Iterator<Item = TreeNodeIndex> {
// Start with leaf
let mut full_path = vec![TreeNodeIndex::Leaf(leaf_index)];
let mut full_path = Vec::with_capacity(size.height());
full_path.push(TreeNodeIndex::Leaf(leaf_index));
let mut direct_path = direct_path(leaf_index, size);
if !direct_path.is_empty() {
// Remove root
direct_path.pop();
direct_path.pop(); // Remove root
}
full_path.append(
&mut direct_path
.iter()
.map(|i| TreeNodeIndex::Parent(*i))
.collect(),
);
let parent_direct_path = direct_path.into_iter().map(TreeNodeIndex::Parent);
full_path.extend(parent_direct_path);

full_path.into_iter().map(sibling).collect()
full_path.into_iter().map(sibling)
}

/// Common ancestor of two leaf nodes, aka the node where their direct paths
Expand Down Expand Up @@ -426,7 +429,7 @@ pub(crate) fn common_direct_path(
x_path.reverse();
y_path.reverse();

let mut common_path = vec![];
let mut common_path = Vec::with_capacity(size.height());

for (x, y) in x_path.iter().zip(y_path.iter()) {
if x == y {
Expand Down Expand Up @@ -459,7 +462,7 @@ fn test_node_in_tree() {
for test in tests.iter() {
assert!(is_node_in_tree(
TreeNodeIndex::new(test.0),
TreeSize::new(test.1)
TreeSize::new(test.1),
));
}
}
Expand All @@ -470,7 +473,7 @@ fn test_node_not_in_tree() {
for test in tests.iter() {
assert!(!is_node_in_tree(
TreeNodeIndex::new(test.0),
TreeSize::new(test.1)
TreeSize::new(test.1),
));
}
}
4 changes: 1 addition & 3 deletions openmls/src/treesync/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,9 @@ impl<'a> TreeSyncDiff<'a> {
let copath_resolutions = self.copath_resolutions(leaf_index);

// The two vectors should have the same length
debug_assert_eq!(copath.len(), copath_resolutions.len());
// debug_assert_eq!(copath.cloned().count(), copath_resolutions.len());

copath
.into_iter()
.zip(copath_resolutions)
.filter_map(|(index, resolution)| {
// Filter out the nodes whose copath resolution is empty
Expand Down Expand Up @@ -539,7 +538,6 @@ impl<'a> TreeSyncDiff<'a> {
// each node.
self.diff
.copath(leaf_index)
.into_iter()
.map(|node_index| self.resolution(node_index, &HashSet::new()))
.collect()
}
Expand Down

0 comments on commit 7ad3f91

Please sign in to comment.