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.

Author: TheBlueMatt

.github/workflows/build.yml

@@ -131,7 +131,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 cargo bench
+      - name: Run benchmarks with hashbrown on Rust ${{ matrix.toolchain }}
+        run: |
+          cd bench
+          RUSTFLAGS=--cfg=ldk_bench cargo bench --features hashbrown
 
   check_commits:
     runs-on: ubuntu-latest

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

bench/Cargo.toml

@@ -0,0 +1,24 @@
+[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"

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::chain::keysinterface::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);

lightning-persister/Cargo.toml

@@ -13,14 +13,13 @@ edition = "2018"
 all-features = true
 rustdoc-args = ["--cfg", "docsrs"]
 
-[features]
-_bench_unstable = ["lightning/_bench_unstable"]
-
 [dependencies]
 bitcoin = "0.29.0"
 lightning = { version = "0.0.114", path = "../lightning" }
 libc = "0.2"
 
+criterion = { version = "0.4", optional = true, default-features = false }
+
 [target.'cfg(windows)'.dependencies]
 winapi = { version = "0.3", features = ["winbase"] }
 

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);
 	}
 }

lightning-rapid-gossip-sync/Cargo.toml

@@ -13,11 +13,12 @@ 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.114", path = "../lightning", default-features = false }
 bitcoin = { version = "0.29.0", default-features = false }
 
+criterion = { version = "0.4", optional = true, default-features = false }
+
 [dev-dependencies]
 lightning = { version = "0.0.114", path = "../lightning", features = ["_test_utils"] }

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,9 +284,10 @@ mod tests {
 	}
 }
 
-#[cfg(all(test, feature = "_bench_unstable"))]
+#[cfg(ldk_bench)]
+/// Benches
 pub mod bench {
-	use test::Bencher;
+	use criterion::Criterion;
 
 	use bitcoin::Network;
 
@@ -299,13 +297,13 @@ pub mod bench {
 
 	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");
+			let sync_result = rapid_sync.sync_network_graph_with_file_path("../lightning-rapid-gossip-sync/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))]
@@ -316,7 +314,7 @@ pub mod bench {
 				#[cfg(require_route_graph_test)]
 				panic!("{}", error_string);
 			}
-			assert!(sync_result.is_ok())
-		});
+			assert!(sync_result.is_ok());
+		}));
 	}
 }

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 = []
 
 no-std = ["hashbrown", "bitcoin/no-std", "core2/alloc"]
 std = ["bitcoin/std"]
@@ -48,6 +47,8 @@ backtrace = { version = "0.3", optional = true }
 
 core2 = { version = "0.3.0", optional = true, default-features = false }
 
+criterion = { version = "0.4", optional = true, default-features = false }
+
 [dev-dependencies]
 hex = "0.4"
 regex = "1.5.6"

lightning/src/chain/keysinterface.rs

@@ -1483,8 +1483,8 @@ pub fn dyn_sign() {
 	let _signer: Box<dyn EcdsaChannelSigner>;
 }
 
-#[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;
@@ -1493,10 +1493,9 @@ mod benches {
 	use bitcoin::Network;
 	use crate::chain::keysinterface::{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()));
@@ -1522,11 +1521,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(());
@@ -1535,5 +1531,4 @@ mod benches {
 			handle.join().unwrap();
 		}
 	}
-
 }

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;
@@ -175,7 +174,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;

lightning/src/ln/channelmanager.rs

@@ -8827,7 +8827,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};
@@ -8847,7 +8847,7 @@ pub mod bench {
 
 	use crate::sync::{Arc, Mutex};
 
-	use test::Bencher;
+	use criterion::Criterion;
 
 	struct NodeHolder<'a, P: Persist<InMemorySigner>> {
 		node: &'a ChannelManager<
@@ -8859,13 +8859,11 @@ pub mod bench {
 			&'a test_utils::TestLogger>,
 	}
 
-	#[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.
@@ -9021,9 +9019,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);
-		});
+		}));
 	}
 }

lightning/src/ln/functional_test_utils.rs

@@ -1671,7 +1671,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())
@@ -1698,7 +1698,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();
@@ -1807,7 +1807,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);
@@ -1819,7 +1819,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);