Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: Run benchmarks with Ethernet and max MTUs #2183

Merged
merged 11 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 39 additions & 34 deletions .github/workflows/bench.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
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.
Expand Down Expand Up @@ -132,45 +134,48 @@ 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
done
# 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.
Expand Down
10 changes: 5 additions & 5 deletions neqo-bin/benches/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -20,18 +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 = env::var("MTU").map_or_else(|_| String::new(), |mtu| format!("/mtu-{mtu}"));
for Benchmark { name, requests } in [
Benchmark {
name: "1-conn/1-100mb-resp (aka. Download)".to_string(),
name: format!("1-conn/1-100mb-resp{mtu} (aka. Download)"),
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{mtu} (aka. RPS)"),
requests: vec![1; 10_000],
},
Benchmark {
name: "1-conn/1-1b-resp (aka. HPS)".to_string(),
name: format!("1-conn/1-1b-resp{mtu} (aka. HPS)"),
requests: vec![1; 1],
},
] {
Expand Down
10 changes: 7 additions & 3 deletions neqo-transport/benches/transfer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<impl AsRef<str>>) {
fn benchmark_transfer(c: &mut Criterion, label: &str, seed: Option<&impl AsRef<str>>) {
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
Expand Down Expand Up @@ -63,14 +63,18 @@ fn benchmark_transfer(c: &mut Criterion, label: &str, seed: &Option<impl AsRef<s
}

fn benchmark_transfer_variable(c: &mut Criterion) {
benchmark_transfer(c, "varying-seeds", &std::env::var("SIMULATION_SEED").ok());
benchmark_transfer(
c,
"varying-seeds",
std::env::var("SIMULATION_SEED").ok().as_ref(),
);
}

fn benchmark_transfer_fixed(c: &mut Criterion) {
benchmark_transfer(
c,
"same-seed",
&Some("62df6933ba1f543cece01db8f27fb2025529b27f93df39e19f006e1db3b8c843"),
Some(&"62df6933ba1f543cece01db8f27fb2025529b27f93df39e19f006e1db3b8c843"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you are passing &&str and changing the signature to &impl AsRef? That seems a bit "wat" to me.

impl AsRef<str> matches String and &str. Why does it need an extra &? That only reduces ergonomics.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clippy wants it

);
}

Expand Down
Loading