From: Matt Corallo Date: Wed, 6 Dec 2023 06:02:37 +0000 (+0000) Subject: Make `find_route`'s `dist` map elements fit in 128 bytes X-Git-Tag: v0.0.119~18^2~2 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=e2f34cb12216f2b2f6292615bb41bad882eec742;p=rust-lightning Make `find_route`'s `dist` map elements fit in 128 bytes We'd previously aggressively cached elements in the `PathBuildingHop` struct (and its sub-structs), which resulted in a rather bloated size. This implied cache misses as we read from and write to multiple cache lines during processing of a single channel. Here, we reduce caching in `DirectedChannelInfo`, fitting the `(NodeId, PathBuildingHop)` tuple in exactly 128 bytes. While this should fit in a single cache line, it sadly does not generally lie in only two lines, as glibc returns large buffers from `malloc` which are very well aligned, plus 16 bytes (for its own allocation tracking). Thus, we try to avoid reading from the last 16 bytes of a `PathBuildingHop`, but luckily that isn't super hard. Note that here we make accessing `DirectedChannelInfo::effective_capacity` somewhat slower, but that's okay as its only ever done once per `DirectedChannelInfo` anyway. While our routing benchmarks are quite noisy, this appears to result in between a 5% and 15% performance improvement in the probabilistic scoring benchmarks. --- diff --git a/lightning/src/routing/gossip.rs b/lightning/src/routing/gossip.rs index 974062891..9b4e41ae1 100644 --- a/lightning/src/routing/gossip.rs +++ b/lightning/src/routing/gossip.rs @@ -990,8 +990,6 @@ impl Readable for ChannelInfo { pub struct DirectedChannelInfo<'a> { channel: &'a ChannelInfo, direction: &'a ChannelUpdateInfo, - htlc_maximum_msat: u64, - effective_capacity: EffectiveCapacity, /// 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, @@ -1000,39 +998,30 @@ pub struct DirectedChannelInfo<'a> { impl<'a> DirectedChannelInfo<'a> { #[inline] fn new(channel: &'a ChannelInfo, direction: &'a ChannelUpdateInfo, from_node_one: bool) -> Self { - let mut htlc_maximum_msat = direction.htlc_maximum_msat; - let capacity_msat = channel.capacity_sats.map(|capacity_sats| capacity_sats * 1000); - - let effective_capacity = match capacity_msat { - Some(capacity_msat) => { - htlc_maximum_msat = cmp::min(htlc_maximum_msat, capacity_msat); - EffectiveCapacity::Total { capacity_msat, htlc_maximum_msat } - }, - None => EffectiveCapacity::AdvertisedMaxHTLC { amount_msat: htlc_maximum_msat }, - }; - - Self { - channel, direction, htlc_maximum_msat, effective_capacity, from_node_one, - } + Self { channel, direction, from_node_one } } /// Returns information for the channel. #[inline] pub fn channel(&self) -> &'a ChannelInfo { self.channel } - /// Returns the maximum HTLC amount allowed over the channel in the direction. - #[inline] - pub fn htlc_maximum_msat(&self) -> u64 { - self.htlc_maximum_msat - } - /// Returns the [`EffectiveCapacity`] of the channel in the direction. /// /// This is either the total capacity from the funding transaction, if known, or the /// `htlc_maximum_msat` for the direction as advertised by the gossip network, if known, /// otherwise. + #[inline] pub fn effective_capacity(&self) -> EffectiveCapacity { - self.effective_capacity + let mut htlc_maximum_msat = self.direction().htlc_maximum_msat; + let capacity_msat = self.channel.capacity_sats.map(|capacity_sats| capacity_sats * 1000); + + match capacity_msat { + Some(capacity_msat) => { + htlc_maximum_msat = cmp::min(htlc_maximum_msat, capacity_msat); + EffectiveCapacity::Total { capacity_msat, htlc_maximum_msat } + }, + None => EffectiveCapacity::AdvertisedMaxHTLC { amount_msat: htlc_maximum_msat }, + } } /// Returns information for the direction. @@ -1042,11 +1031,13 @@ impl<'a> DirectedChannelInfo<'a> { /// Returns the `node_id` of the source hop. /// /// Refers to the `node_id` forwarding the payment to the next hop. + #[inline] pub(super) fn source(&self) -> &'a NodeId { if self.from_node_one { &self.channel.node_one } else { &self.channel.node_two } } /// Returns the `node_id` of the target hop. /// /// Refers to the `node_id` receiving the payment from the previous hop. + #[inline] pub(super) fn target(&self) -> &'a NodeId { if self.from_node_one { &self.channel.node_two } else { &self.channel.node_one } } } diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 7b1df46fb..652c8cd32 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -1186,6 +1186,10 @@ impl<'a> CandidateRouteHop<'a> { } } + /// Fetch the effective capacity of this hop. + /// + /// Note that this may be somewhat expensive, so calls to this should be limited and results + /// cached! fn effective_capacity(&self) -> EffectiveCapacity { match self { CandidateRouteHop::FirstHop { details, .. } => EffectiveCapacity::ExactLiquidity { @@ -1341,6 +1345,18 @@ struct PathBuildingHop<'a> { value_contribution_msat: u64, } +// Checks that the entries in the `find_route` `dist` map fit in (exactly) two standard x86-64 +// cache lines. Sadly, they're not guaranteed to actually lie on a cache line (and in fact, +// generally won't, because at least glibc's malloc will align to a nice, big, round +// boundary...plus 16), but at least it will reduce the amount of data we'll need to load. +// +// Note that these assertions only pass on somewhat recent rustc, and thus are gated on the +// ldk_bench flag. +#[cfg(ldk_bench)] +const _NODE_MAP_SIZE_TWO_CACHE_LINES: usize = 128 - core::mem::size_of::<(NodeId, PathBuildingHop)>(); +#[cfg(ldk_bench)] +const _NODE_MAP_SIZE_EXACTLY_CACHE_LINES: usize = core::mem::size_of::<(NodeId, PathBuildingHop)>() - 128; + impl<'a> core::fmt::Debug for PathBuildingHop<'a> { fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> { let mut debug_struct = f.debug_struct("PathBuildingHop");