Skip to content

Commit

Permalink
fix(chain): avoid using BTreeMap::append
Browse files Browse the repository at this point in the history
The implementation of `BTreeMap::append` is non-performant making
merging changesets very slow. We use `Extend::extend` instead.

Refer to:
rust-lang/rust#34666 (comment)
  • Loading branch information
evanlinjin committed Jan 11, 2024
1 parent 8ef25db commit 04809e9
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 10 deletions.
5 changes: 3 additions & 2 deletions crates/chain/src/keychain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,9 @@ impl<K: Ord> Append for ChangeSet<K> {
*index = other_index.max(*index);
}
});

self.0.append(&mut other.0);
// We use `extend` instead of `BTreeMap::append` due to performance issues with `append`.
// Refer to https://github.com/rust-lang/rust/issues/34666#issuecomment-675658420
self.0.extend(other.0);
}

/// Returns whether the changeset are empty.
Expand Down
12 changes: 8 additions & 4 deletions crates/chain/src/tx_data_traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,10 @@ pub trait Append {
}

impl<K: Ord, V> Append for BTreeMap<K, V> {
fn append(&mut self, mut other: Self) {
BTreeMap::append(self, &mut other)
fn append(&mut self, other: Self) {
// We use `extend` instead of `BTreeMap::append` due to performance issues with `append`.
// Refer to https://github.com/rust-lang/rust/issues/34666#issuecomment-675658420
BTreeMap::extend(self, other)
}

fn is_empty(&self) -> bool {
Expand All @@ -133,8 +135,10 @@ impl<K: Ord, V> Append for BTreeMap<K, V> {
}

impl<T: Ord> Append for BTreeSet<T> {
fn append(&mut self, mut other: Self) {
BTreeSet::append(self, &mut other)
fn append(&mut self, other: Self) {
// We use `extend` instead of `BTreeMap::append` due to performance issues with `append`.
// Refer to https://github.com/rust-lang/rust/issues/34666#issuecomment-675658420
BTreeSet::extend(self, other)
}

fn is_empty(&self) -> bool {
Expand Down
10 changes: 6 additions & 4 deletions crates/chain/src/tx_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1271,10 +1271,12 @@ impl<A> ChangeSet<A> {
}

impl<A: Ord> Append for ChangeSet<A> {
fn append(&mut self, mut other: Self) {
self.txs.append(&mut other.txs);
self.txouts.append(&mut other.txouts);
self.anchors.append(&mut other.anchors);
fn append(&mut self, other: Self) {
// We use `extend` instead of `BTreeMap::append` due to performance issues with `append`.
// Refer to https://github.com/rust-lang/rust/issues/34666#issuecomment-675658420
self.txs.extend(other.txs);
self.txouts.extend(other.txouts);
self.anchors.extend(other.anchors);

// last_seen timestamps should only increase
self.last_seen.extend(
Expand Down

0 comments on commit 04809e9

Please sign in to comment.