Replace std's unmaintained bench with criterion

TheBlueMatt · TheBlueMatt · commit 1701b021241f · 2023-05-11T06:11:49.000Z
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.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<InMemorySigner, &'a test_utils::TestChainSource,
@@ -9308,13 +9308,11 @@ pub mod bench {
 		fn chain_monitor(&self) -> 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<P: Persist<InMemorySigner>>(bench: &mut Bencher, persister_a: P, persister_b: P) {
+	pub fn bench_two_sends<P: Persist<InMemorySigner>>(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
@@ -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
diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs
diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs
diff --git a/lightning/src/sync/mod.rs b/lightning/src/sync/mod.rs
diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs
