Skip to content

Commit 738b392

Browse files
committed
Swap IndexedMap implementation for a HashMap+B-Tree
Our network graph has to be iterable in a deterministic order and with the ability to iterate over a specific range. Thus, historically, we've used a `BTreeMap` to do the iteration. This is fine, except our map needs to also provide high performance lookups in order to make route-finding fast. Sadly, `BTreeMap`s are quite slow due to the branching penalty. Here we replace the implementation of our `IndexedMap` with a `HashMap` to store the elements itself and a `BTreeSet` to store the keys set in sorted order for iteration. As of this commit on the same hardware as the above few commits, the benchmark results are: ``` test routing::router::benches::generate_mpp_routes_with_probabilistic_scorer ... bench: 109,544,993 ns/iter (+/- 27,553,574) test routing::router::benches::generate_mpp_routes_with_zero_penalty_scorer ... bench: 81,164,590 ns/iter (+/- 55,422,930) test routing::router::benches::generate_routes_with_probabilistic_scorer ... bench: 34,726,569 ns/iter (+/- 9,646,345) test routing::router::benches::generate_routes_with_zero_penalty_scorer ... bench: 22,772,355 ns/iter (+/- 9,574,418) ```
1 parent ee9f4f2 commit 738b392

File tree

1 file changed

+68
-23
lines changed

1 file changed

+68
-23
lines changed

lightning/src/util/indexed_map.rs

+68-23
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
//! This module has a map which can be iterated in a deterministic order. See the [`IndexedMap`].
22
3-
use alloc::collections::{BTreeMap, btree_map};
3+
use crate::prelude::{HashMap, hash_map};
4+
use alloc::collections::{BTreeSet, btree_set};
5+
use core::hash::Hash;
46
use core::cmp::Ord;
57
use core::ops::RangeBounds;
68

@@ -19,16 +21,19 @@ use core::ops::RangeBounds;
1921
/// keys in the order defined by [`Ord`].
2022
///
2123
/// [`BTreeMap`]: alloc::collections::BTreeMap
22-
#[derive(Clone, PartialEq, Eq)]
23-
pub struct IndexedMap<K: Ord, V> {
24-
map: BTreeMap<K, V>,
24+
#[derive(Clone, Debug, PartialEq, Eq)]
25+
pub struct IndexedMap<K: Hash + Ord, V> {
26+
map: HashMap<K, V>,
27+
// TODO: Explore swapping this for a sorted vec (that is only sorted on first range() call)
28+
keys: BTreeSet<K>,
2529
}
2630

27-
impl<K: Ord, V> IndexedMap<K, V> {
31+
impl<K: Clone + Hash + Ord, V> IndexedMap<K, V> {
2832
/// Constructs a new, empty map
2933
pub fn new() -> Self {
3034
Self {
31-
map: BTreeMap::new(),
35+
map: HashMap::new(),
36+
keys: BTreeSet::new(),
3237
}
3338
}
3439

@@ -51,26 +56,37 @@ impl<K: Ord, V> IndexedMap<K, V> {
5156

5257
/// Removes the element with the given `key`, returning it, if one exists.
5358
pub fn remove(&mut self, key: &K) -> Option<V> {
54-
self.map.remove(key)
59+
let ret = self.map.remove(key);
60+
if let Some(_) = ret {
61+
assert!(self.keys.remove(key), "map and keys must be consistent");
62+
}
63+
ret
5564
}
5665

5766
/// Inserts the given `key`/`value` pair into the map, returning the element that was
5867
/// previously stored at the given `key`, if one exists.
5968
pub fn insert(&mut self, key: K, value: V) -> Option<V> {
60-
self.map.insert(key, value)
69+
let ret = self.map.insert(key.clone(), value);
70+
if ret.is_none() {
71+
assert!(self.keys.insert(key), "map and keys must be consistent");
72+
}
73+
ret
6174
}
6275

6376
/// Returns an [`Entry`] for the given `key` in the map, allowing access to the value.
6477
pub fn entry(&mut self, key: K) -> Entry<'_, K, V> {
65-
match self.map.entry(key) {
66-
btree_map::Entry::Vacant(entry) => {
78+
match self.map.entry(key.clone()) {
79+
hash_map::Entry::Vacant(entry) => {
6780
Entry::Vacant(VacantEntry {
68-
underlying_entry: entry
81+
underlying_entry: entry,
82+
key,
83+
keys: &mut self.keys,
6984
})
7085
},
71-
btree_map::Entry::Occupied(entry) => {
86+
hash_map::Entry::Occupied(entry) => {
7287
Entry::Occupied(OccupiedEntry {
73-
underlying_entry: entry
88+
underlying_entry: entry,
89+
keys: &mut self.keys,
7490
})
7591
}
7692
}
@@ -93,8 +109,11 @@ impl<K: Ord, V> IndexedMap<K, V> {
93109
}
94110

95111
/// Returns an iterator which iterates over the `key`/`value` pairs in a given range.
96-
pub fn range<R: RangeBounds<K>>(&self, range: R) -> btree_map::Range<K, V> {
97-
self.map.range(range)
112+
pub fn range<R: RangeBounds<K>>(&self, range: R) -> Range<K, V> {
113+
Range {
114+
inner_range: self.keys.range(range),
115+
map: &self.map,
116+
}
98117
}
99118

100119
/// Returns the number of `key`/`value` pairs in the map
@@ -108,36 +127,62 @@ impl<K: Ord, V> IndexedMap<K, V> {
108127
}
109128
}
110129

130+
/// An iterator over a range of values in an [`IndexedMap`]
131+
pub struct Range<'a, K: Hash + Ord, V> {
132+
inner_range: btree_set::Range<'a, K>,
133+
map: &'a HashMap<K, V>,
134+
}
135+
impl<'a, K: Hash + Ord, V: 'a> Iterator for Range<'a, K, V> {
136+
type Item = (&'a K, &'a V);
137+
fn next(&mut self) -> Option<(&'a K, &'a V)> {
138+
self.inner_range.next().map(|k| {
139+
(k, self.map.get(k).expect("map and keys must be consistent"))
140+
})
141+
}
142+
}
143+
111144
/// An [`Entry`] for a key which currently has no value
112-
pub struct VacantEntry<'a, K: Ord, V> {
113-
underlying_entry: btree_map::VacantEntry<'a, K, V>,
145+
pub struct VacantEntry<'a, K: Hash + Ord, V> {
146+
#[cfg(feature = "hashbrown")]
147+
underlying_entry: hash_map::VacantEntry<'a, K, V, hash_map::DefaultHashBuilder>,
148+
#[cfg(not(feature = "hashbrown"))]
149+
underlying_entry: hash_map::VacantEntry<'a, K, V>,
150+
key: K,
151+
keys: &'a mut BTreeSet<K>,
114152
}
115153

116154
/// An [`Entry`] for an existing key-value pair
117-
pub struct OccupiedEntry<'a, K: Ord, V> {
118-
underlying_entry: btree_map::OccupiedEntry<'a, K, V>,
155+
pub struct OccupiedEntry<'a, K: Hash + Ord, V> {
156+
#[cfg(feature = "hashbrown")]
157+
underlying_entry: hash_map::OccupiedEntry<'a, K, V, hash_map::DefaultHashBuilder>,
158+
#[cfg(not(feature = "hashbrown"))]
159+
underlying_entry: hash_map::OccupiedEntry<'a, K, V>,
160+
keys: &'a mut BTreeSet<K>,
119161
}
120162

121163
/// A mutable reference to a position in the map. This can be used to reference, add, or update the
122164
/// value at a fixed key.
123-
pub enum Entry<'a, K: Ord, V> {
165+
pub enum Entry<'a, K: Hash + Ord, V> {
124166
/// A mutable reference to a position within the map where there is no value.
125167
Vacant(VacantEntry<'a, K, V>),
126168
/// A mutable reference to a position within the map where there is currently a value.
127169
Occupied(OccupiedEntry<'a, K, V>),
128170
}
129171

130-
impl<'a, K: Ord, V> VacantEntry<'a, K, V> {
172+
impl<'a, K: Hash + Ord, V> VacantEntry<'a, K, V> {
131173
/// Insert a value into the position described by this entry.
132174
pub fn insert(self, value: V) -> &'a mut V {
175+
assert!(self.keys.insert(self.key), "map and keys must be consistent");
133176
self.underlying_entry.insert(value)
134177
}
135178
}
136179

137-
impl<'a, K: Ord, V> OccupiedEntry<'a, K, V> {
180+
impl<'a, K: Hash + Ord, V> OccupiedEntry<'a, K, V> {
138181
/// Remove the value at the position described by this entry.
139182
pub fn remove_entry(self) -> (K, V) {
140-
self.underlying_entry.remove_entry()
183+
let res = self.underlying_entry.remove_entry();
184+
assert!(self.keys.remove(&res.0), "map and keys must be consistent");
185+
res
141186
}
142187

143188
/// Get a reference to the value at the position described by this entry.

0 commit comments

Comments
 (0)