X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Frouting%2Frouter.rs;h=7bd1cebc532de093c4ce81f315f3759eb0501494;hb=d15a354f775b9e1bb6bdc0795bc948b03e6cb7ac;hp=652c8cd32f2cec946db87fddaf3db99ceffe5632;hpb=e2f34cb12216f2b2f6292615bb41bad882eec742;p=rust-lightning diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 652c8cd3..7bd1cebc 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -968,33 +968,24 @@ impl_writeable_tlv_based!(RouteHintHop, { }); #[derive(Eq, PartialEq)] +#[repr(align(64))] // Force the size to 64 bytes struct RouteGraphNode { node_id: NodeId, - lowest_fee_to_node: u64, - total_cltv_delta: u32, + score: u64, // The maximum value a yet-to-be-constructed payment path might flow through this node. // This value is upper-bounded by us by: // - how much is needed for a path being constructed // - how much value can channels following this node (up to the destination) can contribute, // considering their capacity and fees value_contribution_msat: u64, - /// The effective htlc_minimum_msat at this hop. If a later hop on the path had a higher HTLC - /// minimum, we use it, plus the fees required at each earlier hop to meet it. - path_htlc_minimum_msat: u64, - /// All penalties incurred from this hop on the way to the destination, as calculated using - /// channel scoring. - path_penalty_msat: u64, + total_cltv_delta: u32, /// The number of hops walked up to this node. path_length_to_node: u8, } impl cmp::Ord for RouteGraphNode { fn cmp(&self, other: &RouteGraphNode) -> cmp::Ordering { - let other_score = cmp::max(other.lowest_fee_to_node, other.path_htlc_minimum_msat) - .saturating_add(other.path_penalty_msat); - let self_score = cmp::max(self.lowest_fee_to_node, self.path_htlc_minimum_msat) - .saturating_add(self.path_penalty_msat); - other_score.cmp(&self_score).then_with(|| other.node_id.cmp(&self.node_id)) + other.score.cmp(&self.score).then_with(|| other.node_id.cmp(&self.node_id)) } } @@ -1004,6 +995,16 @@ impl cmp::PartialOrd for RouteGraphNode { } } +// While RouteGraphNode can be laid out with fewer bytes, performance appears to be improved +// substantially when it is laid out at exactly 64 bytes. +// +// Thus, we use `#[repr(C)]` on the struct to force a suboptimal layout and check that it stays 64 +// bytes here. +#[cfg(any(ldk_bench, not(any(test, fuzzing))))] +const _GRAPH_NODE_SMALL: usize = 64 - core::mem::size_of::(); +#[cfg(any(ldk_bench, not(any(test, fuzzing))))] +const _GRAPH_NODE_FIXED_SIZE: usize = core::mem::size_of::() - 64; + /// A wrapper around the various hop representations. /// /// Can be used to examine the properties of a hop, @@ -1311,15 +1312,15 @@ fn iter_equal(mut iter_a: I1, mut iter_b: I2) /// Fee values should be updated only in the context of the whole path, see update_value_and_recompute_fees. /// These fee values are useful to choose hops as we traverse the graph "payee-to-payer". #[derive(Clone)] +#[repr(C)] // Force fields to appear in the order we define them. struct PathBuildingHop<'a> { candidate: CandidateRouteHop<'a>, - fee_msat: u64, - - /// All the fees paid *after* this channel on the way to the destination - next_hops_fee_msat: u64, - /// Fee paid for the use of the current channel (see candidate.fees()). - /// The value will be actually deducted from the counterparty balance on the previous link. - hop_use_fee_msat: u64, + /// If we've already processed a node as the best node, we shouldn't process it again. Normally + /// we'd just ignore it if we did as all channels would have a higher new fee, but because we + /// may decrease the amounts in use as we walk the graph, the actual calculated fee may + /// decrease as well. Thus, we have to explicitly track which nodes have been processed and + /// avoid processing them again. + was_processed: 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. @@ -1331,12 +1332,20 @@ struct PathBuildingHop<'a> { /// All penalties incurred from this channel on the way to the destination, as calculated using /// channel scoring. path_penalty_msat: u64, - /// If we've already processed a node as the best node, we shouldn't process it again. Normally - /// we'd just ignore it if we did as all channels would have a higher new fee, but because we - /// may decrease the amounts in use as we walk the graph, the actual calculated fee may - /// decrease as well. Thus, we have to explicitly track which nodes have been processed and - /// avoid processing them again. - was_processed: bool, + + // The last 16 bytes are on the next cache line by default in glibc's malloc. Thus, we should + // only place fields which are not hot there. Luckily, the next three fields are only read if + // we end up on the selected path, and only in the final path layout phase, so we don't care + // too much if reading them is slow. + + fee_msat: u64, + + /// All the fees paid *after* this channel on the way to the destination + next_hops_fee_msat: u64, + /// Fee paid for the use of the current channel (see candidate.fees()). + /// The value will be actually deducted from the counterparty balance on the previous link. + hop_use_fee_msat: u64, + #[cfg(all(not(ldk_bench), any(test, fuzzing)))] // In tests, we apply further sanity checks on cases where we skip nodes we already processed // to ensure it is specifically in cases where the fee has gone down because of a decrease in @@ -2112,15 +2121,6 @@ where L::Target: Logger { score_params); let path_penalty_msat = $next_hops_path_penalty_msat .saturating_add(channel_penalty_msat); - let new_graph_node = RouteGraphNode { - node_id: src_node_id, - lowest_fee_to_node: total_fee_msat, - total_cltv_delta: hop_total_cltv_delta, - value_contribution_msat, - path_htlc_minimum_msat, - path_penalty_msat, - path_length_to_node, - }; // Update the way of reaching $candidate.source() // with the given short_channel_id (from $candidate.target()), @@ -2145,6 +2145,13 @@ where L::Target: Logger { .saturating_add(path_penalty_msat); if !old_entry.was_processed && new_cost < old_cost { + let new_graph_node = RouteGraphNode { + node_id: src_node_id, + score: cmp::max(total_fee_msat, path_htlc_minimum_msat).saturating_add(path_penalty_msat), + total_cltv_delta: hop_total_cltv_delta, + value_contribution_msat, + path_length_to_node, + }; targets.push(new_graph_node); old_entry.next_hops_fee_msat = $next_hops_fee_msat; old_entry.hop_use_fee_msat = hop_use_fee_msat; @@ -2218,18 +2225,26 @@ where L::Target: Logger { // meaning how much will be paid in fees after this node (to the best of our knowledge). // This data can later be helpful to optimize routing (pay lower fees). macro_rules! add_entries_to_cheapest_to_target_node { - ( $node: expr, $node_id: expr, $fee_to_target_msat: expr, $next_hops_value_contribution: expr, - $next_hops_path_htlc_minimum_msat: expr, $next_hops_path_penalty_msat: expr, + ( $node: expr, $node_id: expr, $next_hops_value_contribution: expr, $next_hops_cltv_delta: expr, $next_hops_path_length: expr ) => { + let fee_to_target_msat; + let next_hops_path_htlc_minimum_msat; + let next_hops_path_penalty_msat; let skip_node = if let Some(elem) = dist.get_mut(&$node_id) { 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; was_processed } else { // Entries are added to dist in add_entry!() when there is a channel from a node. // Because there are no channels from payee, it will not have a dist entry at this point. // If we're processing any other node, it is always be the result of a channel from it. debug_assert_eq!($node_id, maybe_dummy_payee_node_id); + fee_to_target_msat = 0; + next_hops_path_htlc_minimum_msat = 0; + next_hops_path_penalty_msat = 0; false }; @@ -2239,9 +2254,9 @@ where L::Target: Logger { let candidate = CandidateRouteHop::FirstHop { details, payer_node_id: &our_node_id, }; - add_entry!(&candidate, $fee_to_target_msat, + add_entry!(&candidate, fee_to_target_msat, $next_hops_value_contribution, - $next_hops_path_htlc_minimum_msat, $next_hops_path_penalty_msat, + next_hops_path_htlc_minimum_msat, next_hops_path_penalty_msat, $next_hops_cltv_delta, $next_hops_path_length); } } @@ -2264,10 +2279,10 @@ where L::Target: Logger { short_channel_id: *chan_id, }; add_entry!(&candidate, - $fee_to_target_msat, + fee_to_target_msat, $next_hops_value_contribution, - $next_hops_path_htlc_minimum_msat, - $next_hops_path_penalty_msat, + next_hops_path_htlc_minimum_msat, + next_hops_path_penalty_msat, $next_hops_cltv_delta, $next_hops_path_length); } } @@ -2312,7 +2327,7 @@ where L::Target: Logger { // If not, targets.pop() will not even let us enter the loop in step 2. None => {}, Some(node) => { - add_entries_to_cheapest_to_target_node!(node, payee, 0, path_value_msat, 0, 0u64, 0, 0); + add_entries_to_cheapest_to_target_node!(node, payee, path_value_msat, 0, 0); }, }); @@ -2519,7 +2534,7 @@ where L::Target: Logger { // Both these cases (and other cases except reaching recommended_value_msat) mean that // paths_collection will be stopped because found_new_path==false. // This is not necessarily a routing failure. - 'path_construction: while let Some(RouteGraphNode { node_id, lowest_fee_to_node, total_cltv_delta, mut value_contribution_msat, path_htlc_minimum_msat, path_penalty_msat, path_length_to_node, .. }) = targets.pop() { + 'path_construction: while let Some(RouteGraphNode { node_id, total_cltv_delta, mut value_contribution_msat, path_length_to_node, .. }) = targets.pop() { // Since we're going payee-to-payer, hitting our node as a target means we should stop // traversing the graph and arrange the path out of what we found. @@ -2654,8 +2669,8 @@ where L::Target: Logger { match network_nodes.get(&node_id) { None => {}, Some(node) => { - add_entries_to_cheapest_to_target_node!(node, node_id, lowest_fee_to_node, - value_contribution_msat, path_htlc_minimum_msat, path_penalty_msat, + add_entries_to_cheapest_to_target_node!(node, node_id, + value_contribution_msat, total_cltv_delta, path_length_to_node); }, } @@ -8145,6 +8160,7 @@ mod tests { pub(crate) mod bench_utils { use super::*; use std::fs::File; + use std::time::Duration; use bitcoin::hashes::Hash; use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey}; @@ -8293,10 +8309,10 @@ pub(crate) mod bench_utils { if let Ok(route) = route_res { for path in route.paths { if seed & 0x80 == 0 { - scorer.payment_path_successful(&path); + scorer.payment_path_successful(&path, Duration::ZERO); } else { let short_channel_id = path.hops[path.hops.len() / 2].short_channel_id; - scorer.payment_path_failed(&path, short_channel_id); + scorer.payment_path_failed(&path, short_channel_id, Duration::ZERO); } seed = seed.overflowing_mul(6364136223846793005).0.overflowing_add(1).0; }