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

Performance regression in Rust 1.71 #33

Closed
dantheman3333 opened this issue Sep 17, 2023 · 2 comments · Fixed by #36
Closed

Performance regression in Rust 1.71 #33

dantheman3333 opened this issue Sep 17, 2023 · 2 comments · Fixed by #36

Comments

@dantheman3333
Copy link
Owner

dantheman3333 commented Sep 17, 2023

Looks like a performance regression introduced by the compiler in x86 linux:

1.70:

frost$ hyperfine -w 5 "./target/release/frost info frost/tests/fixtures/test_large.bag"
Benchmark 1: ./target/release/frost info frost/tests/fixtures/test_large.bag
  Time (mean ± σ):     580.8 ms ±   0.9 ms    [User: 311.6 ms, System: 269.4 ms]
  Range (min … max):   579.7 ms … 582.2 ms    10 runs

1.71

frost$ hyperfine -w 5 "./target/release/frost info frost/tests/fixtures/test_large.bag"
Benchmark 1: ./target/release/frost info frost/tests/fixtures/test_large.bag
  Time (mean ± σ):     708.8 ms ±   4.9 ms    [User: 447.1 ms, System: 261.6 ms]
  Range (min … max):   703.3 ms … 716.7 ms    10 runs

1.72:

frost$ hyperfine -w 5 "./target/release/frost info frost/tests/fixtures/test_large.bag"
Benchmark 1: ./target/release/frost info frost/tests/fixtures/test_large.bag
  Time (mean ± σ):     716.9 ms ±   5.2 ms    [User: 452.5 ms, System: 263.8 ms]
  Range (min … max):   711.8 ms … 730.0 ms    10 runs
hyperfine -w 5 \
"./target/release/frost_1_69 info frost/tests/fixtures/test_large.bag" \
"./target/release/frost_1_70 info frost/tests/fixtures/test_large.bag" \
"./target/release/frost_1_71 info frost/tests/fixtures/test_large.bag" \
"./target/release/frost_1_72 info frost/tests/fixtures/test_large.bag" \
"./target/release/frost_1_73b info frost/tests/fixtures/test_large.bag" \
"./target/release/frost_1_74n info frost/tests/fixtures/test_large.bag"


Summary
  './target/release/frost_1_69 info frost/tests/fixtures/test_large.bag' ran
    1.00 ± 0.00 times faster than './target/release/frost_1_70 info frost/tests/fixtures/test_large.bag'
    1.20 ± 0.01 times faster than './target/release/frost_1_71 info frost/tests/fixtures/test_large.bag'
    1.22 ± 0.01 times faster than './target/release/frost_1_72 info frost/tests/fixtures/test_large.bag'
    1.51 ± 0.01 times faster than './target/release/frost_1_73b info frost/tests/fixtures/test_large.bag'
    1.54 ± 0.10 times faster than './target/release/frost_1_74n info frost/tests/fixtures/test_large.bag'

Test bag can be made by uncommenting https://github.com/dantheman3333/frost/blob/master/frost/tests/scripts/generate.sh#L22

@dantheman3333
Copy link
Owner Author

dantheman3333 commented Sep 17, 2023

bisection is crude, but ran 3x produces the same commit:

./bench.sh:

#!/bin/env bash

cargo build --release 
hyperfine -w 2 "$CARGO_TARGET_DIR/$CARGO_BUILD_TARGET/release/frost info frost/tests/fixtures/test_large.bag" --export-json /tmp/hyper

MEAN=$(cat /tmp/hyper | jq ".results[0].mean")

if (( $(echo "$MEAN > 0.61" |bc -l) )); then
    exit 1
fi

exit 0
********************************************************************************
Regression in 915aa06700af4a2363639bae70201cd7387470ad
********************************************************************************

Attempting to search unrolled perf builds
ERROR: couldn't find perf build comment
==================================================================================
= Please file this regression report on the rust-lang/rust GitHub repository     =
=        New issue: https://github.com/rust-lang/rust/issues/new                 =
=     Known issues: https://github.com/rust-lang/rust/issues                     =
= Copy and paste the text below into the issue report thread.  Thanks!           =
==================================================================================

searched nightlies: from nightly-2023-01-01 to nightly-2023-09-16
regressed nightly: nightly-2023-04-24
searched commit range: https://github.com/rust-lang/rust/compare/b628260df0587ae559253d8640ecb8738d3de613...7f94b314cead7059a71a265a8b64905ef2511796
regressed commit: https://github.com/rust-lang/rust/commit/915aa06700af4a2363639bae70201cd7387470ad

<details>
<summary>bisected with <a href='https://github.com/rust-lang/cargo-bisect-rustc'>cargo-bisect-rustc</a> v0.6.7</summary>


Host triple: x86_64-unknown-linux-gnu
Reproduce with:
```bash
cargo bisect-rustc --script=./bench.sh --start=2023-01-01 --end=2023-09-16 --preserve 
```

rust-lang/rust#110705 says performance tradeoffs were already weighed

@dantheman3333
Copy link
Owner Author

dantheman3333 commented Sep 17, 2023

1.70 - 1.71
inferno-diff-folded stacks_1_70.folded stacks_1_71.folded | inferno-flamegraph --negate > base_feature.svg
base_feature

1.71 - 1.70
inferno-diff-folded stacks_1_71.folded stacks_1_70.folded | inferno-flamegraph > feature_base.svg
feature_base

regression looks to be in the flatmap's next.

also interesting that the Bag::from_reader call looks to be optimized out in 1.70, but not important because if we force inline always on it, no optimizations occur, performance is still just as bad

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant