Skip to content

Commit

Permalink
Make find_route's dist map elements fit in 128 bytes
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
TheBlueMatt committed Dec 6, 2023
1 parent 40795ac commit 2fc2bbc
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 23 deletions.
37 changes: 14 additions & 23 deletions lightning/src/routing/gossip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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.
Expand All @@ -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 } }
}

Expand Down
16 changes: 16 additions & 0 deletions lightning/src/routing/router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1183,6 +1183,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 {
Expand Down Expand Up @@ -1338,6 +1342,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");
Expand Down

0 comments on commit 2fc2bbc

Please sign in to comment.