Speed up `remove_stale_channels_and_tracking` nontrivially
What changed, and why it matters
This commit is a performance optimization, not a security fix. It speeds up a startup cleanup routine in the Lightning routing code by removing many stale channels in one pass instead of one at a time. The change reduces a reported 7.5-second freeze to about 1.4 seconds on a high-end CPU. There is no indication it fixes a vulnerability or changes security behavior.
No security action required. Treat as a normal performance improvement during review/merge.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch refactors remove_stale_channels_and_tracking in lightning/src/routing/gossip.rs to collect stale SCIDs into a HashSet and call a new IndexedMap::remove_fetch_bulk method. The new method removes matching entries from the inner HashMap and then uses a single Vec::retain pass to compact the ordered keys vector, avoiding repeated O(n) linear scans and shifts. The functional result—removing stale channels and recording them in removed_channels—is unchanged; only the algorithmic complexity and locking pattern differ slightly.
Changed components
lightning/src/routing/gossip.rslightning/src/util/indexed_map.rsInspect captured patch +20 / −9
diff --git a/lightning/src/routing/gossip.rs b/lightning/src/routing/gossip.rs
index 5be09d7..e5477ff 100644
--- a/lightning/src/routing/gossip.rs
+++ b/lightning/src/routing/gossip.rs
@@ -2356,9 +2356,7 @@ where
return;
}
let min_time_unix: u32 = (current_time_unix - STALE_CHANNEL_UPDATE_AGE_LIMIT_SECS) as u32;
- // Sadly BTreeMap::retain was only stabilized in 1.53 so we can't switch to it for some
- // time.
- let mut scids_to_remove = Vec::new();
+ let mut scids_to_remove = new_hash_set();
for (scid, info) in channels.unordered_iter_mut() {
if info.one_to_two.is_some()
&& info.one_to_two.as_ref().unwrap().last_update < min_time_unix
@@ -2382,18 +2380,19 @@ where
if announcement_received_timestamp < min_time_unix as u64 {
log_gossip!(self.logger, "Removing channel {} because both directional updates are missing and its announcement timestamp {} being below {}",
scid, announcement_received_timestamp, min_time_unix);
- scids_to_remove.push(*scid);
+ scids_to_remove.insert(*scid);
}
}
}
if !scids_to_remove.is_empty() {
let mut nodes = self.nodes.write().unwrap();
- for scid in scids_to_remove {
- let info = channels
- .remove(&scid)
- .expect("We just accessed this scid, it should be present");
+ let mut removed_channels_lck = self.removed_channels.lock().unwrap();
+
+ let channels_removed_bulk = channels.remove_fetch_bulk(&scids_to_remove);
+ removed_channels_lck.reserve(channels_removed_bulk.len());
+ for (scid, info) in channels_removed_bulk {
self.remove_channel_in_nodes(&mut nodes, &info, scid);
- self.removed_channels.lock().unwrap().insert(scid, Some(current_time_unix));
+ removed_channels_lck.insert(scid, Some(current_time_unix));
}
}
diff --git a/lightning/src/util/indexed_map.rs b/lightning/src/util/indexed_map.rs
index 3f8b357..ae90cea 100644
--- a/lightning/src/util/indexed_map.rs
+++ b/lightning/src/util/indexed_map.rs
@@ -72,6 +72,18 @@ impl<K: Clone + Hash + Ord, V> IndexedMap<K, V> {
ret
}
+ /// Removes elements with the given `keys` in bulk, returning the set of removed elements.
+ pub fn remove_fetch_bulk(&mut self, keys: &HashSet<K>) -> Vec<(K, V)> {
+ let mut res = Vec::with_capacity(keys.len());
+ for key in keys.iter() {
+ if let Some((k, v)) = self.map.remove_entry(key) {
+ res.push((k, v));
+ }
+ }
+ self.keys.retain(|k| !keys.contains(k));
+ res
+ }
+
/// Inserts the given `key`/`value` pair into the map, returning the element that was
/// previously stored at the given `key`, if one exists.
pub fn insert(&mut self, key: K, value: V) -> Option<V> {
Why this scored 20/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.