From fbaa3c4855ec3ebfb62fe91437f4237582db2577 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 11 May 2023 05:34:00 +0000 Subject: [PATCH 1/5] Unify route benchmarking with route tests There's a few route tests which do the same thing as the benchmarks as they're also a good test. However, they didn't share code, which is somewhat wasteful, so we fix that here. --- lightning/src/routing/router.rs | 247 +++++++++++++++----------------- 1 file changed, 119 insertions(+), 128 deletions(-) diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index b33e021ab4f..fc65bc99000 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -5791,44 +5791,26 @@ mod tests { println!("Using seed of {}", seed); seed } - #[cfg(not(feature = "no-std"))] - use crate::util::ser::ReadableArgs; #[test] #[cfg(not(feature = "no-std"))] fn generate_routes() { use crate::routing::scoring::{ProbabilisticScorer, ProbabilisticScoringFeeParameters}; - let mut d = match super::bench_utils::get_route_file() { + let logger = ln_test_utils::TestLogger::new(); + let graph = match super::bench_utils::read_network_graph(&logger) { Ok(f) => f, Err(e) => { eprintln!("{}", e); return; }, }; - let logger = ln_test_utils::TestLogger::new(); - let graph = NetworkGraph::read(&mut d, &logger).unwrap(); - let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); - let random_seed_bytes = keys_manager.get_secure_random_bytes(); - // First, get 100 (source, destination) pairs for which route-getting actually succeeds... - let mut seed = random_init_seed() as usize; - let nodes = graph.read_only().nodes().clone(); - 'load_endpoints: for _ in 0..10 { - loop { - seed = seed.overflowing_mul(0xdeadbeef).0; - let src = &PublicKey::from_slice(nodes.unordered_keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap(); - seed = seed.overflowing_mul(0xdeadbeef).0; - let dst = PublicKey::from_slice(nodes.unordered_keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap(); - let payment_params = PaymentParameters::from_node_id(dst, 42); - let amt = seed as u64 % 200_000_000; - let params = ProbabilisticScoringFeeParameters::default(); - let scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &graph, &logger); - if get_route(src, &payment_params, &graph.read_only(), None, amt, &logger, &scorer, ¶ms, &random_seed_bytes).is_ok() { - continue 'load_endpoints; - } - } - } + let params = ProbabilisticScoringFeeParameters::default(); + let mut scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &graph, &logger); + let features = super::InvoiceFeatures::empty(); + + super::bench_utils::generate_test_routes(&graph, &mut scorer, ¶ms, features, random_init_seed() as usize, 2); } #[test] @@ -5836,37 +5818,20 @@ mod tests { fn generate_routes_mpp() { use crate::routing::scoring::{ProbabilisticScorer, ProbabilisticScoringFeeParameters}; - let mut d = match super::bench_utils::get_route_file() { + let logger = ln_test_utils::TestLogger::new(); + let graph = match super::bench_utils::read_network_graph(&logger) { Ok(f) => f, Err(e) => { eprintln!("{}", e); return; }, }; - let logger = ln_test_utils::TestLogger::new(); - let graph = NetworkGraph::read(&mut d, &logger).unwrap(); - let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); - let random_seed_bytes = keys_manager.get_secure_random_bytes(); - let config = UserConfig::default(); - // First, get 100 (source, destination) pairs for which route-getting actually succeeds... - let mut seed = random_init_seed() as usize; - let nodes = graph.read_only().nodes().clone(); - 'load_endpoints: for _ in 0..10 { - loop { - seed = seed.overflowing_mul(0xdeadbeef).0; - let src = &PublicKey::from_slice(nodes.unordered_keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap(); - seed = seed.overflowing_mul(0xdeadbeef).0; - let dst = PublicKey::from_slice(nodes.unordered_keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap(); - let payment_params = PaymentParameters::from_node_id(dst, 42).with_bolt11_features(channelmanager::provided_invoice_features(&config)).unwrap(); - let amt = seed as u64 % 200_000_000; - let params = ProbabilisticScoringFeeParameters::default(); - let scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &graph, &logger); - if get_route(src, &payment_params, &graph.read_only(), None, amt, &logger, &scorer, ¶ms, &random_seed_bytes).is_ok() { - continue 'load_endpoints; - } - } - } + let params = ProbabilisticScoringFeeParameters::default(); + let mut scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &graph, &logger); + let features = channelmanager::provided_invoice_features(&UserConfig::default()); + + super::bench_utils::generate_test_routes(&graph, &mut scorer, ¶ms, features, random_init_seed() as usize, 2); } #[test] @@ -6054,7 +6019,21 @@ mod tests { #[cfg(all(test, not(feature = "no-std")))] pub(crate) mod bench_utils { + use super::*; use std::fs::File; + + use bitcoin::hashes::Hash; + use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey}; + + use crate::chain::transaction::OutPoint; + use crate::sign::{EntropySource, KeysManager}; + use crate::ln::channelmanager::{self, ChannelCounterparty, ChannelDetails}; + use crate::ln::features::InvoiceFeatures; + use crate::routing::gossip::NetworkGraph; + use crate::util::config::UserConfig; + use crate::util::ser::ReadableArgs; + use crate::util::test_utils::TestLogger; + /// Tries to open a network graph file, or panics with a URL to fetch it. pub(crate) fn get_route_file() -> Result { let res = File::open("net_graph-2023-01-18.bin") // By default we're run in RL/lightning @@ -6077,42 +6056,18 @@ pub(crate) mod bench_utils { #[cfg(not(require_route_graph_test))] return res; } -} - -#[cfg(all(test, feature = "_bench_unstable", not(feature = "no-std")))] -mod benches { - use super::*; - use bitcoin::hashes::Hash; - use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey}; - use crate::chain::transaction::OutPoint; - use crate::sign::{EntropySource, KeysManager}; - use crate::ln::channelmanager::{self, ChannelCounterparty, ChannelDetails}; - use crate::ln::features::InvoiceFeatures; - use crate::routing::gossip::NetworkGraph; - use crate::routing::scoring::{FixedPenaltyScorer, ProbabilisticScorer, ProbabilisticScoringFeeParameters, ProbabilisticScoringDecayParameters}; - use crate::util::config::UserConfig; - use crate::util::logger::{Logger, Record}; - use crate::util::ser::ReadableArgs; - - use test::Bencher; - - struct DummyLogger {} - impl Logger for DummyLogger { - fn log(&self, _record: &Record) {} - } - fn read_network_graph(logger: &DummyLogger) -> NetworkGraph<&DummyLogger> { - let mut d = bench_utils::get_route_file().unwrap(); - NetworkGraph::read(&mut d, logger).unwrap() + pub(crate) fn read_network_graph(logger: &TestLogger) -> Result, &'static str> { + get_route_file().map(|mut f| NetworkGraph::read(&mut f, logger).unwrap()) } - fn payer_pubkey() -> PublicKey { + pub(crate) fn payer_pubkey() -> PublicKey { let secp_ctx = Secp256k1::new(); PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()) } #[inline] - fn first_hop(node_id: PublicKey) -> ChannelDetails { + pub(crate) fn first_hop(node_id: PublicKey) -> ChannelDetails { ChannelDetails { channel_id: [0; 32], counterparty: ChannelCounterparty { @@ -6151,63 +6106,29 @@ mod benches { } } - #[bench] - fn generate_routes_with_zero_penalty_scorer(bench: &mut Bencher) { - let logger = DummyLogger {}; - let network_graph = read_network_graph(&logger); - let scorer = FixedPenaltyScorer::with_penalty(0); - generate_routes(bench, &network_graph, scorer, &(), InvoiceFeatures::empty()); - } - - #[bench] - fn generate_mpp_routes_with_zero_penalty_scorer(bench: &mut Bencher) { - let logger = DummyLogger {}; - let network_graph = read_network_graph(&logger); - let scorer = FixedPenaltyScorer::with_penalty(0); - generate_routes(bench, &network_graph, scorer, &(), channelmanager::provided_invoice_features(&UserConfig::default())); - } - - #[bench] - fn generate_routes_with_probabilistic_scorer(bench: &mut Bencher) { - let logger = DummyLogger {}; - let network_graph = read_network_graph(&logger); - let params = ProbabilisticScoringFeeParameters::default(); - let scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &network_graph, &logger); - generate_routes(bench, &network_graph, scorer, ¶ms, InvoiceFeatures::empty()); - } - - #[bench] - fn generate_mpp_routes_with_probabilistic_scorer(bench: &mut Bencher) { - let logger = DummyLogger {}; - let network_graph = read_network_graph(&logger); - let params = ProbabilisticScoringFeeParameters::default(); - let scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &network_graph, &logger); - generate_routes(bench, &network_graph, scorer, ¶ms, channelmanager::provided_invoice_features(&UserConfig::default())); - } - - fn generate_routes( - bench: &mut Bencher, graph: &NetworkGraph<&DummyLogger>, mut scorer: S, score_params: &S::ScoreParams, - features: InvoiceFeatures - ) { - let nodes = graph.read_only().nodes().clone(); + pub(crate) fn generate_test_routes(graph: &NetworkGraph<&TestLogger>, scorer: &mut S, + score_params: &S::ScoreParams, features: InvoiceFeatures, mut seed: usize, route_count: usize, + ) -> Vec<(ChannelDetails, PaymentParameters, u64)> { let payer = payer_pubkey(); let keys_manager = KeysManager::new(&[0u8; 32], 42, 42); let random_seed_bytes = keys_manager.get_secure_random_bytes(); - // First, get 100 (source, destination) pairs for which route-getting actually succeeds... - let mut routes = Vec::new(); + let nodes = graph.read_only().nodes().clone(); let mut route_endpoints = Vec::new(); - let mut seed: usize = 0xdeadbeef; - 'load_endpoints: for _ in 0..150 { + let mut routes = Vec::new(); + + 'load_endpoints: for _ in 0..route_count * 3 /2 { loop { - seed *= 0xdeadbeef; + seed = seed.overflowing_mul(0xdeadbeef).0; let src = PublicKey::from_slice(nodes.unordered_keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap(); - seed *= 0xdeadbeef; + seed = seed.overflowing_mul(0xdeadbeef).0; let dst = PublicKey::from_slice(nodes.unordered_keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap(); let params = PaymentParameters::from_node_id(dst, 42).with_bolt11_features(features.clone()).unwrap(); let first_hop = first_hop(src); let amt = seed as u64 % 1_000_000; - if let Ok(route) = get_route(&payer, ¶ms, &graph.read_only(), Some(&[&first_hop]), amt, &DummyLogger{}, &scorer, score_params, &random_seed_bytes) { + if let Ok(route) = get_route(&payer, ¶ms, &graph.read_only(), Some(&[&first_hop]), + amt, &TestLogger::new(), &scorer, score_params, &random_seed_bytes, + ) { routes.push(route); route_endpoints.push((first_hop, params, amt)); continue 'load_endpoints; @@ -6230,20 +6151,90 @@ mod benches { } } - // Because we've changed channel scores, its possible we'll take different routes to the + // Because we've changed channel scores, it's possible we'll take different routes to the // selected destinations, possibly causing us to fail because, eg, the newly-selected path // requires a too-high CLTV delta. route_endpoints.retain(|(first_hop, params, amt)| { - get_route(&payer, params, &graph.read_only(), Some(&[first_hop]), *amt, &DummyLogger{}, &scorer, score_params, &random_seed_bytes).is_ok() + get_route(&payer, params, &graph.read_only(), Some(&[first_hop]), *amt, + &TestLogger::new(), &scorer, score_params, &random_seed_bytes).is_ok() }); - route_endpoints.truncate(100); - assert_eq!(route_endpoints.len(), 100); + route_endpoints.truncate(route_count); + assert_eq!(route_endpoints.len(), route_count); + route_endpoints + } +} + +#[cfg(all(test, feature = "_bench_unstable", not(feature = "no-std")))] +mod benches { + use super::*; + use crate::sign::{EntropySource, KeysManager}; + use crate::ln::channelmanager; + use crate::ln::features::InvoiceFeatures; + use crate::routing::gossip::NetworkGraph; + use crate::routing::scoring::{FixedPenaltyScorer, ProbabilisticScorer, ProbabilisticScoringFeeParameters, ProbabilisticScoringDecayParameters}; + use crate::util::config::UserConfig; + use crate::util::logger::{Logger, Record}; + use crate::util::test_utils::TestLogger; + + use test::Bencher; + + struct DummyLogger {} + impl Logger for DummyLogger { + fn log(&self, _record: &Record) {} + } + + + #[bench] + fn generate_routes_with_zero_penalty_scorer(bench: &mut Bencher) { + let logger = TestLogger::new(); + let network_graph = bench_utils::read_network_graph(&logger).unwrap(); + let scorer = FixedPenaltyScorer::with_penalty(0); + generate_routes(bench, &network_graph, scorer, &(), InvoiceFeatures::empty()); + } + + #[bench] + fn generate_mpp_routes_with_zero_penalty_scorer(bench: &mut Bencher) { + let logger = TestLogger::new(); + let network_graph = bench_utils::read_network_graph(&logger).unwrap(); + let scorer = FixedPenaltyScorer::with_penalty(0); + generate_routes(bench, &network_graph, scorer, &(), channelmanager::provided_invoice_features(&UserConfig::default())); + } + + #[bench] + fn generate_routes_with_probabilistic_scorer(bench: &mut Bencher) { + let logger = TestLogger::new(); + let network_graph = bench_utils::read_network_graph(&logger).unwrap(); + let params = ProbabilisticScoringFeeParameters::default(); + let scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &network_graph, &logger); + generate_routes(bench, &network_graph, scorer, ¶ms, InvoiceFeatures::empty()); + } + + #[bench] + fn generate_mpp_routes_with_probabilistic_scorer(bench: &mut Bencher) { + let logger = TestLogger::new(); + let network_graph = bench_utils::read_network_graph(&logger).unwrap(); + let params = ProbabilisticScoringFeeParameters::default(); + let scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &network_graph, &logger); + generate_routes(bench, &network_graph, scorer, ¶ms, channelmanager::provided_invoice_features(&UserConfig::default())); + } + + fn generate_routes( + bench: &mut Bencher, graph: &NetworkGraph<&TestLogger>, mut scorer: S, + score_params: &S::ScoreParams, features: InvoiceFeatures, + ) { + let payer = bench_utils::payer_pubkey(); + let keys_manager = KeysManager::new(&[0u8; 32], 42, 42); + let random_seed_bytes = keys_manager.get_secure_random_bytes(); + + // First, get 100 (source, destination) pairs for which route-getting actually succeeds... + let route_endpoints = bench_utils::generate_test_routes(graph, &mut scorer, score_params, features, 0xdeadbeef, 100); // ...then benchmark finding paths between the nodes we learned. let mut idx = 0; bench.iter(|| { let (first_hop, params, amt) = &route_endpoints[idx % route_endpoints.len()]; - assert!(get_route(&payer, params, &graph.read_only(), Some(&[first_hop]), *amt, &DummyLogger{}, &scorer, score_params, &random_seed_bytes).is_ok()); + assert!(get_route(&payer, params, &graph.read_only(), Some(&[first_hop]), *amt, + &DummyLogger{}, &scorer, score_params, &random_seed_bytes).is_ok()); idx += 1; }); } From 2775902f3ab0fd8a25a2ec83303fb959d7468abb Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 11 May 2023 05:46:38 +0000 Subject: [PATCH 2/5] Add an additional test/bench for routing larger amounts, score more When benchmarking our router, we previously only ever tested with amounts under 1,000 sats, which is an incredibly small amount. While this ensures we have the maximal number of available channels to consider, it prevents our scorer from getting exercise across its range. Further, we only score the immediate path we are expecting to to send over, and not randomly but rather based on the amount sent. Here we try to make the benchmarks a bit more realistic by adding a new benchmark which attempts to send around 100K sats, which is a reasonable amount to send over a channel today. We also convert the scoring data to be randomized based on the seed as well as attempt to (possibly) find a new route for a much larger value and score based on that. This potentially allows us to score multiple potential paths between the source and destination as the large route-find may return an MPP result. --- lightning/src/routing/router.rs | 133 ++++++++++++++++++++++---------- 1 file changed, 91 insertions(+), 42 deletions(-) diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index fc65bc99000..420fc3d202d 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -5810,7 +5810,7 @@ mod tests { let mut scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &graph, &logger); let features = super::InvoiceFeatures::empty(); - super::bench_utils::generate_test_routes(&graph, &mut scorer, ¶ms, features, random_init_seed() as usize, 2); + super::bench_utils::generate_test_routes(&graph, &mut scorer, ¶ms, features, random_init_seed(), 0, 2); } #[test] @@ -5831,7 +5831,28 @@ mod tests { let mut scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &graph, &logger); let features = channelmanager::provided_invoice_features(&UserConfig::default()); - super::bench_utils::generate_test_routes(&graph, &mut scorer, ¶ms, features, random_init_seed() as usize, 2); + super::bench_utils::generate_test_routes(&graph, &mut scorer, ¶ms, features, random_init_seed(), 0, 2); + } + + #[test] + #[cfg(not(feature = "no-std"))] + fn generate_large_mpp_routes() { + use crate::routing::scoring::{ProbabilisticScorer, ProbabilisticScoringFeeParameters}; + + let logger = ln_test_utils::TestLogger::new(); + let graph = match super::bench_utils::read_network_graph(&logger) { + Ok(f) => f, + Err(e) => { + eprintln!("{}", e); + return; + }, + }; + + let params = ProbabilisticScoringFeeParameters::default(); + let mut scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &graph, &logger); + let features = channelmanager::provided_invoice_features(&UserConfig::default()); + + super::bench_utils::generate_test_routes(&graph, &mut scorer, ¶ms, features, random_init_seed(), 1_000_000, 2); } #[test] @@ -6085,11 +6106,11 @@ pub(crate) mod bench_utils { short_channel_id: Some(1), inbound_scid_alias: None, outbound_scid_alias: None, - channel_value_satoshis: 10_000_000, + channel_value_satoshis: 10_000_000_000, user_channel_id: 0, - balance_msat: 10_000_000, - outbound_capacity_msat: 10_000_000, - next_outbound_htlc_limit_msat: 10_000_000, + balance_msat: 10_000_000_000, + outbound_capacity_msat: 10_000_000_000, + next_outbound_htlc_limit_msat: 10_000_000_000, inbound_capacity_msat: 0, unspendable_punishment_reserve: None, confirmations_required: None, @@ -6107,7 +6128,8 @@ pub(crate) mod bench_utils { } pub(crate) fn generate_test_routes(graph: &NetworkGraph<&TestLogger>, scorer: &mut S, - score_params: &S::ScoreParams, features: InvoiceFeatures, mut seed: usize, route_count: usize, + score_params: &S::ScoreParams, features: InvoiceFeatures, mut seed: u64, + starting_amount: u64, route_count: usize, ) -> Vec<(ChannelDetails, PaymentParameters, u64)> { let payer = payer_pubkey(); let keys_manager = KeysManager::new(&[0u8; 32], 42, 42); @@ -6115,38 +6137,56 @@ pub(crate) mod bench_utils { let nodes = graph.read_only().nodes().clone(); let mut route_endpoints = Vec::new(); - let mut routes = Vec::new(); - - 'load_endpoints: for _ in 0..route_count * 3 /2 { + // Fetch 1.5x more routes than we need as after we do some scorer updates we may end up + // with some routes we picked being un-routable. + for _ in 0..route_count * 3 / 2 { loop { - seed = seed.overflowing_mul(0xdeadbeef).0; - let src = PublicKey::from_slice(nodes.unordered_keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap(); - seed = seed.overflowing_mul(0xdeadbeef).0; - let dst = PublicKey::from_slice(nodes.unordered_keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap(); - let params = PaymentParameters::from_node_id(dst, 42).with_bolt11_features(features.clone()).unwrap(); + seed = seed.overflowing_mul(6364136223846793005).0.overflowing_add(1).0; + let src = PublicKey::from_slice(nodes.unordered_keys() + .skip((seed as usize) % nodes.len()).next().unwrap().as_slice()).unwrap(); + seed = seed.overflowing_mul(6364136223846793005).0.overflowing_add(1).0; + let dst = PublicKey::from_slice(nodes.unordered_keys() + .skip((seed as usize) % nodes.len()).next().unwrap().as_slice()).unwrap(); + let params = PaymentParameters::from_node_id(dst, 42) + .with_bolt11_features(features.clone()).unwrap(); let first_hop = first_hop(src); - let amt = seed as u64 % 1_000_000; - if let Ok(route) = get_route(&payer, ¶ms, &graph.read_only(), Some(&[&first_hop]), - amt, &TestLogger::new(), &scorer, score_params, &random_seed_bytes, - ) { - routes.push(route); - route_endpoints.push((first_hop, params, amt)); - continue 'load_endpoints; - } - } - } + let amt = starting_amount + seed % 1_000_000; + let path_exists = + get_route(&payer, ¶ms, &graph.read_only(), Some(&[&first_hop]), + amt, &TestLogger::new(), &scorer, score_params, &random_seed_bytes).is_ok(); + if path_exists { + // ...and seed the scorer with success and failure data... + seed = seed.overflowing_mul(6364136223846793005).0.overflowing_add(1).0; + let mut score_amt = seed % 1_000_000_000; + loop { + // Generate fail/success paths for a wider range of potential amounts with + // MPP enabled to give us a chance to apply penalties for more potential + // routes. + let mpp_features = channelmanager::provided_invoice_features(&UserConfig::default()); + let params = PaymentParameters::from_node_id(dst, 42) + .with_bolt11_features(mpp_features).unwrap(); + + let route_res = get_route(&payer, ¶ms, &graph.read_only(), + Some(&[&first_hop]), score_amt, &TestLogger::new(), &scorer, + score_params, &random_seed_bytes); + if let Ok(route) = route_res { + for path in route.paths { + if seed & 0x80 == 0 { + scorer.payment_path_successful(&path); + } else { + let short_channel_id = path.hops[path.hops.len() / 2].short_channel_id; + scorer.payment_path_failed(&path, short_channel_id); + } + seed = seed.overflowing_mul(6364136223846793005).0.overflowing_add(1).0; + } + break; + } + // If we couldn't find a path with a higer amount, reduce and try again. + score_amt /= 100; + } - // ...and seed the scorer with success and failure data... - for route in routes { - let amount = route.get_total_amount(); - if amount < 250_000 { - for path in route.paths { - scorer.payment_path_successful(&path); - } - } else if amount > 750_000 { - for path in route.paths { - let short_channel_id = path.hops[path.hops.len() / 2].short_channel_id; - scorer.payment_path_failed(&path, short_channel_id); + route_endpoints.push((first_hop, params, amt)); + break; } } } @@ -6189,7 +6229,7 @@ mod benches { let logger = TestLogger::new(); let network_graph = bench_utils::read_network_graph(&logger).unwrap(); let scorer = FixedPenaltyScorer::with_penalty(0); - generate_routes(bench, &network_graph, scorer, &(), InvoiceFeatures::empty()); + generate_routes(bench, &network_graph, scorer, &(), InvoiceFeatures::empty(), 0); } #[bench] @@ -6197,7 +6237,7 @@ mod benches { let logger = TestLogger::new(); let network_graph = bench_utils::read_network_graph(&logger).unwrap(); let scorer = FixedPenaltyScorer::with_penalty(0); - generate_routes(bench, &network_graph, scorer, &(), channelmanager::provided_invoice_features(&UserConfig::default())); + generate_routes(bench, &network_graph, scorer, &(), channelmanager::provided_invoice_features(&UserConfig::default()), 0); } #[bench] @@ -6206,7 +6246,7 @@ mod benches { let network_graph = bench_utils::read_network_graph(&logger).unwrap(); let params = ProbabilisticScoringFeeParameters::default(); let scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &network_graph, &logger); - generate_routes(bench, &network_graph, scorer, ¶ms, InvoiceFeatures::empty()); + generate_routes(bench, &network_graph, scorer, ¶ms, InvoiceFeatures::empty(), 0); } #[bench] @@ -6215,19 +6255,28 @@ mod benches { let network_graph = bench_utils::read_network_graph(&logger).unwrap(); let params = ProbabilisticScoringFeeParameters::default(); let scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &network_graph, &logger); - generate_routes(bench, &network_graph, scorer, ¶ms, channelmanager::provided_invoice_features(&UserConfig::default())); + generate_routes(bench, &network_graph, scorer, ¶ms, channelmanager::provided_invoice_features(&UserConfig::default()), 0); + } + + #[bench] + fn generate_large_mpp_routes_with_probabilistic_scorer(bench: &mut Bencher) { + let logger = TestLogger::new(); + let network_graph = bench_utils::read_network_graph(&logger).unwrap(); + let params = ProbabilisticScoringFeeParameters::default(); + let scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &network_graph, &logger); + generate_routes(bench, &network_graph, scorer, ¶ms, channelmanager::provided_invoice_features(&UserConfig::default()), 100_000_000); } fn generate_routes( bench: &mut Bencher, graph: &NetworkGraph<&TestLogger>, mut scorer: S, - score_params: &S::ScoreParams, features: InvoiceFeatures, + score_params: &S::ScoreParams, features: InvoiceFeatures, starting_amount: u64, ) { let payer = bench_utils::payer_pubkey(); let keys_manager = KeysManager::new(&[0u8; 32], 42, 42); let random_seed_bytes = keys_manager.get_secure_random_bytes(); // First, get 100 (source, destination) pairs for which route-getting actually succeeds... - let route_endpoints = bench_utils::generate_test_routes(graph, &mut scorer, score_params, features, 0xdeadbeef, 100); + let route_endpoints = bench_utils::generate_test_routes(graph, &mut scorer, score_params, features, 0xdeadbeef, starting_amount, 50); // ...then benchmark finding paths between the nodes we learned. let mut idx = 0; From 1701b021241feae4091a5cda9f68c649070a416a Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 11 May 2023 06:03:57 +0000 Subject: [PATCH 3/5] Replace std's unmaintained bench with criterion Rather than using the std benchmark framework (which isn't maintained and is unlikely to get any further maintenance), we swap for criterion, which at least gets us a variable number of test runs so our benchmarks don't take forever. We also fix the RGS benchmark to pass now that the file in use is stale compared to today's date. --- .github/workflows/build.yml | 7 ++- Cargo.toml | 6 +- bench/Cargo.toml | 25 ++++++++ bench/benches/bench.rs | 22 +++++++ lightning-persister/Cargo.toml | 6 +- lightning-persister/src/lib.rs | 19 +++--- lightning-rapid-gossip-sync/Cargo.toml | 4 +- lightning-rapid-gossip-sync/src/lib.rs | 51 ++++++++-------- lightning/Cargo.toml | 4 +- lightning/src/lib.rs | 7 +-- lightning/src/ln/channelmanager.rs | 16 +++-- lightning/src/ln/functional_test_utils.rs | 8 +-- lightning/src/routing/gossip.rs | 25 ++++---- lightning/src/routing/router.rs | 74 ++++++++++++++--------- lightning/src/sign/mod.rs | 17 ++---- lightning/src/sync/mod.rs | 14 ++--- lightning/src/util/test_utils.rs | 2 +- 17 files changed, 183 insertions(+), 124 deletions(-) create mode 100644 bench/Cargo.toml create mode 100644 bench/benches/bench.rs diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 8e02ec9da75..3b6d1a0388c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -141,7 +141,12 @@ jobs: cd .. - name: Run benchmarks on Rust ${{ matrix.toolchain }} run: | - RUSTC_BOOTSTRAP=1 cargo bench --features _bench_unstable + cd bench + RUSTFLAGS="--cfg=ldk_bench --cfg=require_route_graph_test" cargo bench + - name: Run benchmarks with hashbrown on Rust ${{ matrix.toolchain }} + run: | + cd bench + RUSTFLAGS="--cfg=ldk_bench --cfg=require_route_graph_test" cargo bench --features hashbrown check_commits: runs-on: ubuntu-latest diff --git a/Cargo.toml b/Cargo.toml index afc0092c72d..a3acccfdaea 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,6 +14,7 @@ exclude = [ "lightning-custom-message", "lightning-transaction-sync", "no-std-check", + "bench", ] # Our tests do actual crypto and lots of work, the tradeoff for -O2 is well @@ -35,8 +36,3 @@ lto = "off" opt-level = 3 lto = true panic = "abort" - -[profile.bench] -opt-level = 3 -codegen-units = 1 -lto = true diff --git a/bench/Cargo.toml b/bench/Cargo.toml new file mode 100644 index 00000000000..e582d29da81 --- /dev/null +++ b/bench/Cargo.toml @@ -0,0 +1,25 @@ +[package] +name = "lightning-bench" +version = "0.0.1" +authors = ["Matt Corallo"] +edition = "2018" + +[[bench]] +name = "bench" +harness = false + +[features] +hashbrown = ["lightning/hashbrown"] + +[dependencies] +lightning = { path = "../lightning", features = ["_test_utils", "criterion"] } +lightning-persister = { path = "../lightning-persister", features = ["criterion"] } +lightning-rapid-gossip-sync = { path = "../lightning-rapid-gossip-sync", features = ["criterion"] } +criterion = { version = "0.4", default-features = false } + +[profile.release] +opt-level = 3 +codegen-units = 1 +lto = true +panic = "abort" +debug = true diff --git a/bench/benches/bench.rs b/bench/benches/bench.rs new file mode 100644 index 00000000000..54799f44c95 --- /dev/null +++ b/bench/benches/bench.rs @@ -0,0 +1,22 @@ +extern crate lightning; +extern crate lightning_persister; + +extern crate criterion; + +use criterion::{criterion_group, criterion_main}; + +criterion_group!(benches, + // Note that benches run in the order given here. Thus, they're sorted according to how likely + // developers are to be working on the specific code listed, then by runtime. + lightning::routing::router::benches::generate_routes_with_zero_penalty_scorer, + lightning::routing::router::benches::generate_mpp_routes_with_zero_penalty_scorer, + lightning::routing::router::benches::generate_routes_with_probabilistic_scorer, + lightning::routing::router::benches::generate_mpp_routes_with_probabilistic_scorer, + lightning::routing::router::benches::generate_large_mpp_routes_with_probabilistic_scorer, + lightning::sign::benches::bench_get_secure_random_bytes, + lightning::ln::channelmanager::bench::bench_sends, + lightning_persister::bench::bench_sends, + lightning_rapid_gossip_sync::bench::bench_reading_full_graph_from_file, + lightning::routing::gossip::benches::read_network_graph, + lightning::routing::gossip::benches::write_network_graph); +criterion_main!(benches); diff --git a/lightning-persister/Cargo.toml b/lightning-persister/Cargo.toml index 88132bdcfd6..22d4b16c42d 100644 --- a/lightning-persister/Cargo.toml +++ b/lightning-persister/Cargo.toml @@ -13,9 +13,6 @@ edition = "2018" all-features = true rustdoc-args = ["--cfg", "docsrs"] -[features] -_bench_unstable = ["lightning/_bench_unstable"] - [dependencies] bitcoin = "0.29.0" lightning = { version = "0.0.115", path = "../lightning" } @@ -24,5 +21,8 @@ libc = "0.2" [target.'cfg(windows)'.dependencies] winapi = { version = "0.3", features = ["winbase"] } +[target.'cfg(ldk_bench)'.dependencies] +criterion = { version = "0.4", optional = true, default-features = false } + [dev-dependencies] lightning = { version = "0.0.115", path = "../lightning", features = ["_test_utils"] } diff --git a/lightning-persister/src/lib.rs b/lightning-persister/src/lib.rs index d25ab6f9fca..f65ed111e39 100644 --- a/lightning-persister/src/lib.rs +++ b/lightning-persister/src/lib.rs @@ -8,8 +8,7 @@ #![cfg_attr(docsrs, feature(doc_auto_cfg))] -#![cfg_attr(all(test, feature = "_bench_unstable"), feature(test))] -#[cfg(all(test, feature = "_bench_unstable"))] extern crate test; +#[cfg(ldk_bench)] extern crate criterion; mod util; @@ -91,13 +90,13 @@ impl FilesystemPersister { continue; } - let txid = Txid::from_hex(filename.split_at(64).0) + let txid: Txid = Txid::from_hex(filename.split_at(64).0) .map_err(|_| std::io::Error::new( std::io::ErrorKind::InvalidData, "Invalid tx ID in filename", ))?; - let index = filename.split_at(65).1.parse() + let index: u16 = filename.split_at(65).1.parse() .map_err(|_| std::io::Error::new( std::io::ErrorKind::InvalidData, "Invalid tx index in filename", @@ -338,14 +337,16 @@ mod tests { } } -#[cfg(all(test, feature = "_bench_unstable"))] +#[cfg(ldk_bench)] +/// Benches pub mod bench { - use test::Bencher; + use criterion::Criterion; - #[bench] - fn bench_sends(bench: &mut Bencher) { + /// Bench! + pub fn bench_sends(bench: &mut Criterion) { let persister_a = super::FilesystemPersister::new("bench_filesystem_persister_a".to_string()); let persister_b = super::FilesystemPersister::new("bench_filesystem_persister_b".to_string()); - lightning::ln::channelmanager::bench::bench_two_sends(bench, persister_a, persister_b); + lightning::ln::channelmanager::bench::bench_two_sends( + bench, "bench_filesystem_persisted_sends", persister_a, persister_b); } } diff --git a/lightning-rapid-gossip-sync/Cargo.toml b/lightning-rapid-gossip-sync/Cargo.toml index 6673431d93b..73a68751128 100644 --- a/lightning-rapid-gossip-sync/Cargo.toml +++ b/lightning-rapid-gossip-sync/Cargo.toml @@ -13,11 +13,13 @@ Utility to process gossip routing data from Rapid Gossip Sync Server. default = ["std"] no-std = ["lightning/no-std"] std = ["lightning/std"] -_bench_unstable = [] [dependencies] lightning = { version = "0.0.115", path = "../lightning", default-features = false } bitcoin = { version = "0.29.0", default-features = false } +[target.'cfg(ldk_bench)'.dependencies] +criterion = { version = "0.4", optional = true, default-features = false } + [dev-dependencies] lightning = { version = "0.0.115", path = "../lightning", features = ["_test_utils"] } diff --git a/lightning-rapid-gossip-sync/src/lib.rs b/lightning-rapid-gossip-sync/src/lib.rs index c8f140cc189..5a61be7990e 100644 --- a/lightning-rapid-gossip-sync/src/lib.rs +++ b/lightning-rapid-gossip-sync/src/lib.rs @@ -64,10 +64,7 @@ #![cfg_attr(all(not(feature = "std"), not(test)), no_std)] -// Allow and import test features for benching -#![cfg_attr(all(test, feature = "_bench_unstable"), feature(test))] -#[cfg(all(test, feature = "_bench_unstable"))] -extern crate test; +#[cfg(ldk_bench)] extern crate criterion; #[cfg(not(feature = "std"))] extern crate alloc; @@ -287,36 +284,42 @@ mod tests { } } -#[cfg(all(test, feature = "_bench_unstable"))] +#[cfg(ldk_bench)] +/// Benches pub mod bench { - use test::Bencher; - use bitcoin::Network; - use lightning::ln::msgs::DecodeError; + use criterion::Criterion; + + use std::fs; + use lightning::routing::gossip::NetworkGraph; use lightning::util::test_utils::TestLogger; use crate::RapidGossipSync; - #[bench] - fn bench_reading_full_graph_from_file(b: &mut Bencher) { + /// Bench! + pub fn bench_reading_full_graph_from_file(b: &mut Criterion) { let logger = TestLogger::new(); - b.iter(|| { + b.bench_function("read_full_graph_from_rgs", |b| b.iter(|| { let network_graph = NetworkGraph::new(Network::Bitcoin, &logger); let rapid_sync = RapidGossipSync::new(&network_graph, &logger); - let sync_result = rapid_sync.sync_network_graph_with_file_path("./res/full_graph.lngossip"); - if let Err(crate::error::GraphSyncError::DecodeError(DecodeError::Io(io_error))) = &sync_result { - let error_string = format!("Input file lightning-rapid-gossip-sync/res/full_graph.lngossip is missing! Download it from https://bitcoin.ninja/ldk-compressed_graph-bc08df7542-2022-05-05.bin\n\n{:?}", io_error); - #[cfg(not(require_route_graph_test))] - { - println!("{}", error_string); - return; - } - #[cfg(require_route_graph_test)] - panic!("{}", error_string); - } - assert!(sync_result.is_ok()) - }); + let mut file = match fs::read("../lightning-rapid-gossip-sync/res/full_graph.lngossip") { + Ok(f) => f, + Err(io_error) => { + let error_string = format!( + "Input file lightning-rapid-gossip-sync/res/full_graph.lngossip is missing! Download it from https://bitcoin.ninja/ldk-compressed_graph-bc08df7542-2022-05-05.bin\n\n{:?}", + io_error); + #[cfg(not(require_route_graph_test))] + { + println!("{}", error_string); + return; + } + #[cfg(require_route_graph_test)] + panic!("{}", error_string); + }, + }; + rapid_sync.update_network_graph_no_std(&mut file, None).unwrap(); + })); } } diff --git a/lightning/Cargo.toml b/lightning/Cargo.toml index 1804be0f1bb..efaa82d2031 100644 --- a/lightning/Cargo.toml +++ b/lightning/Cargo.toml @@ -28,7 +28,6 @@ max_level_trace = [] # Allow signing of local transactions that may have been revoked or will be revoked, for functional testing (e.g. justice tx handling). # This is unsafe to use in production because it may result in the counterparty publishing taking our funds. unsafe_revoked_tx_signing = [] -_bench_unstable = [] # Override signing to not include randomness when generating signatures for test vectors. _test_vectors = [] @@ -59,5 +58,8 @@ version = "0.29.0" default-features = false features = ["bitcoinconsensus", "secp-recovery"] +[target.'cfg(ldk_bench)'.dependencies] +criterion = { version = "0.4", optional = true, default-features = false } + [target.'cfg(taproot)'.dependencies] musig2 = { git = "https://github.com/arik-so/rust-musig2", rev = "27797d7" } diff --git a/lightning/src/lib.rs b/lightning/src/lib.rs index e7e7e0ede6b..cea15b21ad2 100644 --- a/lightning/src/lib.rs +++ b/lightning/src/lib.rs @@ -54,9 +54,6 @@ #![cfg_attr(all(not(feature = "std"), not(test)), no_std)] -#![cfg_attr(all(any(test, feature = "_test_utils"), feature = "_bench_unstable"), feature(test))] -#[cfg(all(any(test, feature = "_test_utils"), feature = "_bench_unstable"))] extern crate test; - #[cfg(not(any(feature = "std", feature = "no-std")))] compile_error!("at least one of the `std` or `no-std` features must be enabled"); @@ -74,6 +71,8 @@ extern crate core; #[cfg(not(feature = "std"))] extern crate core2; +#[cfg(ldk_bench)] extern crate criterion; + #[macro_use] pub mod util; pub mod chain; @@ -177,7 +176,7 @@ mod prelude { pub use alloc::string::ToString; } -#[cfg(all(not(feature = "_bench_unstable"), feature = "backtrace", feature = "std", test))] +#[cfg(all(not(ldk_bench), feature = "backtrace", feature = "std", test))] extern crate backtrace; mod sync; diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 4797d05be65..52cd2d8bc83 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -9267,7 +9267,7 @@ mod tests { } } -#[cfg(all(any(test, feature = "_test_utils"), feature = "_bench_unstable"))] +#[cfg(ldk_bench)] pub mod bench { use crate::chain::Listen; use crate::chain::chainmonitor::{ChainMonitor, Persist}; @@ -9287,7 +9287,7 @@ pub mod bench { use crate::sync::{Arc, Mutex}; - use test::Bencher; + use criterion::Criterion; type Manager<'a, P> = ChannelManager< &'a ChainMonitor Option<&test_utils::TestChainMonitor> { None } } - #[cfg(test)] - #[bench] - fn bench_sends(bench: &mut Bencher) { - bench_two_sends(bench, test_utils::TestPersister::new(), test_utils::TestPersister::new()); + pub fn bench_sends(bench: &mut Criterion) { + bench_two_sends(bench, "bench_sends", test_utils::TestPersister::new(), test_utils::TestPersister::new()); } - pub fn bench_two_sends>(bench: &mut Bencher, persister_a: P, persister_b: P) { + pub fn bench_two_sends>(bench: &mut Criterion, bench_name: &str, persister_a: P, persister_b: P) { // Do a simple benchmark of sending a payment back and forth between two nodes. // Note that this is unrealistic as each payment send will require at least two fsync // calls per node. @@ -9470,9 +9468,9 @@ pub mod bench { } } - bench.iter(|| { + bench.bench_function(bench_name, |b| b.iter(|| { send_payment!(node_a, node_b); send_payment!(node_b, node_a); - }); + })); } } diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index a75d0c92285..56f08c7656a 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -1769,7 +1769,7 @@ macro_rules! get_route_and_payment_hash { } #[macro_export] -#[cfg(any(test, feature = "_bench_unstable", feature = "_test_utils"))] +#[cfg(any(test, ldk_bench, feature = "_test_utils"))] macro_rules! expect_payment_claimable { ($node: expr, $expected_payment_hash: expr, $expected_payment_secret: expr, $expected_recv_value: expr) => { expect_payment_claimable!($node, $expected_payment_hash, $expected_payment_secret, $expected_recv_value, None, $node.node.get_our_node_id()) @@ -1796,7 +1796,7 @@ macro_rules! expect_payment_claimable { } #[macro_export] -#[cfg(any(test, feature = "_bench_unstable", feature = "_test_utils"))] +#[cfg(any(test, ldk_bench, feature = "_test_utils"))] macro_rules! expect_payment_claimed { ($node: expr, $expected_payment_hash: expr, $expected_recv_value: expr) => { let events = $node.node.get_and_clear_pending_events(); @@ -1913,7 +1913,7 @@ macro_rules! expect_payment_forwarded { } } -#[cfg(any(test, feature = "_bench_unstable", feature = "_test_utils"))] +#[cfg(any(test, ldk_bench, feature = "_test_utils"))] pub fn expect_channel_pending_event<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, expected_counterparty_node_id: &PublicKey) { let events = node.node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); @@ -1925,7 +1925,7 @@ pub fn expect_channel_pending_event<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, } } -#[cfg(any(test, feature = "_bench_unstable", feature = "_test_utils"))] +#[cfg(any(test, ldk_bench, feature = "_test_utils"))] pub fn expect_channel_ready_event<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, expected_counterparty_node_id: &PublicKey) { let events = node.node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); diff --git a/lightning/src/routing/gossip.rs b/lightning/src/routing/gossip.rs index c5c08cf4032..d11eaa6bc36 100644 --- a/lightning/src/routing/gossip.rs +++ b/lightning/src/routing/gossip.rs @@ -3388,31 +3388,28 @@ pub(crate) mod tests { } } -#[cfg(all(test, feature = "_bench_unstable"))] -mod benches { +#[cfg(ldk_bench)] +pub mod benches { use super::*; - - use test::Bencher; use std::io::Read; + use criterion::{black_box, Criterion}; - #[bench] - fn read_network_graph(bench: &mut Bencher) { + pub fn read_network_graph(bench: &mut Criterion) { let logger = crate::util::test_utils::TestLogger::new(); let mut d = crate::routing::router::bench_utils::get_route_file().unwrap(); let mut v = Vec::new(); d.read_to_end(&mut v).unwrap(); - bench.iter(|| { - let _ = NetworkGraph::read(&mut std::io::Cursor::new(&v), &logger).unwrap(); - }); + bench.bench_function("read_network_graph", |b| b.iter(|| + NetworkGraph::read(&mut std::io::Cursor::new(black_box(&v)), &logger).unwrap() + )); } - #[bench] - fn write_network_graph(bench: &mut Bencher) { + pub fn write_network_graph(bench: &mut Criterion) { let logger = crate::util::test_utils::TestLogger::new(); let mut d = crate::routing::router::bench_utils::get_route_file().unwrap(); let net_graph = NetworkGraph::read(&mut d, &logger).unwrap(); - bench.iter(|| { - let _ = net_graph.encode(); - }); + bench.bench_function("write_network_graph", |b| b.iter(|| + black_box(&net_graph).encode() + )); } } diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 420fc3d202d..ef6fef0a7eb 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -1015,7 +1015,7 @@ 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, - #[cfg(all(not(feature = "_bench_unstable"), any(test, fuzzing)))] + #[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 // value_contribution_msat, which requires tracking it here. See comments below where it is @@ -1036,7 +1036,7 @@ impl<'a> core::fmt::Debug for PathBuildingHop<'a> { .field("path_penalty_msat", &self.path_penalty_msat) .field("path_htlc_minimum_msat", &self.path_htlc_minimum_msat) .field("cltv_expiry_delta", &self.candidate.cltv_expiry_delta()); - #[cfg(all(not(feature = "_bench_unstable"), any(test, fuzzing)))] + #[cfg(all(not(ldk_bench), any(test, fuzzing)))] let debug_struct = debug_struct .field("value_contribution_msat", &self.value_contribution_msat); debug_struct.finish() @@ -1570,14 +1570,14 @@ where L::Target: Logger { path_htlc_minimum_msat, path_penalty_msat: u64::max_value(), was_processed: false, - #[cfg(all(not(feature = "_bench_unstable"), any(test, fuzzing)))] + #[cfg(all(not(ldk_bench), any(test, fuzzing)))] value_contribution_msat, } }); #[allow(unused_mut)] // We only use the mut in cfg(test) let mut should_process = !old_entry.was_processed; - #[cfg(all(not(feature = "_bench_unstable"), any(test, fuzzing)))] + #[cfg(all(not(ldk_bench), any(test, fuzzing)))] { // In test/fuzzing builds, we do extra checks to make sure the skipping // of already-seen nodes only happens in cases we expect (see below). @@ -1648,13 +1648,13 @@ where L::Target: Logger { old_entry.fee_msat = 0; // This value will be later filled with hop_use_fee_msat of the following channel old_entry.path_htlc_minimum_msat = path_htlc_minimum_msat; old_entry.path_penalty_msat = path_penalty_msat; - #[cfg(all(not(feature = "_bench_unstable"), any(test, fuzzing)))] + #[cfg(all(not(ldk_bench), any(test, fuzzing)))] { old_entry.value_contribution_msat = value_contribution_msat; } did_add_update_path_to_src_node = true; } else if old_entry.was_processed && new_cost < old_cost { - #[cfg(all(not(feature = "_bench_unstable"), any(test, fuzzing)))] + #[cfg(all(not(ldk_bench), any(test, fuzzing)))] { // If we're skipping processing a node which was previously // processed even though we found another path to it with a @@ -6038,7 +6038,7 @@ mod tests { } } -#[cfg(all(test, not(feature = "no-std")))] +#[cfg(all(any(test, ldk_bench), not(feature = "no-std")))] pub(crate) mod bench_utils { use super::*; use std::fs::File; @@ -6068,7 +6068,18 @@ pub(crate) mod bench_utils { path.pop(); // target path.push("lightning"); path.push("net_graph-2023-01-18.bin"); - eprintln!("{}", path.to_str().unwrap()); + File::open(path) + }) + .or_else(|_| { // Fall back to guessing based on the binary location for a subcrate + // path is likely something like .../rust-lightning/bench/target/debug/deps/bench.. + let mut path = std::env::current_exe().unwrap(); + path.pop(); // bench... + path.pop(); // deps + path.pop(); // debug + path.pop(); // target + path.pop(); // bench + path.push("lightning"); + path.push("net_graph-2023-01-18.bin"); File::open(path) }) .map_err(|_| "Please fetch https://bitcoin.ninja/ldk-net_graph-v0.0.113-2023-01-18.bin and place it at lightning/net_graph-2023-01-18.bin"); @@ -6204,8 +6215,8 @@ pub(crate) mod bench_utils { } } -#[cfg(all(test, feature = "_bench_unstable", not(feature = "no-std")))] -mod benches { +#[cfg(ldk_bench)] +pub mod benches { use super::*; use crate::sign::{EntropySource, KeysManager}; use crate::ln::channelmanager; @@ -6216,60 +6227,63 @@ mod benches { use crate::util::logger::{Logger, Record}; use crate::util::test_utils::TestLogger; - use test::Bencher; + use criterion::Criterion; struct DummyLogger {} impl Logger for DummyLogger { fn log(&self, _record: &Record) {} } - - #[bench] - fn generate_routes_with_zero_penalty_scorer(bench: &mut Bencher) { + pub fn generate_routes_with_zero_penalty_scorer(bench: &mut Criterion) { let logger = TestLogger::new(); let network_graph = bench_utils::read_network_graph(&logger).unwrap(); let scorer = FixedPenaltyScorer::with_penalty(0); - generate_routes(bench, &network_graph, scorer, &(), InvoiceFeatures::empty(), 0); + generate_routes(bench, &network_graph, scorer, &(), InvoiceFeatures::empty(), 0, + "generate_routes_with_zero_penalty_scorer"); } - #[bench] - fn generate_mpp_routes_with_zero_penalty_scorer(bench: &mut Bencher) { + pub fn generate_mpp_routes_with_zero_penalty_scorer(bench: &mut Criterion) { let logger = TestLogger::new(); let network_graph = bench_utils::read_network_graph(&logger).unwrap(); let scorer = FixedPenaltyScorer::with_penalty(0); - generate_routes(bench, &network_graph, scorer, &(), channelmanager::provided_invoice_features(&UserConfig::default()), 0); + generate_routes(bench, &network_graph, scorer, &(), + channelmanager::provided_invoice_features(&UserConfig::default()), 0, + "generate_mpp_routes_with_zero_penalty_scorer"); } - #[bench] - fn generate_routes_with_probabilistic_scorer(bench: &mut Bencher) { + pub fn generate_routes_with_probabilistic_scorer(bench: &mut Criterion) { let logger = TestLogger::new(); let network_graph = bench_utils::read_network_graph(&logger).unwrap(); let params = ProbabilisticScoringFeeParameters::default(); let scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &network_graph, &logger); - generate_routes(bench, &network_graph, scorer, ¶ms, InvoiceFeatures::empty(), 0); + generate_routes(bench, &network_graph, scorer, ¶ms, InvoiceFeatures::empty(), 0, + "generate_routes_with_probabilistic_scorer"); } - #[bench] - fn generate_mpp_routes_with_probabilistic_scorer(bench: &mut Bencher) { + pub fn generate_mpp_routes_with_probabilistic_scorer(bench: &mut Criterion) { let logger = TestLogger::new(); let network_graph = bench_utils::read_network_graph(&logger).unwrap(); let params = ProbabilisticScoringFeeParameters::default(); let scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &network_graph, &logger); - generate_routes(bench, &network_graph, scorer, ¶ms, channelmanager::provided_invoice_features(&UserConfig::default()), 0); + generate_routes(bench, &network_graph, scorer, ¶ms, + channelmanager::provided_invoice_features(&UserConfig::default()), 0, + "generate_mpp_routes_with_probabilistic_scorer"); } - #[bench] - fn generate_large_mpp_routes_with_probabilistic_scorer(bench: &mut Bencher) { + pub fn generate_large_mpp_routes_with_probabilistic_scorer(bench: &mut Criterion) { let logger = TestLogger::new(); let network_graph = bench_utils::read_network_graph(&logger).unwrap(); let params = ProbabilisticScoringFeeParameters::default(); let scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &network_graph, &logger); - generate_routes(bench, &network_graph, scorer, ¶ms, channelmanager::provided_invoice_features(&UserConfig::default()), 100_000_000); + generate_routes(bench, &network_graph, scorer, ¶ms, + channelmanager::provided_invoice_features(&UserConfig::default()), 100_000_000, + "generate_large_mpp_routes_with_probabilistic_scorer"); } fn generate_routes( - bench: &mut Bencher, graph: &NetworkGraph<&TestLogger>, mut scorer: S, + bench: &mut Criterion, graph: &NetworkGraph<&TestLogger>, mut scorer: S, score_params: &S::ScoreParams, features: InvoiceFeatures, starting_amount: u64, + bench_name: &'static str, ) { let payer = bench_utils::payer_pubkey(); let keys_manager = KeysManager::new(&[0u8; 32], 42, 42); @@ -6280,11 +6294,11 @@ mod benches { // ...then benchmark finding paths between the nodes we learned. let mut idx = 0; - bench.iter(|| { + bench.bench_function(bench_name, |b| b.iter(|| { let (first_hop, params, amt) = &route_endpoints[idx % route_endpoints.len()]; assert!(get_route(&payer, params, &graph.read_only(), Some(&[first_hop]), *amt, &DummyLogger{}, &scorer, score_params, &random_seed_bytes).is_ok()); idx += 1; - }); + })); } } diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs index 9ba3ba556c8..bc4521fa861 100644 --- a/lightning/src/sign/mod.rs +++ b/lightning/src/sign/mod.rs @@ -1539,8 +1539,8 @@ pub fn dyn_sign() { let _signer: Box; } -#[cfg(all(test, feature = "_bench_unstable", not(feature = "no-std")))] -mod benches { +#[cfg(ldk_bench)] +pub mod benches { use std::sync::{Arc, mpsc}; use std::sync::mpsc::TryRecvError; use std::thread; @@ -1549,10 +1549,9 @@ mod benches { use bitcoin::Network; use crate::sign::{EntropySource, KeysManager}; - use test::Bencher; + use criterion::Criterion; - #[bench] - fn bench_get_secure_random_bytes(bench: &mut Bencher) { + pub fn bench_get_secure_random_bytes(bench: &mut Criterion) { let seed = [0u8; 32]; let now = Duration::from_secs(genesis_block(Network::Testnet).header.time as u64); let keys_manager = Arc::new(KeysManager::new(&seed, now.as_secs(), now.subsec_micros())); @@ -1578,11 +1577,8 @@ mod benches { stops.push(stop_sender); } - bench.iter(|| { - for _ in 1..100 { - keys_manager.get_secure_random_bytes(); - } - }); + bench.bench_function("get_secure_random_bytes", |b| b.iter(|| + keys_manager.get_secure_random_bytes())); for stop in stops { let _ = stop.send(()); @@ -1591,5 +1587,4 @@ mod benches { handle.join().unwrap(); } } - } diff --git a/lightning/src/sync/mod.rs b/lightning/src/sync/mod.rs index 1b2b9a739b8..348bd90274a 100644 --- a/lightning/src/sync/mod.rs +++ b/lightning/src/sync/mod.rs @@ -3,7 +3,7 @@ pub(crate) enum LockHeldState { HeldByThread, NotHeldByThread, - #[cfg(any(feature = "_bench_unstable", not(test)))] + #[cfg(any(ldk_bench, not(test)))] Unsupported, } @@ -20,20 +20,20 @@ pub(crate) trait LockTestExt<'a> { fn unsafe_well_ordered_double_lock_self(&'a self) -> Self::ExclLock; } -#[cfg(all(feature = "std", not(feature = "_bench_unstable"), test))] +#[cfg(all(feature = "std", not(ldk_bench), test))] mod debug_sync; -#[cfg(all(feature = "std", not(feature = "_bench_unstable"), test))] +#[cfg(all(feature = "std", not(ldk_bench), test))] pub use debug_sync::*; -#[cfg(all(feature = "std", not(feature = "_bench_unstable"), test))] +#[cfg(all(feature = "std", not(ldk_bench), test))] // Note that to make debug_sync's regex work this must not contain `debug_string` in the module name mod test_lockorder_checks; -#[cfg(all(feature = "std", any(feature = "_bench_unstable", not(test))))] +#[cfg(all(feature = "std", any(ldk_bench, not(test))))] pub(crate) mod fairrwlock; -#[cfg(all(feature = "std", any(feature = "_bench_unstable", not(test))))] +#[cfg(all(feature = "std", any(ldk_bench, not(test))))] pub use {std::sync::{Arc, Mutex, Condvar, MutexGuard, RwLock, RwLockReadGuard, RwLockWriteGuard}, fairrwlock::FairRwLock}; -#[cfg(all(feature = "std", any(feature = "_bench_unstable", not(test))))] +#[cfg(all(feature = "std", any(ldk_bench, not(test))))] mod ext_impl { use super::*; impl<'a, T: 'a> LockTestExt<'a> for Mutex { diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index e51bbb92719..b6f4aa5d43d 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -738,7 +738,7 @@ impl Logger for TestLogger { fn log(&self, record: &Record) { *self.lines.lock().unwrap().entry((record.module_path.to_string(), format!("{}", record.args))).or_insert(0) += 1; if record.level >= self.level { - #[cfg(feature = "std")] + #[cfg(all(not(ldk_bench), feature = "std"))] println!("{:<5} {} [{} : {}, {}] {}", record.level.to_string(), self.id, record.module_path, record.file, record.line, record.args); } } From 6ddc88b8d7d796ac8538b29714016a1c6e5df0ef Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 2 May 2023 17:04:11 +0000 Subject: [PATCH 4/5] Add trivial README to bench to describe how to run them. --- bench/README.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 bench/README.md diff --git a/bench/README.md b/bench/README.md new file mode 100644 index 00000000000..7d4cacc9e87 --- /dev/null +++ b/bench/README.md @@ -0,0 +1,6 @@ +This crate uses criterion to benchmark various LDK functions. + +It can be run as `RUSTFLAGS=--cfg=ldk_bench cargo bench`. + +For routing or other HashMap-bottlenecked functions, the `hashbrown` feature +should also be benchmarked. From 4b27cc486ceb8cd8361c0c6c33841e9443363167 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 2 May 2023 17:13:02 +0000 Subject: [PATCH 5/5] Update .gitignore to ignore benchmark data files --- .gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 7a889b5b823..22969fae328 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,7 @@ lightning-c-bindings/a.out Cargo.lock .idea lightning/target -lightning/ldk-net_graph-*.bin +lightning/net_graph-*.bin +lightning-rapid-gossip-sync/res/full_graph.lngossip lightning-custom-message/target no-std-check/target