From a68dd1c2a1aead9117b808dc22329ca928d86cd8 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Sat, 16 Mar 2024 05:57:12 +0200 Subject: [PATCH 1/5] test: Make `criterion` calculate transfer benchmark throughputs (#1738) * test: Make `criterion` calculate transfer benchmark throughputs Like: ``` transfer/Run multiple transfers with varying seeds time: [213.61 ms 224.95 ms 235.29 ms] thrpt: [17.000 MiB/s 17.782 MiB/s 18.726 MiB/s] change: time: [-17.204% -6.9690% +1.8324%] (p = 0.21 > 0.05) thrpt: [-1.7994% +7.4910% +20.778%] No change in performance detected. Found 5 outliers among 100 measurements (5.00%) 5 (5.00%) low mild transfer/Run multiple transfers with the same seed time: [175.41 ms 188.40 ms 201.32 ms] thrpt: [19.869 MiB/s 21.231 MiB/s 22.803 MiB/s] change: time: [-5.9393% +5.5417% +18.399%] (p = 0.35 > 0.05) thrpt: [-15.540% -5.2507% +6.3144%] No change in performance detected. Found 14 outliers among 100 measurements (14.00%) 8 (8.00%) low mild 5 (5.00%) high mild 1 (1.00%) high severe ``` * This doesn't actually run `cargo bench`, it runs `cargo flamegraph` --- .github/workflows/bench.yml | 7 ------- neqo-transport/benches/transfer.rs | 7 +++++-- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index 72b835f843..81ef297a9e 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -62,16 +62,9 @@ jobs: # Work around https://github.com/flamegraph-rs/flamegraph/issues/248 by passing explicit perf arguments. - name: Profile cargo bench transfer run: | - # This re-runs part of the previous step, and would hence overwrite part of the criterion results. - # Avoid that by shuffling the directories around so this run uses its own results directory. - mv target/criterion target/criterion-bench - mv target/criterion-transfer-profile target/criterion || true taskset -c 0 nice -n -20 \ cargo "+$TOOLCHAIN" flamegraph -v -c "$PERF_CMD" --features bench --bench transfer -- \ --bench --exact "Run multiple transfers with varying seeds" --noplot - # And now restore the directories. - mv target/criterion target/criterion-transfer-profile - mv target/criterion-bench target/criterion - name: Profile client/server transfer run: | diff --git a/neqo-transport/benches/transfer.rs b/neqo-transport/benches/transfer.rs index 444f738f9c..b13075a4ff 100644 --- a/neqo-transport/benches/transfer.rs +++ b/neqo-transport/benches/transfer.rs @@ -6,7 +6,7 @@ use std::time::Duration; -use criterion::{criterion_group, criterion_main, BatchSize::SmallInput, Criterion}; +use criterion::{criterion_group, criterion_main, BatchSize::SmallInput, Criterion, Throughput}; use test_fixture::{ boxed, sim::{ @@ -21,7 +21,9 @@ const JITTER: Duration = Duration::from_millis(10); const TRANSFER_AMOUNT: usize = 1 << 22; // 4Mbyte fn benchmark_transfer(c: &mut Criterion, label: &str, seed: Option>) { - c.bench_function(label, |b| { + let mut group = c.benchmark_group("transfer"); + group.throughput(Throughput::Bytes(u64::try_from(TRANSFER_AMOUNT).unwrap())); + group.bench_function(label, |b| { b.iter_batched( || { let nodes = boxed![ @@ -44,6 +46,7 @@ fn benchmark_transfer(c: &mut Criterion, label: &str, seed: Option Date: Sat, 16 Mar 2024 14:17:48 +1000 Subject: [PATCH 2/5] Strip ANSI escapes from Markdown output --- .github/workflows/mutants.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/mutants.yml b/.github/workflows/mutants.yml index 90001bd8b7..cd28cfe604 100644 --- a/.github/workflows/mutants.yml +++ b/.github/workflows/mutants.yml @@ -12,7 +12,7 @@ concurrency: cancel-in-progress: true jobs: - incremental-mutants: + mutants: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 @@ -40,14 +40,14 @@ jobs: git diff origin/${{ github.base_ref }}.. > pr.diff set -o pipefail cargo mutants --test-tool=nextest --no-shuffle -j 2 -vV --in-diff pr.diff | tee results.txt || true - echo 'TITLE="Incremental Mutants"' >> "$GITHUB_ENV" + echo 'TITLE=Incremental Mutants' >> "$GITHUB_ENV" - name: Find mutants if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' run: | set -o pipefail cargo mutants --test-tool=nextest -vV --in-place | tee results.txt || true - echo 'TITLE="All Mutants"' >> "$GITHUB_ENV" + echo 'TITLE=All Mutants' >> "$GITHUB_ENV" - name: Post step summary if: always() @@ -56,7 +56,7 @@ jobs: echo "### $TITLE" echo "See https://mutants.rs/using-results.html for more information." echo '```' - cat results.txt + cat results.txt | sed 's/[^[:print:]]//g' echo '```' } > "$GITHUB_STEP_SUMMARY" From 83e673335875d426faed91fc6e095ef521565f1d Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Sat, 16 Mar 2024 14:19:01 +1000 Subject: [PATCH 3/5] Fix `actionlint` issue --- .github/workflows/mutants.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/mutants.yml b/.github/workflows/mutants.yml index cd28cfe604..e11eb96f21 100644 --- a/.github/workflows/mutants.yml +++ b/.github/workflows/mutants.yml @@ -56,7 +56,7 @@ jobs: echo "### $TITLE" echo "See https://mutants.rs/using-results.html for more information." echo '```' - cat results.txt | sed 's/[^[:print:]]//g' + sed 's/[^[:print:]]//g' results.txt echo '```' } > "$GITHUB_STEP_SUMMARY" From 9ff375129b9a84b9589a5dbecc87117e0c92337b Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Sat, 16 Mar 2024 06:44:49 +0200 Subject: [PATCH 4/5] build: Enable "fat" LTO for release builds (#1751) Because that's what Firefox does: https://searchfox.org/mozilla-central/source/config/makefiles/rust.mk#95 --- Cargo.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 35916da5b1..e5bec00796 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,6 +25,9 @@ rust-version = "1.74.0" [workspace.lints.clippy] pedantic = { level = "warn", priority = -1 } +[profile.release] +lto = "fat" + [profile.bench] # Inherits from the "release" profile, so just provide overrides here: # https://doc.rust-lang.org/cargo/reference/profiles.html#release From ec4ec8e3019a55f6cf1bb11709a76b40205c5b0e Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Sat, 16 Mar 2024 21:35:06 +1000 Subject: [PATCH 5/5] Properly strip ANSI sequences from Markdown --- .github/workflows/mutants.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/mutants.yml b/.github/workflows/mutants.yml index e11eb96f21..4db2e4b925 100644 --- a/.github/workflows/mutants.yml +++ b/.github/workflows/mutants.yml @@ -56,7 +56,7 @@ jobs: echo "### $TITLE" echo "See https://mutants.rs/using-results.html for more information." echo '```' - sed 's/[^[:print:]]//g' results.txt + sed 's/\x1b\[[0-9;]*[mGKHF]//g' results.txt echo '```' } > "$GITHUB_STEP_SUMMARY"