mirror of
https://github.com/lightningdevkit/rust-lightning.git
synced 2025-03-15 15:39:09 +01:00
Make router benchmarks more realistic by not running test-only code
`cargo bench` sets `cfg(test)`, causing us to hit some test-only code in the router when benchmarking, throwing off our benchmarks substantially. Here we swap from the `unstable` feature to a more clearly internal feature (`_bench_unstable`) and also checking for it when enabling test-only code.
This commit is contained in:
parent
78c6154d9a
commit
c8e3078ff7
9 changed files with 17 additions and 17 deletions
2
.github/workflows/build.yml
vendored
2
.github/workflows/build.yml
vendored
|
@ -231,7 +231,7 @@ jobs:
|
|||
cd ..
|
||||
- name: Run benchmarks on Rust ${{ matrix.toolchain }}
|
||||
run: |
|
||||
cargo bench --features unstable
|
||||
cargo bench --features _bench_unstable
|
||||
|
||||
check_commits:
|
||||
runs-on: ubuntu-latest
|
||||
|
|
|
@ -9,7 +9,7 @@ Utilities to manage Rust-Lightning channel data persistence and retrieval.
|
|||
"""
|
||||
|
||||
[features]
|
||||
unstable = ["lightning/unstable"]
|
||||
_bench_unstable = ["lightning/_bench_unstable"]
|
||||
|
||||
[dependencies]
|
||||
bitcoin = "0.27"
|
||||
|
|
|
@ -3,8 +3,8 @@
|
|||
#![deny(broken_intra_doc_links)]
|
||||
#![deny(missing_docs)]
|
||||
|
||||
#![cfg_attr(all(test, feature = "unstable"), feature(test))]
|
||||
#[cfg(all(test, feature = "unstable"))] extern crate test;
|
||||
#![cfg_attr(all(test, feature = "_bench_unstable"), feature(test))]
|
||||
#[cfg(all(test, feature = "_bench_unstable"))] extern crate test;
|
||||
|
||||
mod util;
|
||||
|
||||
|
@ -362,7 +362,7 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
#[cfg(all(test, feature = "unstable"))]
|
||||
#[cfg(all(test, feature = "_bench_unstable"))]
|
||||
pub mod bench {
|
||||
use test::Bencher;
|
||||
|
||||
|
|
|
@ -24,7 +24,7 @@ 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 = []
|
||||
unstable = []
|
||||
_bench_unstable = []
|
||||
|
||||
no-std = ["hashbrown", "bitcoin/no-std", "core2/alloc"]
|
||||
std = ["bitcoin/std"]
|
||||
|
|
|
@ -30,8 +30,8 @@
|
|||
|
||||
#![cfg_attr(all(not(feature = "std"), not(test)), no_std)]
|
||||
|
||||
#![cfg_attr(all(any(test, feature = "_test_utils"), feature = "unstable"), feature(test))]
|
||||
#[cfg(all(any(test, feature = "_test_utils"), feature = "unstable"))] extern crate test;
|
||||
#![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");
|
||||
|
|
|
@ -7132,7 +7132,7 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
#[cfg(all(any(test, feature = "_test_utils"), feature = "unstable"))]
|
||||
#[cfg(all(any(test, feature = "_test_utils"), feature = "_bench_unstable"))]
|
||||
pub mod bench {
|
||||
use chain::Listen;
|
||||
use chain::chainmonitor::{ChainMonitor, Persist};
|
||||
|
|
|
@ -1137,7 +1137,7 @@ macro_rules! expect_pending_htlcs_forwardable_from_events {
|
|||
}}
|
||||
}
|
||||
|
||||
#[cfg(any(test, feature = "unstable"))]
|
||||
#[cfg(any(test, feature = "_bench_unstable"))]
|
||||
macro_rules! expect_payment_received {
|
||||
($node: expr, $expected_payment_hash: expr, $expected_payment_secret: expr, $expected_recv_value: expr) => {
|
||||
let events = $node.node.get_and_clear_pending_events();
|
||||
|
|
|
@ -2687,7 +2687,7 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
#[cfg(all(test, feature = "unstable"))]
|
||||
#[cfg(all(test, feature = "_bench_unstable"))]
|
||||
mod benches {
|
||||
use super::*;
|
||||
|
||||
|
|
|
@ -455,7 +455,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(any(test, feature = "fuzztarget"))]
|
||||
#[cfg(all(not(feature = "_bench_unstable"), any(test, feature = "fuzztarget")))]
|
||||
// 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
|
||||
|
@ -896,14 +896,14 @@ where L::Target: Logger {
|
|||
path_htlc_minimum_msat,
|
||||
path_penalty_msat: u64::max_value(),
|
||||
was_processed: false,
|
||||
#[cfg(any(test, feature = "fuzztarget"))]
|
||||
#[cfg(all(not(feature = "_bench_unstable"), any(test, feature = "fuzztarget")))]
|
||||
value_contribution_msat,
|
||||
}
|
||||
});
|
||||
|
||||
#[allow(unused_mut)] // We only use the mut in cfg(test)
|
||||
let mut should_process = !old_entry.was_processed;
|
||||
#[cfg(any(test, feature = "fuzztarget"))]
|
||||
#[cfg(all(not(feature = "_bench_unstable"), any(test, feature = "fuzztarget")))]
|
||||
{
|
||||
// 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).
|
||||
|
@ -992,13 +992,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(any(test, feature = "fuzztarget"))]
|
||||
#[cfg(all(not(feature = "_bench_unstable"), any(test, feature = "fuzztarget")))]
|
||||
{
|
||||
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(any(test, feature = "fuzztarget"))]
|
||||
#[cfg(all(not(feature = "_bench_unstable"), any(test, feature = "fuzztarget")))]
|
||||
{
|
||||
// If we're skipping processing a node which was previously
|
||||
// processed even though we found another path to it with a
|
||||
|
@ -4976,7 +4976,7 @@ pub(crate) mod test_utils {
|
|||
}
|
||||
}
|
||||
|
||||
#[cfg(all(test, feature = "unstable", not(feature = "no-std")))]
|
||||
#[cfg(all(test, feature = "_bench_unstable", not(feature = "no-std")))]
|
||||
mod benches {
|
||||
use super::*;
|
||||
use bitcoin::hashes::Hash;
|
||||
|
|
Loading…
Add table
Reference in a new issue