From 025e604dd8e7139eb17abfb4433bba23d5f13e3b Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Tue, 15 Oct 2024 16:13:47 +0200 Subject: [PATCH 01/10] ci: Run benchmarks with Ethernet and max MTUs --- .github/workflows/bench.yml | 73 ++++++++++++++++++++----------------- neqo-bin/benches/main.rs | 15 +++++--- 2 files changed, 48 insertions(+), 40 deletions(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index fcece8bd42..350f321b93 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -87,8 +87,10 @@ jobs: # # Run all benchmarks at elevated priority. taskset -c 0 nice -n -20 cargo "+$TOOLCHAIN" bench --workspace --exclude neqo-bin --features bench -- --noplot | tee results.txt - nice -n -20 cargo "+$TOOLCHAIN" bench --package neqo-bin --features bench -- --noplot | tee -a results.txt - + for MTU in 1500 65536; do + sudo ip link set dev lo mtu "$MTU" + nice -n -20 cargo "+$TOOLCHAIN" bench --package neqo-bin --features bench -- --noplot | tee -a results.txt + done # Compare various configurations of neqo against msquic, and gather perf data # during the hyperfine runs. @@ -132,37 +134,40 @@ jobs: fi } - for server in msquic neqo; do - for client in msquic neqo; do - # msquic doesn't let us configure the congestion control or pacing. - if [ "$client" == "msquic" ] && [ "$server" == "msquic" ]; then - cc_opt=("") - pacing_opt=("") - else - cc_opt=("reno" "cubic") - pacing_opt=("on" "") - fi - for cc in "${cc_opt[@]}"; do - for pacing in "${pacing_opt[@]}"; do - # Make a tag string for this test, for the results. - TAG="$client,$server,$cc,$pacing" - echo "Running benchmarks for $TAG" | tee -a comparison.txt - transmogrify "${server_cmd[$server]}" "$cc" "$pacing" - # shellcheck disable=SC2086 - taskset -c 0 nice -n -20 \ - perf $PERF_OPT -o "$client-$server$EXT.server.perf" $CMD & - PID=$! - transmogrify "${client_cmd[$client]}" "$cc" "$pacing" - # shellcheck disable=SC2086 - taskset -c 1 nice -n -20 \ - perf $PERF_OPT -o "$client-$server$EXT.client.perf" \ - hyperfine -N --output null -w 1 -s "sleep 1" -n "$TAG" -u millisecond --export-markdown step.md "$CMD" | - tee -a comparison.txt - echo >> comparison.txt - kill $PID - cat step.md >> steps.md - # Sanity check the size of the last retrieved file. - [ "$(wc -c <"$SIZE")" -eq "$SIZE" ] || exit 1 + for mtu in 1500 65536; do + sudo ip link set dev lo mtu "$mtu" + for server in msquic neqo; do + for client in msquic neqo; do + # msquic doesn't let us configure the congestion control or pacing. + if [ "$client" == "msquic" ] && [ "$server" == "msquic" ]; then + cc_opt=("") + pacing_opt=("") + else + cc_opt=("reno" "cubic") + pacing_opt=("on" "") + fi + for cc in "${cc_opt[@]}"; do + for pacing in "${pacing_opt[@]}"; do + # Make a tag string for this test, for the results. + TAG="$client,$server,$cc,$pacing,$mtu" + echo "Running benchmarks for $TAG" | tee -a comparison.txt + transmogrify "${server_cmd[$server]}" "$cc" "$pacing" + # shellcheck disable=SC2086 + taskset -c 0 nice -n -20 \ + perf $PERF_OPT -o "$client-$server$EXT.server.perf" $CMD & + PID=$! + transmogrify "${client_cmd[$client]}" "$cc" "$pacing" + # shellcheck disable=SC2086 + taskset -c 1 nice -n -20 \ + perf $PERF_OPT -o "$client-$server$EXT.client.perf" \ + hyperfine -N --output null -w 1 -s "sleep 1" -n "$TAG" -u millisecond --export-markdown step.md "$CMD" | + tee -a comparison.txt + echo >> comparison.txt + kill $PID + cat step.md >> steps.md + # Sanity check the size of the last retrieved file. + [ "$(wc -c <"$SIZE")" -eq "$SIZE" ] || exit 1 + done done done done @@ -170,7 +175,7 @@ jobs: # Merge the results tables generated by hyperfine into a single table. echo "Transfer of $SIZE bytes over loopback." > comparison.md awk '(!/^\| Command/ || !c++) && (!/^\|:/ || !d++)' < steps.md |\ - sed -E 's/`//g; s/^\|:/\|:---\|:---\|:---\|:/g; s/,/ \| /g; s/^\| Command/\| Client \| Server \| CC \| Pacing/g' >> comparison.md + sed -E 's/`//g; s/^\|:/\|:---\|:---\|:---\|:/g; s/,/ \| /g; s/^\| Command/\| Client \| Server \| CC \| Pacing \| MTU/g' >> comparison.md rm -r "$TMP" # Re-enable turboboost, hyperthreading and use powersave governor. diff --git a/neqo-bin/benches/main.rs b/neqo-bin/benches/main.rs index 4237c13408..f3c1ff940f 100644 --- a/neqo-bin/benches/main.rs +++ b/neqo-bin/benches/main.rs @@ -4,7 +4,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use std::{path::PathBuf, str::FromStr}; +use std::{env, path::PathBuf, str::FromStr}; use criterion::{criterion_group, criterion_main, BatchSize, Criterion, Throughput}; use neqo_bin::{client, server}; @@ -20,18 +20,22 @@ fn transfer(c: &mut Criterion) { neqo_crypto::init_db(PathBuf::from_str("../test-fixture/db").unwrap()).unwrap(); let done_sender = spawn_server(); - + let mtu = if let Ok(mtu) = env::var("MTU") { + format!("/mtu-{}", mtu) + } else { + "".to_string() + }; for Benchmark { name, requests } in [ Benchmark { - name: "1-conn/1-100mb-resp (aka. Download)".to_string(), + name: format!("1-conn/1-100mb-resp{} (aka. Download)", mtu), requests: vec![100 * 1024 * 1024], }, Benchmark { - name: "1-conn/10_000-parallel-1b-resp (aka. RPS)".to_string(), + name: format!("1-conn/10_000-parallel-1b-resp{} (aka. RPS)", mtu), requests: vec![1; 10_000], }, Benchmark { - name: "1-conn/1-1b-resp (aka. HPS)".to_string(), + name: format!("1-conn/1-1b-resp{} (aka. HPS)", mtu), requests: vec![1; 1], }, ] { @@ -57,7 +61,6 @@ fn transfer(c: &mut Criterion) { done_sender.send(()).unwrap(); } -#[allow(clippy::redundant_pub_crate)] // Bug in clippy nursery? Not sure how this lint could fire here. fn spawn_server() -> tokio::sync::oneshot::Sender<()> { let (done_sender, mut done_receiver) = tokio::sync::oneshot::channel(); std::thread::spawn(move || { From 76cd77ee3655b9d188996b6652ed63816cf5f0fb Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Tue, 15 Oct 2024 16:37:37 +0200 Subject: [PATCH 02/10] Fixes --- neqo-bin/benches/main.rs | 13 +++++-------- neqo-transport/benches/transfer.rs | 10 +++++++--- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/neqo-bin/benches/main.rs b/neqo-bin/benches/main.rs index f3c1ff940f..8793bf0928 100644 --- a/neqo-bin/benches/main.rs +++ b/neqo-bin/benches/main.rs @@ -20,22 +20,18 @@ fn transfer(c: &mut Criterion) { neqo_crypto::init_db(PathBuf::from_str("../test-fixture/db").unwrap()).unwrap(); let done_sender = spawn_server(); - let mtu = if let Ok(mtu) = env::var("MTU") { - format!("/mtu-{}", mtu) - } else { - "".to_string() - }; + let mtu = env::var("MTU").map_or_else(|_| String::new(), |mtu| format!("/mtu-{mtu}")); for Benchmark { name, requests } in [ Benchmark { - name: format!("1-conn/1-100mb-resp{} (aka. Download)", mtu), + name: format!("1-conn/1-100mb-resp{mtu} (aka. Download)"), requests: vec![100 * 1024 * 1024], }, Benchmark { - name: format!("1-conn/10_000-parallel-1b-resp{} (aka. RPS)", mtu), + name: format!("1-conn/10_000-parallel-1b-resp{mtu} (aka. RPS)"), requests: vec![1; 10_000], }, Benchmark { - name: format!("1-conn/1-1b-resp{} (aka. HPS)", mtu), + name: format!("1-conn/1-1b-resp{mtu} (aka. HPS)"), requests: vec![1; 1], }, ] { @@ -61,6 +57,7 @@ fn transfer(c: &mut Criterion) { done_sender.send(()).unwrap(); } +#[allow(clippy::redundant_pub_crate)] // Bug in clippy nursery? Not sure how this lint could fire here. fn spawn_server() -> tokio::sync::oneshot::Sender<()> { let (done_sender, mut done_receiver) = tokio::sync::oneshot::channel(); std::thread::spawn(move || { diff --git a/neqo-transport/benches/transfer.rs b/neqo-transport/benches/transfer.rs index be4876cc9e..f95d26e371 100644 --- a/neqo-transport/benches/transfer.rs +++ b/neqo-transport/benches/transfer.rs @@ -21,7 +21,7 @@ const ZERO: Duration = Duration::from_millis(0); const JITTER: Duration = Duration::from_millis(10); const TRANSFER_AMOUNT: usize = 1 << 22; // 4Mbyte -fn benchmark_transfer(c: &mut Criterion, label: &str, seed: &Option>) { +fn benchmark_transfer(c: &mut Criterion, label: &str, seed: Option<&impl AsRef>) { for pacing in [false, true] { let mut group = c.benchmark_group(format!("transfer/pacing-{pacing}")); // Don't let criterion calculate throughput, as that's based on wall-clock time, not @@ -63,14 +63,18 @@ fn benchmark_transfer(c: &mut Criterion, label: &str, seed: &Option Date: Tue, 15 Oct 2024 17:03:25 +0200 Subject: [PATCH 03/10] Export --- .github/workflows/bench.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index 350f321b93..f328e8089c 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -89,7 +89,7 @@ jobs: taskset -c 0 nice -n -20 cargo "+$TOOLCHAIN" bench --workspace --exclude neqo-bin --features bench -- --noplot | tee results.txt for MTU in 1500 65536; do sudo ip link set dev lo mtu "$MTU" - nice -n -20 cargo "+$TOOLCHAIN" bench --package neqo-bin --features bench -- --noplot | tee -a results.txt + MTU=$MTU nice -n -20 cargo "+$TOOLCHAIN" bench --package neqo-bin --features bench -- --noplot | tee -a results.txt done # Compare various configurations of neqo against msquic, and gather perf data From d1be7d4ec255c1a58a7a6d8d27efed634cd9a089 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Fri, 18 Oct 2024 11:04:19 +0300 Subject: [PATCH 04/10] Try with len that is multiple of 8 --- .github/workflows/bench.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index f328e8089c..40eb2f747d 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -134,7 +134,7 @@ jobs: fi } - for mtu in 1500 65536; do + for mtu in 1496 65536; do sudo ip link set dev lo mtu "$mtu" for server in msquic neqo; do for client in msquic neqo; do @@ -180,7 +180,10 @@ jobs: # Re-enable turboboost, hyperthreading and use powersave governor. - name: Restore machine - run: sudo /root/bin/unprep.sh + run: | + sudo /root/bin/unprep.sh + # In case the previous test failed: + sudo ip link set dev lo mtu 65536 if: success() || failure() || cancelled() - name: Post-process perf data From f548144f5f642f36bbd76f0bf512413812238124 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Fri, 18 Oct 2024 11:47:16 +0300 Subject: [PATCH 05/10] 2000 --- .github/workflows/bench.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index 40eb2f747d..2440571df2 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -134,7 +134,7 @@ jobs: fi } - for mtu in 1496 65536; do + for mtu in 2000 65536; do sudo ip link set dev lo mtu "$mtu" for server in msquic neqo; do for client in msquic neqo; do From 137fce949ff6aee233d1e4342232a4071f6dcfac Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Fri, 18 Oct 2024 13:08:05 +0300 Subject: [PATCH 06/10] 1600 --- .github/workflows/bench.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index 2440571df2..bf90c9c731 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -134,7 +134,7 @@ jobs: fi } - for mtu in 2000 65536; do + for mtu in 1600 65536; do sudo ip link set dev lo mtu "$mtu" for server in msquic neqo; do for client in msquic neqo; do @@ -175,7 +175,7 @@ jobs: # Merge the results tables generated by hyperfine into a single table. echo "Transfer of $SIZE bytes over loopback." > comparison.md awk '(!/^\| Command/ || !c++) && (!/^\|:/ || !d++)' < steps.md |\ - sed -E 's/`//g; s/^\|:/\|:---\|:---\|:---\|:/g; s/,/ \| /g; s/^\| Command/\| Client \| Server \| CC \| Pacing \| MTU/g' >> comparison.md + sed -E 's/`//g; s/^\|:/\|:---\|:---\|:---\|:---\|:/g; s/,/ \| /g; s/^\| Command/\| Client \| Server \| CC \| Pacing \| MTU/g' >> comparison.md rm -r "$TMP" # Re-enable turboboost, hyperthreading and use powersave governor. From 3f3208c3aab34d46e994069c8e996e9ef005b9f2 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Fri, 18 Oct 2024 14:17:03 +0300 Subject: [PATCH 07/10] Try more values --- .github/workflows/bench.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index bf90c9c731..0c878fbdfc 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -134,7 +134,7 @@ jobs: fi } - for mtu in 1600 65536; do + for mtu in 1488 1500 1504 1520 1536 1600 65536; do sudo ip link set dev lo mtu "$mtu" for server in msquic neqo; do for client in msquic neqo; do From 1398f125454342473f8b3f7c282de236eaeadbcd Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Fri, 18 Oct 2024 14:45:23 +0300 Subject: [PATCH 08/10] Again --- .github/workflows/bench.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index 0c878fbdfc..d28fdd9e42 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -134,7 +134,7 @@ jobs: fi } - for mtu in 1488 1500 1504 1520 1536 1600 65536; do + for mtu in 1504 1520 1536 1600 65536; do sudo ip link set dev lo mtu "$mtu" for server in msquic neqo; do for client in msquic neqo; do From 999aba44da3398d7668ac13325028678e779ac9d Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Fri, 18 Oct 2024 16:08:15 +0300 Subject: [PATCH 09/10] 1488 --- .github/workflows/bench.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index d28fdd9e42..0a0d61a987 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -134,7 +134,7 @@ jobs: fi } - for mtu in 1504 1520 1536 1600 65536; do + for mtu in 1488 1504 65536; do sudo ip link set dev lo mtu "$mtu" for server in msquic neqo; do for client in msquic neqo; do From 0f4b5783df851efb7a7c7e1ad5e7cd8b1ebfd94d Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Fri, 18 Oct 2024 17:29:43 +0300 Subject: [PATCH 10/10] 1504 it is for now --- .github/workflows/bench.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index 0a0d61a987..b2e7740cd0 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -134,7 +134,8 @@ jobs: fi } - for mtu in 1488 1504 65536; do + # See https://github.com/microsoft/msquic/issues/4618#issuecomment-2422611592 + for mtu in 1504 65536; do sudo ip link set dev lo mtu "$mtu" for server in msquic neqo; do for client in msquic neqo; do