Skip to content

Commit

Permalink
ci: Compare performance to msquic (#1750)
Browse files Browse the repository at this point in the history 
* ci: Compare performance to msquic

In progress

* Make it run

* Fixes

* Fix path

* Use msquic `secnetperf`

* Fix actionlint

* Sigh

* Retry

* Better reporting

* Debug

* Retry

* Sigh^2

* Fix

* Fixes

* Fixes

* Update bench.yml

* Fix

* Squashed commit of the following:

commit 9ca5ecc
Merge: f50f414 f408321
Author: Lars Eggert <lars@eggert.org>
Date:   Fri Mar 15 00:12:29 2024 +0200

    Merge branch 'main' into ci-bench-cc

    Signed-off-by: Lars Eggert <lars@eggert.org>

commit f50f414
Merge: 8e5290b bc262a5
Author: Lars Eggert <lars@eggert.org>
Date:   Wed Mar 13 07:59:01 2024 +0200

    Merge branch 'main' into ci-bench-cc

commit 8e5290b
Merge: f0cd19e 2ff9742
Author: Lars Eggert <lars@eggert.org>
Date:   Tue Mar 12 22:42:54 2024 +0200

    Merge branch 'main' into ci-bench-cc

commit f0cd19e
Merge: b2bb855 17c4175
Author: Lars Eggert <lars@eggert.org>
Date:   Tue Mar 12 21:54:08 2024 +0200

    Merge branch 'main' into ci-bench-cc

commit b2bb855
Merge: d072504 4ea2c56
Author: Lars Eggert <lars@eggert.org>
Date:   Tue Mar 12 17:25:13 2024 +0200

    Merge branch 'ci-bench-cc' of github.com:larseggert/neqo into ci-bench-cc

commit d072504
Author: Lars Eggert <lars@eggert.org>
Date:   Tue Mar 12 17:24:52 2024 +0200

    Reorder things so `results.ms` is included in the exported artifact

commit 4ea2c56
Merge: c82ff3a 5c72890
Author: Lars Eggert <lars@eggert.org>
Date:   Tue Mar 12 17:18:37 2024 +0200

    Merge branch 'main' into ci-bench-cc

commit c82ff3a
Author: Lars Eggert <lars@eggert.org>
Date:   Tue Mar 12 16:41:59 2024 +0200

    `killall` -> `pkill`

commit d37e706
Author: Lars Eggert <lars@eggert.org>
Date:   Tue Mar 12 16:37:50 2024 +0200

    Go back to `killall`

commit 11320d0
Author: Lars Eggert <lars@eggert.org>
Date:   Tue Mar 12 16:11:38 2024 +0200

    No -INT

commit 407bd4f
Author: Lars Eggert <lars@eggert.org>
Date:   Tue Mar 12 14:33:52 2024 +0200

    kill -> killall

    Also reduce test transfer size.

commit 9d3a8b7
Author: Lars Eggert <lars@eggert.org>
Date:   Tue Mar 12 13:57:51 2024 +0200

    Use temp dir, and fix path error

commit 84e2206
Merge: 925cc12 b0d816a
Author: Lars Eggert <lars@eggert.org>
Date:   Tue Mar 12 11:10:41 2024 +0200

    Merge branch 'main' into ci-bench-cc

commit 925cc12
Merge: 3241f93 5889038
Author: Lars Eggert <lars@eggert.org>
Date:   Tue Mar 12 11:05:42 2024 +0200

    Merge branch 'main' into ci-bench-cc

commit 3241f93
Merge: 02620a7 d48fbed
Author: Lars Eggert <lars@eggert.org>
Date:   Tue Mar 12 09:59:24 2024 +0200

    Merge branch 'main' into ci-bench-cc

    Signed-off-by: Lars Eggert <lars@eggert.org>

commit 02620a7
Author: Lars Eggert <lars@eggert.org>
Date:   Tue Mar 12 09:57:33 2024 +0200

    Try to kill via `$!`

commit b32ce9e
Merge: 9ea3a99 db1dbb2
Author: Lars Eggert <lars@eggert.org>
Date:   Tue Mar 12 09:15:18 2024 +0200

    Merge branch 'ci-bench-cc' of github.com:larseggert/neqo into ci-bench-cc

commit 9ea3a99
Author: Lars Eggert <lars@eggert.org>
Date:   Tue Mar 12 09:15:05 2024 +0200

    Address comments from @martinthomson

commit db1dbb2
Merge: 681bbb7 869afea
Author: Lars Eggert <lars@eggert.org>
Date:   Mon Mar 11 19:33:53 2024 +0200

    Merge branch 'main' into ci-bench-cc

commit 681bbb7
Merge: bd742af 532dcc5
Author: Lars Eggert <lars@eggert.org>
Date:   Mon Mar 11 18:21:06 2024 +0200

    Merge branch 'main' into ci-bench-cc

    Signed-off-by: Lars Eggert <lars@eggert.org>

commit bd742af
Author: Lars Eggert <lars@eggert.org>
Date:   Mon Mar 11 17:00:34 2024 +0200

    mkdir -p

commit bc7b99f
Author: Lars Eggert <lars@eggert.org>
Date:   Mon Mar 11 16:29:14 2024 +0200

    Fix

commit e7bf509
Merge: de64b3e cbd4441
Author: Lars Eggert <lars@eggert.org>
Date:   Mon Mar 11 16:27:56 2024 +0200

    Merge branch 'main' into ci-bench-cc

commit de64b3e
Author: Lars Eggert <lars@eggert.org>
Date:   Mon Mar 11 16:00:19 2024 +0200

    Wait for output before continuing

commit 12386a3
Author: Lars Eggert <lars@eggert.org>
Date:   Mon Mar 11 15:25:40 2024 +0200

    ci: Benchmark NewReno and Cubic

* Fixes

* Fix

* Fixes

* Tweaks

* Debug

* actionlint

* Fix sed

* Pacing changed

* Fixes

* msquic server exits with non-zero

* echo

* Again

* Again

* A new hope

* Finalize

* Fixes

* ls

* Add comments

* Report transfer size

* Again

* Quiet the server down

* Remove debug output

* Reformat table

* Fix

* Fix table

* Mac sed != GNU sed

* Avoid piping commands into themselves

* Fix comment

---------

Signed-off-by: Lars Eggert <lars@eggert.org>
  • Loading branch information
@larseggert
larseggert authored Mar 25, 2024
1 parent 50876af commit 76630a5
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 45 deletions.
2 changes: 1 addition & 1 deletion .github/actions/rust/action.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ runs:

- name: Install Rust tools
shell: bash
run: cargo +${{ inputs.version }} binstall --no-confirm cargo-llvm-cov cargo-nextest flamegraph cargo-hack cargo-mutants
run: cargo +${{ inputs.version }} binstall --no-confirm cargo-llvm-cov cargo-nextest flamegraph cargo-hack cargo-mutants hyperfine

# sccache slows CI down, so we leave it disabled.
# Leaving the steps below commented out, so we can re-evaluate enabling it later.
Expand Down
158 changes: 115 additions & 43 deletions .github/workflows/bench.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ env:
RUST_BACKTRACE: 1
TOOLCHAIN: nightly
RUSTFLAGS: -C link-arg=-fuse-ld=lld -C link-arg=-Wl,--no-rosegment, -C force-frame-pointers=yes
PERF_CMD: record -o perf.data -F997 --call-graph fp -g
PERF_OPT: record -F997 --call-graph fp -g

jobs:
bench:
Expand All @@ -23,9 +23,17 @@ jobs:
shell: bash

steps:
- name: Checkout
- name: Checkout neqo
uses: actions/checkout@v4

- name: Checkout msquic
uses: actions/checkout@v4
with:
repository: microsoft/msquic
ref: main
path: msquic
submodules: true

- name: Set PATH
run: echo "/home/bench/.cargo/bin" >> "${GITHUB_PATH}"

Expand All @@ -38,10 +46,17 @@ jobs:
- name: Fetch and build NSS and NSPR
uses: ./.github/actions/nss

- name: Build
- name: Build neqo
run: |
cargo "+$TOOLCHAIN" bench --features bench --no-run
cargo "+$TOOLCHAIN" build --release --bin neqo-client --bin neqo-server
cargo "+$TOOLCHAIN" build --release
- name: Build msquic
run: |
mkdir -p msquic/build
cd msquic/build
cmake -GNinja -DCMAKE_BUILD_TYPE=RelWithDebInfo -DQUIC_BUILD_TOOLS=1 -DQUIC_BUILD_PERF=1 ..
cmake --build .
- name: Download cached main-branch results
id: criterion-cache
Expand All @@ -61,56 +76,107 @@ jobs:
taskset -c 0 nice -n -20 \
cargo "+$TOOLCHAIN" bench --features bench -- --noplot | tee results.txt
# Pin the transfer benchmark to core 0 and run it at elevated priority inside perf.
# Work around https://github.com/flamegraph-rs/flamegraph/issues/248 by passing explicit perf arguments.
- name: Profile cargo bench transfer
run: |
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
- name: Profile client/server transfer
run: |
{ mkdir server; \
cd server; \
taskset -c 0 nice -n -20 \
cargo "+$TOOLCHAIN" flamegraph -v -c "$PERF_CMD" \
--bin neqo-server -- --db ../test-fixture/db "$HOST:4433" || true; } &
mkdir client; \
cd client; \
time taskset -c 1 nice -n -20 \
cargo "+$TOOLCHAIN" flamegraph -v -c "$PERF_CMD" \
--bin neqo-client -- --output-dir . "https://$HOST:4433/$SIZE"
killall -INT neqo-server
cd ${{ github.workspace }}
[ "$(wc -c < client/"$SIZE")" -eq "$SIZE" ] || exit 1
# Compare various configurations of neqo against msquic, and gather perf data
# during the hyperfine runs.
- name: Compare neqo and msquic
env:
HOST: localhost
SIZE: 1073741824 # 1 GB
HOST: 127.0.0.1
PORT: 4433
SIZE: 134217728 # 128 MB
run: |
TMP=$(mktemp -d)
# Make a cert and key for msquic.
openssl req -nodes -new -x509 -keyout "$TMP/key" -out "$TMP/cert" -subj "/CN=DOMAIN" 2>/dev/null
# Make a test file for msquic to serve.
truncate -s "$SIZE" "$TMP/$SIZE"
# Define the commands to run for each client and server.
declare -A client_cmd=(
["neqo"]="target/release/neqo-client _cc _pacing --output-dir . -o -a hq-interop -Q 1 https://$HOST:$PORT/$SIZE"
["msquic"]="msquic/build/bin/Release/quicinterop -test:D -custom:$HOST -port:$PORT -urls:https://$HOST:$PORT/$SIZE"
)
declare -A server_cmd=(
["neqo"]="target/release/neqo-server _cc _pacing -o -a hq-interop -Q 1 $HOST:$PORT 2> /dev/null"
["msquic"]="msquic/build/bin/Release/quicinteropserver -root:$TMP -listen:$HOST -port:$PORT -file:$TMP/cert -key:$TMP/key -noexit > /dev/null || true"
)
# Replace various placeholders in the commands with the actual values.
# Also generate an extension to append to the file name.
function transmogrify {
CMD=$1
local cc=$2
local pacing=$3
if [ "$cc" != "" ]; then
CMD=${CMD//_cc/--cc $cc}
EXT="-$cc"
fi
if [ "$pacing" == "on" ]; then
CMD=${CMD//_pacing/}
EXT="$EXT-pacing"
else
CMD=${CMD//_pacing/--no-pacing}
EXT="$EXT-nopacing"
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
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
rm -r "$TMP"
# Re-enable turboboost, hyperthreading and use powersave governor.
- name: Restore machine
run: sudo /root/bin/unprep.sh
if: success() || failure() || cancelled()

- name: Convert for profiler.firefox.com
- name: Post-process perf data
run: |
perf script -i perf.data -F +pid > transfer.perf &
perf script -i client/perf.data -F +pid > client.perf &
perf script -i server/perf.data -F +pid > server.perf &
for f in *.perf; do
# Convert for profiler.firefox.com
perf script -i "$f" -F +pid > "$f.fx" &
# Generate perf reports
perf report -i "$f" --no-children --stdio > "$f.txt" &
# Generate flamegraphs
flamegraph --perfdata "$f" --palette rust -o "${f//.perf/.svg}" &
done
wait
mv flamegraph.svg transfer.svg
mv client/flamegraph.svg client.svg
mv server/flamegraph.svg server.svg
rm neqo.svg
- name: Generate perf reports
run: |
perf report -i perf.data --no-children --stdio > transfer.perf.txt &
perf report -i client/perf.data --no-children --stdio > client.perf.txt &
perf report -i server/perf.data --no-children --stdio > server.perf.txt &
wait
- name: Format results as Markdown
id: results
run: |
Expand All @@ -132,6 +198,11 @@ jobs:
-e 's/^([a-z0-9].*)$/* **\1**/gi' \
-e 's/(change:[^%]*% )([^%]*%)(.*)/\1**\2**\3/gi' \
>> results.md
{
echo "### Client/server transfer results"
cat comparison.md
} >> results.md
cat results.md > "$GITHUB_STEP_SUMMARY"
- name: Remember main-branch push URL
if: github.ref == 'refs/heads/main'
Expand All @@ -158,6 +229,7 @@ jobs:
path: |
*.svg
*.perf
*.perf.fx
*.txt
results.*
target/criterion*
Expand Down
2 changes: 1 addition & 1 deletion neqo-bin/src/bin/server/old_https.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,6 @@ impl HttpServer for Http09Server {
None => break,
Some(e) => e,
};
qdebug!("Event {event:?}");
match event {
ConnectionEvent::NewStream { stream_id } => {
self.write_state
Expand All @@ -221,6 +220,7 @@ impl HttpServer for Http09Server {
.unwrap();
}
ConnectionEvent::StateChange(_)
| ConnectionEvent::SendStreamCreatable { .. }
| ConnectionEvent::SendStreamComplete { .. } => (),
e => qwarn!("unhandled event {e:?}"),
}
Expand Down

0 comments on commit 76630a5

Please sign in to comment.