Skip to content

Commit

Permalink
fix: Disable generation of criterion HTML graphs (#1728)
Browse files Browse the repository at this point in the history
* fix: Disable generation of criterion HTML graphs

Because we pin the benchmark runs to single cores, and criterion hence
runs the report generation also on those cores, and based on `top`
output it appears as this is parallelized and hence may interfere
with the benchmark runs.

* Try `--noplot` (and patch things so that actually works)

* Missed one

* Don't post comment, since this is being refactored

* Fix merge

---------

Signed-off-by: Lars Eggert <[email protected]>
  • Loading branch information
larseggert authored Mar 12, 2024
1 parent e76645f commit d48fbed
Show file tree
Hide file tree
Showing 8 changed files with 32 additions and 2 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/bench.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ jobs:
- name: Run cargo bench
run: |
taskset -c 0 nice -n -20 \
cargo "+$TOOLCHAIN" bench --features bench | tee results.txt
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.
Expand All @@ -73,7 +73,7 @@ jobs:
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"
--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
Expand Down
6 changes: 6 additions & 0 deletions neqo-bin/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@ license.workspace = true
[[bin]]
name = "neqo-client"
path = "src/bin/client.rs"
bench = false

[[bin]]
name = "neqo-server"
path = "src/bin/server/main.rs"
bench = false

[lints]
workspace = true
Expand All @@ -35,3 +37,7 @@ qlog = { version = "0.12", default-features = false }
regex = { version = "1.9", default-features = false, features = ["unicode-perl"] }
tokio = { version = "1", default-features = false, features = ["net", "time", "macros", "rt", "rt-multi-thread"] }
url = { version = "2.5", default-features = false }

[lib]
# See https://github.com/bheisler/criterion.rs/blob/master/book/src/faq.md#cargo-bench-gives-unrecognized-option-errors-for-valid-command-line-options
bench = false
4 changes: 4 additions & 0 deletions neqo-common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,7 @@ udp = ["dep:quinn-udp", "dep:tokio"]
[target."cfg(windows)".dependencies.winapi]
version = "0.3"
features = ["timeapi"]

[lib]
# See https://github.com/bheisler/criterion.rs/blob/master/book/src/faq.md#cargo-bench-gives-unrecognized-option-errors-for-valid-command-line-options
bench = false
4 changes: 4 additions & 0 deletions neqo-crypto/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,7 @@ test-fixture = { path = "../test-fixture" }
[features]
gecko = ["mozbuild"]
fuzzing = []

[lib]
# See https://github.com/bheisler/criterion.rs/blob/master/book/src/faq.md#cargo-bench-gives-unrecognized-option-errors-for-valid-command-line-options
bench = false
4 changes: 4 additions & 0 deletions neqo-http3/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,7 @@ test-fixture = { path = "../test-fixture" }

[features]
fuzzing = ["neqo-transport/fuzzing", "neqo-crypto/fuzzing"]

[lib]
# See https://github.com/bheisler/criterion.rs/blob/master/book/src/faq.md#cargo-bench-gives-unrecognized-option-errors-for-valid-command-line-options
bench = false
4 changes: 4 additions & 0 deletions neqo-qpack/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,7 @@ static_assertions = { version = "1.1", default-features = false }

[dev-dependencies]
test-fixture = { path = "../test-fixture" }

[lib]
# See https://github.com/bheisler/criterion.rs/blob/master/book/src/faq.md#cargo-bench-gives-unrecognized-option-errors-for-valid-command-line-options
bench = false
4 changes: 4 additions & 0 deletions neqo-transport/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ test-fixture = { path = "../test-fixture" }
bench = []
fuzzing = ["neqo-crypto/fuzzing"]

[lib]
# See https://github.com/bheisler/criterion.rs/blob/master/book/src/faq.md#cargo-bench-gives-unrecognized-option-errors-for-valid-command-line-options
bench = false

[[bench]]
name = "transfer"
harness = false
Expand Down
4 changes: 4 additions & 0 deletions test-fixture/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,7 @@ qlog = { version = "0.12", default-features = false }

[features]
bench = []

[lib]
# See https://github.com/bheisler/criterion.rs/blob/master/book/src/faq.md#cargo-bench-gives-unrecognized-option-errors-for-valid-command-line-options
bench = false

0 comments on commit d48fbed

Please sign in to comment.