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 cleanup routine in the Lightning network graph code that removes old, stale channels and nodes during startup. The change switches from removing nodes one-by-one to removing them in bulk, cutting the time from over 7.5 seconds to about 340 milliseconds on a fast desktop. There is no indication this fixes a vulnerability or changes security behavior.
No security action required. Treat as a normal performance improvement. If reviewing for release notes, note it as a startup-time optimization for nodes with large gossip graphs.
Security signals we found
No security-relevant signals present in commit message or diff
Performance optimization only: O(n^2) individual removals replaced with bulk removal
No new input validation, authorization, or cryptographic logic introduced
No memory safety changes; Rust ownership patterns unchanged
No changes to network message handling or peer trust assumptions
Evidence from the diff
The patch refactors remove_stale_channels_and_tracking in lightning/src/routing/gossip.rs and adds a remove_bulk method to IndexedMap in lightning/src/util/indexed_map.rs. Previously, removing a channel could trigger immediate, individual removal of now-empty nodes via entry.remove_entry(), each requiring a linear scan of the IndexedMap::keys Vec. The new code collects node keys to remove via a callback, then calls nodes.remove_bulk(&nodes_to_remove), which removes all matching keys from the underlying HashMap in one pass and filters the keys Vec in a second pass. This is purely an algorithmic/performance improvement.
Changed components
lightning/src/routing/gossip.rslightning/src/util/indexed_map.rsNetworkGraph::remove_stale_channels_and_trackingIndexedMap::remove_bulkInspect captured patch +33 / −5
diff --git a/lightning/src/routing/gossip.rs b/lightning/src/routing/gossip.rs
index e5477ff..80ffbf9 100644
--- a/lightning/src/routing/gossip.rs
+++ b/lightning/src/routing/gossip.rs
@@ -37,7 +37,9 @@ use crate::ln::types::ChannelId;
use crate::routing::utxo::{self, UtxoLookup, UtxoResolver};
use crate::types::features::{ChannelFeatures, InitFeatures, NodeFeatures};
use crate::types::string::PrintableString;
-use crate::util::indexed_map::{Entry as IndexedMapEntry, IndexedMap};
+use crate::util::indexed_map::{
+ Entry as IndexedMapEntry, IndexedMap, OccupiedEntry as IndexedMapOccupiedEntry,
+};
use crate::util::logger::{Level, Logger};
use crate::util::scid_utils::{block_from_scid, scid_from_parts, MAX_SCID_BLOCK};
use crate::util::ser::{MaybeReadable, Readable, ReadableArgs, RequiredWrapper, Writeable, Writer};
@@ -2389,11 +2391,15 @@ where
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());
+ self.removed_node_counters.lock().unwrap().reserve(channels_removed_bulk.len());
+ let mut nodes_to_remove = hash_set_with_capacity(channels_removed_bulk.len());
for (scid, info) in channels_removed_bulk {
- self.remove_channel_in_nodes(&mut nodes, &info, scid);
+ self.remove_channel_in_nodes_callback(&mut nodes, &info, scid, |e| {
+ nodes_to_remove.insert(*e.key());
+ });
removed_channels_lck.insert(scid, Some(current_time_unix));
}
+ nodes.remove_bulk(&nodes_to_remove);
}
let should_keep_tracking = |time: &mut Option<u64>| {
@@ -2632,8 +2638,9 @@ where
Ok(())
}
- fn remove_channel_in_nodes(
+ fn remove_channel_in_nodes_callback<RM: FnMut(IndexedMapOccupiedEntry<NodeId, NodeInfo>)>(
&self, nodes: &mut IndexedMap<NodeId, NodeInfo>, chan: &ChannelInfo, short_channel_id: u64,
+ mut remove_node: RM,
) {
macro_rules! remove_from_node {
($node_id: expr) => {
@@ -2641,7 +2648,7 @@ where
entry.get_mut().channels.retain(|chan_id| short_channel_id != *chan_id);
if entry.get().channels.is_empty() {
self.removed_node_counters.lock().unwrap().push(entry.get().node_counter);
- entry.remove_entry();
+ remove_node(entry);
}
} else {
panic!(
@@ -2654,6 +2661,14 @@ where
remove_from_node!(chan.node_one);
remove_from_node!(chan.node_two);
}
+
+ fn remove_channel_in_nodes(
+ &self, nodes: &mut IndexedMap<NodeId, NodeInfo>, chan: &ChannelInfo, short_channel_id: u64,
+ ) {
+ self.remove_channel_in_nodes_callback(nodes, chan, short_channel_id, |e| {
+ e.remove_entry();
+ });
+ }
}
impl ReadOnlyNetworkGraph<'_> {
diff --git a/lightning/src/util/indexed_map.rs b/lightning/src/util/indexed_map.rs
index ae90cea..f811395 100644
--- a/lightning/src/util/indexed_map.rs
+++ b/lightning/src/util/indexed_map.rs
@@ -84,6 +84,14 @@ impl<K: Clone + Hash + Ord, V> IndexedMap<K, V> {
res
}
+ /// Removes elements with the given `keys` in bulk.
+ pub fn remove_bulk(&mut self, keys: &HashSet<K>) {
+ for key in keys.iter() {
+ self.map.remove(key);
+ }
+ self.keys.retain(|k| !keys.contains(k));
+ }
+
/// 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> {
@@ -222,6 +230,11 @@ impl<'a, K: Hash + Ord, V> OccupiedEntry<'a, K, V> {
res
}
+ /// Get a reference to the key at the position described by this entry.
+ pub fn key(&self) -> &K {
+ self.underlying_entry.key()
+ }
+
/// Get a reference to the value at the position described by this entry.
pub fn get(&self) -> &V {
self.underlying_entry.get()
Why this scored 21/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.