Skip to content

Commit 46a9f78

Browse files
committed
Align PathBuildingHop to 128b, now that we store them in a Vec
Now that `PathBuildingHop` is stored in a `Vec` (as `Option`s), rather than `HashMap` entries, they can grow to fill a full two cache lines without a memory access performance cost. In the next commit we'll take advantage of this somewhat, but here we update the assertions and drop the `repr(C)`, allowing rust to lay the memory out as it wishes.
1 parent 5f6ed73 commit 46a9f78

File tree

1 file changed

+3
-22
lines changed

1 file changed

+3
-22
lines changed

lightning/src/routing/router.rs

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1150,12 +1150,7 @@ impl cmp::PartialOrd for RouteGraphNode {
11501150

11511151
// While RouteGraphNode can be laid out with fewer bytes, performance appears to be improved
11521152
// substantially when it is laid out at exactly 64 bytes.
1153-
//
1154-
// Thus, we use `#[repr(C)]` on the struct to force a suboptimal layout and check that it stays 64
1155-
// bytes here.
1156-
#[cfg(any(ldk_bench, not(any(test, fuzzing))))]
11571153
const _GRAPH_NODE_SMALL: usize = 64 - core::mem::size_of::<RouteGraphNode>();
1158-
#[cfg(any(ldk_bench, not(any(test, fuzzing))))]
11591154
const _GRAPH_NODE_FIXED_SIZE: usize = core::mem::size_of::<RouteGraphNode>() - 64;
11601155

11611156
/// A [`CandidateRouteHop::FirstHop`] entry.
@@ -1649,7 +1644,7 @@ fn iter_equal<I1: Iterator, I2: Iterator>(mut iter_a: I1, mut iter_b: I2)
16491644
/// Fee values should be updated only in the context of the whole path, see update_value_and_recompute_fees.
16501645
/// These fee values are useful to choose hops as we traverse the graph "payee-to-payer".
16511646
#[derive(Clone)]
1652-
#[repr(C)] // Force fields to appear in the order we define them.
1647+
#[repr(align(128))]
16531648
struct PathBuildingHop<'a> {
16541649
candidate: CandidateRouteHop<'a>,
16551650
target_node_counter: Option<u32>,
@@ -1679,11 +1674,6 @@ struct PathBuildingHop<'a> {
16791674
/// channel scoring.
16801675
path_penalty_msat: u64,
16811676

1682-
// The last 16 bytes are on the next cache line by default in glibc's malloc. Thus, we should
1683-
// only place fields which are not hot there. Luckily, the next three fields are only read if
1684-
// we end up on the selected path, and only in the final path layout phase, so we don't care
1685-
// too much if reading them is slow.
1686-
16871677
fee_msat: u64,
16881678

16891679
/// All the fees paid *after* this channel on the way to the destination
@@ -1700,17 +1690,8 @@ struct PathBuildingHop<'a> {
17001690
value_contribution_msat: u64,
17011691
}
17021692

1703-
// Checks that the entries in the `find_route` `dist` map fit in (exactly) two standard x86-64
1704-
// cache lines. Sadly, they're not guaranteed to actually lie on a cache line (and in fact,
1705-
// generally won't, because at least glibc's malloc will align to a nice, big, round
1706-
// boundary...plus 16), but at least it will reduce the amount of data we'll need to load.
1707-
//
1708-
// Note that these assertions only pass on somewhat recent rustc, and thus are gated on the
1709-
// ldk_bench flag.
1710-
#[cfg(ldk_bench)]
1711-
const _NODE_MAP_SIZE_TWO_CACHE_LINES: usize = 128 - core::mem::size_of::<(NodeId, PathBuildingHop)>();
1712-
#[cfg(ldk_bench)]
1713-
const _NODE_MAP_SIZE_EXACTLY_CACHE_LINES: usize = core::mem::size_of::<(NodeId, PathBuildingHop)>() - 128;
1693+
const _NODE_MAP_SIZE_TWO_CACHE_LINES: usize = 128 - core::mem::size_of::<Option<PathBuildingHop>>();
1694+
const _NODE_MAP_SIZE_EXACTLY_TWO_CACHE_LINES: usize = core::mem::size_of::<Option<PathBuildingHop>>() - 128;
17141695

17151696
impl<'a> core::fmt::Debug for PathBuildingHop<'a> {
17161697
fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> {

0 commit comments

Comments
 (0)