Merge pull request #2803 from TheBlueMatt/2023-12-routing-dist-vec

TheBlueMatt · web-flow · commit ac1463b120eb · 2024-07-17T14:06:05.000Z
Misc routing optimization
diff --git a/lightning/src/ln/features.rs b/lightning/src/ln/features.rs
@@ -801,15 +801,23 @@ impl<T: sealed::Context> Features<T> {
 	pub fn requires_unknown_bits(&self) -> bool {
 		// Bitwise AND-ing with all even bits set except for known features will select required
 		// unknown features.
-		let byte_count = T::KNOWN_FEATURE_MASK.len();
-		self.flags.iter().enumerate().any(|(i, &byte)| {
-			let unknown_features = if i < byte_count {
-				!T::KNOWN_FEATURE_MASK[i]
-			} else {
-				0b11_11_11_11
-			};
-			(byte & (ANY_REQUIRED_FEATURES_MASK & unknown_features)) != 0
-		})
+		let mut known_chunks = T::KNOWN_FEATURE_MASK.chunks(8);
+		for chunk in self.flags.chunks(8) {
+			let mut flag_bytes = [0; 8];
+			flag_bytes[..chunk.len()].copy_from_slice(&chunk);
+			let flag_int = u64::from_le_bytes(flag_bytes);
+
+			let known_chunk = known_chunks.next().unwrap_or(&[0; 0]);
+			let mut known_bytes = [0; 8];
+			known_bytes[..known_chunk.len()].copy_from_slice(&known_chunk);
+			let known_int = u64::from_le_bytes(known_bytes);
+
+			const REQ_MASK: u64 = u64::from_le_bytes([ANY_REQUIRED_FEATURES_MASK; 8]);
+			if flag_int & (REQ_MASK & !known_int) != 0 {
+				return true;
+			}
+		}
+		false
 	}
 
 	pub(crate) fn supports_unknown_bits(&self) -> bool {
diff --git a/lightning/src/routing/gossip.rs b/lightning/src/routing/gossip.rs
@@ -795,22 +795,32 @@ where
 	}
 }
 
+// Fetching values from this struct is very performance sensitive during routefinding. Thus, we
+// want to ensure that all of the fields we care about (all of them except `last_update_message`)
+// sit on the same cache line.
+//
+// We do this by using `repr(C)`, which forces the struct to be laid out in memory the way we write
+// it (ensuring `last_update_message` hangs off the end and no fields are reordered after it), and
+// `align(32)`, ensuring the struct starts either at the start, or in the middle, of a 64b x86-64
+// cache line. This ensures the beginning fields (which are 31 bytes) all sit in the same cache
+// line.
+#[repr(C, align(32))]
 #[derive(Clone, Debug, PartialEq, Eq)]
 /// Details about one direction of a channel as received within a [`ChannelUpdate`].
 pub struct ChannelUpdateInfo {
-	/// When the last update to the channel direction was issued.
-	/// Value is opaque, as set in the announcement.
-	pub last_update: u32,
-	/// Whether the channel can be currently used for payments (in this one direction).
-	pub enabled: bool,
-	/// The difference in CLTV values that you must have when routing through this channel.
-	pub cltv_expiry_delta: u16,
 	/// The minimum value, which must be relayed to the next hop via the channel
 	pub htlc_minimum_msat: u64,
 	/// The maximum value which may be relayed to the next hop via the channel.
 	pub htlc_maximum_msat: u64,
 	/// Fees charged when the channel is used for routing
 	pub fees: RoutingFees,
+	/// When the last update to the channel direction was issued.
+	/// Value is opaque, as set in the announcement.
+	pub last_update: u32,
+	/// The difference in CLTV values that you must have when routing through this channel.
+	pub cltv_expiry_delta: u16,
+	/// Whether the channel can be currently used for payments (in this one direction).
+	pub enabled: bool,
 	/// Most recent update for the channel received from the network
 	/// Mostly redundant with the data we store in fields explicitly.
 	/// Everything else is useful only for sending out for initial routing sync.
@@ -878,22 +888,46 @@ impl Readable for ChannelUpdateInfo {
 	}
 }
 
+// Fetching values from this struct is very performance sensitive during routefinding. Thus, we
+// want to ensure that all of the fields we care about (all of them except `last_update_message`
+// and `announcement_received_time`) sit on the same cache line.
+//
+// Sadly, this is not possible, however we can still do okay - all of the fields before
+// `one_to_two` and `two_to_one` are just under 128 bytes long, so we can ensure they sit on
+// adjacent cache lines (which are generally fetched together in x86_64 processors).
+//
+// This leaves only the two directional channel info structs on separate cache lines.
+//
+// We accomplish this using `repr(C)`, which forces the struct to be laid out in memory the way we
+// write it (ensuring the fields we care about are at the start of the struct) and `align(128)`,
+// ensuring the struct starts at the beginning of two adjacent 64b x86-64 cache lines.
+#[repr(align(128), C)]
 #[derive(Clone, Debug, Eq)]
 /// Details about a channel (both directions).
 /// Received within a channel announcement.
 pub struct ChannelInfo {
 	/// Protocol features of a channel communicated during its announcement
 	pub features: ChannelFeatures,
+
 	/// Source node of the first direction of a channel
 	pub node_one: NodeId,
-	/// Details about the first direction of a channel
-	pub one_to_two: Option<ChannelUpdateInfo>,
+
 	/// Source node of the second direction of a channel
 	pub node_two: NodeId,
-	/// Details about the second direction of a channel
-	pub two_to_one: Option<ChannelUpdateInfo>,
+
+	/// The [`NodeInfo::node_counter`] of the node pointed to by [`Self::node_one`].
+	pub(crate) node_one_counter: u32,
+	/// The [`NodeInfo::node_counter`] of the node pointed to by [`Self::node_two`].
+	pub(crate) node_two_counter: u32,
+
 	/// The channel capacity as seen on-chain, if chain lookup is available.
 	pub capacity_sats: Option<u64>,
+
+	/// Details about the first direction of a channel
+	pub one_to_two: Option<ChannelUpdateInfo>,
+	/// Details about the second direction of a channel
+	pub two_to_one: Option<ChannelUpdateInfo>,
+
 	/// An initial announcement of the channel
 	/// Mostly redundant with the data we store in fields explicitly.
 	/// Everything else is useful only for sending out for initial routing sync.
@@ -903,11 +937,6 @@ pub struct ChannelInfo {
 	/// (which we can probably assume we are - no-std environments probably won't have a full
 	/// network graph in memory!).
 	announcement_received_time: u64,
-
-	/// The [`NodeInfo::node_counter`] of the node pointed to by [`Self::node_one`].
-	pub(crate) node_one_counter: u32,
-	/// The [`NodeInfo::node_counter`] of the node pointed to by [`Self::node_two`].
-	pub(crate) node_two_counter: u32,
 }
 
 impl PartialEq for ChannelInfo {
@@ -1053,6 +1082,8 @@ impl Readable for ChannelInfo {
 pub struct DirectedChannelInfo<'a> {
 	channel: &'a ChannelInfo,
 	direction: &'a ChannelUpdateInfo,
+	source_counter: u32,
+	target_counter: u32,
 	/// The direction this channel is in - if set, it indicates that we're traversing the channel
 	/// from [`ChannelInfo::node_one`] to [`ChannelInfo::node_two`].
 	from_node_one: bool,
@@ -1061,7 +1092,12 @@ pub struct DirectedChannelInfo<'a> {
 impl<'a> DirectedChannelInfo<'a> {
 	#[inline]
 	fn new(channel: &'a ChannelInfo, direction: &'a ChannelUpdateInfo, from_node_one: bool) -> Self {
-		Self { channel, direction, from_node_one }
+		let (source_counter, target_counter) = if from_node_one {
+			(channel.node_one_counter, channel.node_two_counter)
+		} else {
+			(channel.node_two_counter, channel.node_one_counter)
+		};
+		Self { channel, direction, from_node_one, source_counter, target_counter }
 	}
 
 	/// Returns information for the channel.
@@ -1104,12 +1140,12 @@ impl<'a> DirectedChannelInfo<'a> {
 	pub fn target(&self) -> &'a NodeId { if self.from_node_one { &self.channel.node_two } else { &self.channel.node_one } }
 
 	/// Returns the source node's counter
-	#[inline]
-	pub(super) fn source_counter(&self) -> u32 { if self.from_node_one { self.channel.node_one_counter } else { self.channel.node_two_counter } }
+	#[inline(always)]
+	pub(super) fn source_counter(&self) -> u32 { self.source_counter }
 
 	/// Returns the target node's counter
-	#[inline]
-	pub(super) fn target_counter(&self) -> u32 { if self.from_node_one { self.channel.node_two_counter } else { self.channel.node_one_counter } }
+	#[inline(always)]
+	pub(super) fn target_counter(&self) -> u32 { self.target_counter }
 }
 
 impl<'a> fmt::Debug for DirectedChannelInfo<'a> {
diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs
@@ -1437,7 +1437,7 @@ impl<'a> CandidateRouteHop<'a> {
 		}
 	}
 
-	#[inline]
+	#[inline(always)]
 	fn src_node_counter(&self) -> u32 {
 		match self {
 			CandidateRouteHop::FirstHop(hop) => hop.payer_node_counter,
@@ -1772,6 +1772,14 @@ struct PathBuildingHop<'a> {
 	/// decrease as well. Thus, we have to explicitly track which nodes have been processed and
 	/// avoid processing them again.
 	was_processed: bool,
+	/// When processing a node as the next best-score candidate, we want to quickly check if it is
+	/// a direct counterparty of ours, using our local channel information immediately if we can.
+	///
+	/// In order to do so efficiently, we cache whether a node is a direct counterparty here at the
+	/// start of a route-finding pass. Unlike all other fields in this struct, this field is never
+	/// updated after being initialized - it is set at the start of a route-finding pass and only
+	/// read thereafter.
+	is_first_hop_target: bool,
 	/// Used to compare channels when choosing the for routing.
 	/// Includes paying for the use of a hop and the following hops, as well as
 	/// an estimated cost of reaching this hop.
@@ -1810,6 +1818,7 @@ impl<'a> core::fmt::Debug for PathBuildingHop<'a> {
 			.field("source_node_id", &self.candidate.source())
 			.field("target_node_id", &self.candidate.target())
 			.field("short_channel_id", &self.candidate.short_channel_id())
+			.field("is_first_hop_target", &self.is_first_hop_target)
 			.field("total_fee_msat", &self.total_fee_msat)
 			.field("next_hops_fee_msat", &self.next_hops_fee_msat)
 			.field("hop_use_fee_msat", &self.hop_use_fee_msat)
@@ -2383,6 +2392,8 @@ where L::Target: Logger {
 				// if the amount being transferred over this path is lower.
 				// We do this for now, but this is a subject for removal.
 				if let Some(mut available_value_contribution_msat) = htlc_maximum_msat.checked_sub($next_hops_fee_msat) {
+					let cltv_expiry_delta = $candidate.cltv_expiry_delta();
+					let htlc_minimum_msat = $candidate.htlc_minimum_msat();
 					let used_liquidity_msat = used_liquidities
 						.get(&$candidate.id())
 						.map_or(0, |used_liquidity_msat| {
@@ -2406,7 +2417,7 @@ where L::Target: Logger {
 						.checked_sub(2*MEDIAN_HOP_CLTV_EXPIRY_DELTA)
 						.unwrap_or(payment_params.max_total_cltv_expiry_delta - final_cltv_expiry_delta);
 					let hop_total_cltv_delta = ($next_hops_cltv_delta as u32)
-						.saturating_add($candidate.cltv_expiry_delta());
+						.saturating_add(cltv_expiry_delta);
 					let exceeds_cltv_delta_limit = hop_total_cltv_delta > max_total_cltv_expiry_delta;
 
 					let value_contribution_msat = cmp::min(available_value_contribution_msat, $next_hops_value_contribution);
@@ -2417,13 +2428,13 @@ where L::Target: Logger {
 						None => unreachable!(),
 					};
 					#[allow(unused_comparisons)] // $next_hops_path_htlc_minimum_msat is 0 in some calls so rustc complains
-					let over_path_minimum_msat = amount_to_transfer_over_msat >= $candidate.htlc_minimum_msat() &&
+					let over_path_minimum_msat = amount_to_transfer_over_msat >= htlc_minimum_msat &&
 						amount_to_transfer_over_msat >= $next_hops_path_htlc_minimum_msat;
 
 					#[allow(unused_comparisons)] // $next_hops_path_htlc_minimum_msat is 0 in some calls so rustc complains
 					let may_overpay_to_meet_path_minimum_msat =
-						((amount_to_transfer_over_msat < $candidate.htlc_minimum_msat() &&
-						  recommended_value_msat >= $candidate.htlc_minimum_msat()) ||
+						((amount_to_transfer_over_msat < htlc_minimum_msat &&
+						  recommended_value_msat >= htlc_minimum_msat) ||
 						 (amount_to_transfer_over_msat < $next_hops_path_htlc_minimum_msat &&
 						  recommended_value_msat >= $next_hops_path_htlc_minimum_msat));
 
@@ -2493,12 +2504,14 @@ where L::Target: Logger {
 						// payment path (upstream to the payee). To avoid that, we recompute
 						// path fees knowing the final path contribution after constructing it.
 						let curr_min = cmp::max(
-							$next_hops_path_htlc_minimum_msat, $candidate.htlc_minimum_msat()
+							$next_hops_path_htlc_minimum_msat, htlc_minimum_msat
 						);
-						let path_htlc_minimum_msat = compute_fees_saturating(curr_min, $candidate.fees())
+						let candidate_fees = $candidate.fees();
+						let src_node_counter = $candidate.src_node_counter();
+						let path_htlc_minimum_msat = compute_fees_saturating(curr_min, candidate_fees)
 							.saturating_add(curr_min);
 
-						let dist_entry = &mut dist[$candidate.src_node_counter() as usize];
+						let dist_entry = &mut dist[src_node_counter as usize];
 						let old_entry = if let Some(hop) = dist_entry {
 							hop
 						} else {
@@ -2516,6 +2529,7 @@ where L::Target: Logger {
 								path_htlc_minimum_msat,
 								path_penalty_msat: u64::max_value(),
 								was_processed: false,
+								is_first_hop_target: false,
 								#[cfg(all(not(ldk_bench), any(test, fuzzing)))]
 								value_contribution_msat,
 							});
@@ -2540,7 +2554,7 @@ where L::Target: Logger {
 							if src_node_id != our_node_id {
 								// Note that `u64::max_value` means we'll always fail the
 								// `old_entry.total_fee_msat > total_fee_msat` check below
-								hop_use_fee_msat = compute_fees_saturating(amount_to_transfer_over_msat, $candidate.fees());
+								hop_use_fee_msat = compute_fees_saturating(amount_to_transfer_over_msat, candidate_fees);
 								total_fee_msat = total_fee_msat.saturating_add(hop_use_fee_msat);
 							}
 
@@ -2679,12 +2693,14 @@ where L::Target: Logger {
 			let fee_to_target_msat;
 			let next_hops_path_htlc_minimum_msat;
 			let next_hops_path_penalty_msat;
+			let is_first_hop_target;
 			let skip_node = if let Some(elem) = &mut dist[$node.node_counter as usize] {
 				let was_processed = elem.was_processed;
 				elem.was_processed = true;
 				fee_to_target_msat = elem.total_fee_msat;
 				next_hops_path_htlc_minimum_msat = elem.path_htlc_minimum_msat;
 				next_hops_path_penalty_msat = elem.path_penalty_msat;
+				is_first_hop_target = elem.is_first_hop_target;
 				was_processed
 			} else {
 				// Entries are added to dist in add_entry!() when there is a channel from a node.
@@ -2695,21 +2711,24 @@ where L::Target: Logger {
 				fee_to_target_msat = 0;
 				next_hops_path_htlc_minimum_msat = 0;
 				next_hops_path_penalty_msat = 0;
+				is_first_hop_target = false;
 				false
 			};
 
 			if !skip_node {
-				if let Some((first_channels, peer_node_counter)) = first_hop_targets.get(&$node_id) {
-					for details in first_channels {
-						debug_assert_eq!(*peer_node_counter, $node.node_counter);
-						let candidate = CandidateRouteHop::FirstHop(FirstHopCandidate {
-							details, payer_node_id: &our_node_id, payer_node_counter,
-							target_node_counter: $node.node_counter,
-						});
-						add_entry!(&candidate, fee_to_target_msat,
-							$next_hops_value_contribution,
-							next_hops_path_htlc_minimum_msat, next_hops_path_penalty_msat,
-							$next_hops_cltv_delta, $next_hops_path_length);
+				if is_first_hop_target {
+					if let Some((first_channels, peer_node_counter)) = first_hop_targets.get(&$node_id) {
+						for details in first_channels {
+							debug_assert_eq!(*peer_node_counter, $node.node_counter);
+							let candidate = CandidateRouteHop::FirstHop(FirstHopCandidate {
+								details, payer_node_id: &our_node_id, payer_node_counter,
+								target_node_counter: $node.node_counter,
+							});
+							add_entry!(&candidate, fee_to_target_msat,
+								$next_hops_value_contribution,
+								next_hops_path_htlc_minimum_msat, next_hops_path_penalty_msat,
+								$next_hops_cltv_delta, $next_hops_path_length);
+						}
 					}
 				}
 
@@ -2756,6 +2775,32 @@ where L::Target: Logger {
 		for e in dist.iter_mut() {
 			*e = None;
 		}
+		for (_, (chans, peer_node_counter)) in first_hop_targets.iter() {
+			// In order to avoid looking up whether each node is a first-hop target, we store a
+			// dummy entry in dist for each first-hop target, allowing us to do this lookup for
+			// free since we're already looking at the `was_processed` flag.
+			//
+			// Note that all the fields (except `is_first_hop_target`) will be overwritten whenever
+			// we find a path to the target, so are left as dummies here.
+			dist[*peer_node_counter as usize] = Some(PathBuildingHop {
+				candidate: CandidateRouteHop::FirstHop(FirstHopCandidate {
+					details: &chans[0],
+					payer_node_id: &our_node_id,
+					target_node_counter: u32::max_value(),
+					payer_node_counter: u32::max_value(),
+				}),
+				fee_msat: 0,
+				next_hops_fee_msat: u64::max_value(),
+				hop_use_fee_msat: u64::max_value(),
+				total_fee_msat: u64::max_value(),
+				path_htlc_minimum_msat: u64::max_value(),
+				path_penalty_msat: u64::max_value(),
+				was_processed: false,
+				is_first_hop_target: true,
+				#[cfg(all(not(ldk_bench), any(test, fuzzing)))]
+				value_contribution_msat: 0,
+			});
+		}
 		hit_minimum_limit = false;
 
 		// If first hop is a private channel and the only way to reach the payee, this is the only
