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

[DO NOT MERGE] Perf run for zoxc's rustc-hash #96893

Closed
wants to merge 2 commits into from

Conversation

lqd
Copy link
Member

@lqd lqd commented May 10, 2022

This is another perf run to try and see the results of zoxc's version of the 64b fxhash, from their rustc-hash PR.

This was previously tried in #93651 on the older benchmarks, and we now want to see the results on the recently updated rustc-perf benchmarks.

r? @ghost

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 10, 2022
@lqd
Copy link
Member Author

lqd commented May 10, 2022

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 10, 2022
@bors
Copy link
Contributor

bors commented May 10, 2022

⌛ Trying commit ea59ed2 with merge 71812917089730aa5abc2f6c5978c6718c9b0f5f...

@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-llvm-12 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
######################################################################## 100.0%
extracting /checkout/obj/build/cache/2022-04-05/rustfmt-nightly-x86_64-unknown-linux-gnu.tar.xz
Building rustbuild
    Updating crates.io index
    Updating git repository `https://github.com/Zoxc/rustc-hash/`
---
   Compiling tinystr v0.3.4
   Compiling remove_dir_all v0.5.3
   Compiling rustc_graphviz v0.0.0 (/checkout/compiler/rustc_graphviz)
   Compiling cpufeatures v0.2.1
   Compiling rustc-hash v1.0.1 (https://github.com/Zoxc/rustc-hash/?branch=new-hash#a5a1e17e)
   Compiling unic-common v0.9.0
   Compiling unic-char-range v0.9.0
   Compiling scoped-tls v1.0.0
   Compiling self_cell v0.10.2
---
   Compiling tinystr v0.3.4
   Compiling remove_dir_all v0.5.3
   Compiling arrayvec v0.7.0
   Compiling rustc_graphviz v0.0.0 (/checkout/compiler/rustc_graphviz)
   Compiling rustc-hash v1.0.1 (https://github.com/Zoxc/rustc-hash/?branch=new-hash#a5a1e17e)
   Compiling unicode-width v0.1.8
   Compiling unic-char-range v0.9.0
   Compiling scoped-tls v1.0.0
   Compiling unic-common v0.9.0
---

---- [ui] src/test/ui/generic-associated-types/self-outlives-lint.rs stdout ----
diff of stderr:

108    = note: this bound is currently required to ensure that impls have maximum flexibility
109    = note: we are soliciting feedback, see issue #87479 <https://github.com/rust-lang/rust/issues/87479> for more information
- error: missing required bound on `Item`
-   --> $DIR/self-outlives-lint.rs:142:5
-    |
-    |
- LL |     type Item<'a>;
-    |                  |
-    |                  |
-    |                  help: add the required where clause: `where Self: 'a`
-    |
-    = note: this bound is currently required to ensure that impls have maximum flexibility
-    = note: we are soliciting feedback, see issue #87479 <https://github.com/rust-lang/rust/issues/87479> for more information
122 error: missing required bound on `Iterator`
123   --> $DIR/self-outlives-lint.rs:144:5
Some tests failed in compiletest suite=ui mode=ui host=x86_64-unknown-linux-gnu target=x86_64-unknown-linux-gnu
124    |
124    |

126    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
127    |                                                       |
128    |                                                       help: add the required where clause: `where Self: 'a`
+    |
+    = note: this bound is currently required to ensure that impls have maximum flexibility
+    = note: we are soliciting feedback, see issue #87479 <https://github.com/rust-lang/rust/issues/87479> for more information
+ error: missing required bound on `Item`
+   --> $DIR/self-outlives-lint.rs:142:5
+    |
+    |
+ LL |     type Item<'a>;
+    |                  |
+    |                  |
+    |                  help: add the required where clause: `where Self: 'a`
129    |
130    = note: this bound is currently required to ensure that impls have maximum flexibility
131    = note: we are soliciting feedback, see issue #87479 <https://github.com/rust-lang/rust/issues/87479> for more information

The actual stderr differed from the expected stderr.
Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/generic-associated-types/self-outlives-lint/self-outlives-lint.stderr
To update references, rerun the tests and pass the `--bless` flag
To update references, rerun the tests and pass the `--bless` flag
To only update this specific test, also pass `--test-args generic-associated-types/self-outlives-lint.rs`

error: 1 errors occurred comparing output.
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/generic-associated-types/self-outlives-lint.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/generic-associated-types/self-outlives-lint" "-A" "unused" "-Crpath" "-O" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/generic-associated-types/self-outlives-lint/auxiliary"
stdout: none
--- stderr -------------------------------
error: missing required bound on `Item`
   |
   |
LL |     type Item<'x>;
   |                  |
   |                  |
   |                  help: add the required where clause: `where Self: 'x`
   |
   = note: this bound is currently required to ensure that impls have maximum flexibility
   = note: we are soliciting feedback, see issue #87479 <https://github.com/rust-lang/rust/issues/87479> for more information
error: missing required bound on `Out`
  --> /checkout/src/test/ui/generic-associated-types/self-outlives-lint.rs:25:5
   |
   |
LL |     type Out<'x>;
   |                 |
   |                 |
   |                 help: add the required where clause: `where T: 'x`
   |
   = note: this bound is currently required to ensure that impls have maximum flexibility
   = note: we are soliciting feedback, see issue #87479 <https://github.com/rust-lang/rust/issues/87479> for more information
error: missing required bound on `Out`
  --> /checkout/src/test/ui/generic-associated-types/self-outlives-lint.rs:39:5
   |
   |
LL |     type Out<'x>;
   |                 |
   |                 |
   |                 help: add the required where clause: `where T: 'x`
   |
   = note: this bound is currently required to ensure that impls have maximum flexibility
   = note: we are soliciting feedback, see issue #87479 <https://github.com/rust-lang/rust/issues/87479> for more information
error: missing required bounds on `Out`
  --> /checkout/src/test/ui/generic-associated-types/self-outlives-lint.rs:46:5
   |
   |
LL |     type Out<'x, 'y>;
   |                     |
   |                     |
   |                     help: add the required where clauses: `where T: 'x, U: 'y`
   |
   = note: these bounds are currently required to ensure that impls have maximum flexibility
   = note: we are soliciting feedback, see issue #87479 <https://github.com/rust-lang/rust/issues/87479> for more information
error: missing required bound on `Out`
  --> /checkout/src/test/ui/generic-associated-types/self-outlives-lint.rs:61:5
   |
   |
LL |     type Out<'x, D>;
   |                    |
   |                    |
   |                    help: add the required where clause: `where D: 'x`
   |
   = note: this bound is currently required to ensure that impls have maximum flexibility
   = note: we are soliciting feedback, see issue #87479 <https://github.com/rust-lang/rust/issues/87479> for more information
error: missing required bound on `Out`
  --> /checkout/src/test/ui/generic-associated-types/self-outlives-lint.rs:77:5
   |
   |
LL |     type Out<'x, D>;
   |                    |
   |                    |
   |                    help: add the required where clause: `where D: 'x`
   |
   = note: this bound is currently required to ensure that impls have maximum flexibility
   = note: we are soliciting feedback, see issue #87479 <https://github.com/rust-lang/rust/issues/87479> for more information
error: missing required bound on `Out`
  --> /checkout/src/test/ui/generic-associated-types/self-outlives-lint.rs:92:5
   |
   |
LL |     type Out<'x, D>;
   |                    |
   |                    |
   |                    help: add the required where clause: `where D: 'x`
   |
   = note: this bound is currently required to ensure that impls have maximum flexibility
   = note: we are soliciting feedback, see issue #87479 <https://github.com/rust-lang/rust/issues/87479> for more information
error: missing required bounds on `Bar`
  --> /checkout/src/test/ui/generic-associated-types/self-outlives-lint.rs:114:5
   |
   |
LL |     type Bar<'b>;
   |                 |
   |                 |
   |                 help: add the required where clauses: `where Self: 'a, Self: 'b`
   |
   = note: these bounds are currently required to ensure that impls have maximum flexibility
   = note: we are soliciting feedback, see issue #87479 <https://github.com/rust-lang/rust/issues/87479> for more information
error: missing required bound on `Bar`
  --> /checkout/src/test/ui/generic-associated-types/self-outlives-lint.rs:122:5
   |
   |
LL |     type Bar<'b>;
   |                 |
   |                 |
   |                 help: add the required where clause: `where Self: 'b`
   |
   = note: this bound is currently required to ensure that impls have maximum flexibility
   = note: we are soliciting feedback, see issue #87479 <https://github.com/rust-lang/rust/issues/87479> for more information
error: missing required bound on `Bar`
  --> /checkout/src/test/ui/generic-associated-types/self-outlives-lint.rs:129:5
   |
   |
LL |     type Bar<'b>;
   |                 |
   |                 |
   |                 help: add the required where clause: `where Self: 'b`
   |
   = note: this bound is currently required to ensure that impls have maximum flexibility
   = note: we are soliciting feedback, see issue #87479 <https://github.com/rust-lang/rust/issues/87479> for more information
error: missing required bound on `Iterator`
  --> /checkout/src/test/ui/generic-associated-types/self-outlives-lint.rs:144:5
   |
   |
LL |     type Iterator<'a>: Iterator<Item = Self::Item<'a>>;
   |                                                       |
   |                                                       |
   |                                                       help: add the required where clause: `where Self: 'a`
   |
   = note: this bound is currently required to ensure that impls have maximum flexibility
   = note: we are soliciting feedback, see issue #87479 <https://github.com/rust-lang/rust/issues/87479> for more information
error: missing required bound on `Item`
  --> /checkout/src/test/ui/generic-associated-types/self-outlives-lint.rs:142:5
   |
   |
LL |     type Item<'a>;
   |                  |
   |                  |
   |                  help: add the required where clause: `where Self: 'a`
   |
   = note: this bound is currently required to ensure that impls have maximum flexibility
   = note: we are soliciting feedback, see issue #87479 <https://github.com/rust-lang/rust/issues/87479> for more information
error: missing required bound on `Item`
  --> /checkout/src/test/ui/generic-associated-types/self-outlives-lint.rs:150:5
   |
   |
LL |     type Item<'a>;
   |                  |
   |                  |
   |                  help: add the required where clause: `where Self: 'a`
   |
   = note: this bound is currently required to ensure that impls have maximum flexibility
   = note: we are soliciting feedback, see issue #87479 <https://github.com/rust-lang/rust/issues/87479> for more information
error: missing required bound on `Bar`
  --> /checkout/src/test/ui/generic-associated-types/self-outlives-lint.rs:159:5
   |
   |
LL |     type Bar<'a, 'b>;
   |                     |
   |                     |
   |                     help: add the required where clause: `where 'b: 'a`
   |
   = note: this bound is currently required to ensure that impls have maximum flexibility
   = note: we are soliciting feedback, see issue #87479 <https://github.com/rust-lang/rust/issues/87479> for more information
error: missing required bound on `Fut`
  --> /checkout/src/test/ui/generic-associated-types/self-outlives-lint.rs:175:5
   |
   |
LL |     type Fut<'out>;
   |                   |
   |                   |
   |                   help: add the required where clause: `where 'ctx: 'out`
   |
   = note: this bound is currently required to ensure that impls have maximum flexibility
   = note: we are soliciting feedback, see issue #87479 <https://github.com/rust-lang/rust/issues/87479> for more information
error: aborting due to 15 previous errors
------------------------------------------


@bors
Copy link
Contributor

bors commented May 10, 2022

☀️ Try build successful - checks-actions
Build commit: 71812917089730aa5abc2f6c5978c6718c9b0f5f (71812917089730aa5abc2f6c5978c6718c9b0f5f)

@rust-timer
Copy link
Collaborator

Queued 71812917089730aa5abc2f6c5978c6718c9b0f5f with parent d53f1e8, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (71812917089730aa5abc2f6c5978c6718c9b0f5f): comparison url.

Summary:

  • Primary benchmarks: mixed results
  • Secondary benchmarks: mixed results
Regressions 😿
(primary)
Regressions 😿
(secondary)
Improvements 🎉
(primary)
Improvements 🎉
(secondary)
All 😿 🎉
(primary)
count1 1 10 1 8 2
mean2 0.2% 0.3% -0.3% -0.3% -0.1%
max 0.2% 0.4% -0.3% -0.6% -0.3%

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf +perf-regression

Footnotes

  1. number of relevant changes

  2. the arithmetic mean of the percent change

@rustbot rustbot added perf-regression Performance regression. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels May 10, 2022
@nnethercote
Copy link
Contributor

As before: instruction counts are barely changed, but cycles and wall times look like a clear improvement.

@nnethercote
Copy link
Contributor

I did a local run on my Intel Linux box and results were underwhelming. Instruction counts barely changed. Cycles for primary benchmarks, check full runs:

Benchmark Profile Scenario % Change Significance Factor?
libc-0.2.124 check full -1.68% 8.38x
cargo-0.60.0 check full 1.33% 6.63x
unicode-normalization-0.1.19 check full -1.32% 6.61x
stm32f4-0.14.0 check full -1.23% 6.13x
clap-3.1.6 check full 0.91% 4.54x
html5ever-0.26.0 check full 0.56% 2.82x
syn-1.0.89 check full 0.50% 2.49x
diesel-1.4.8 check full -0.48% 2.40x
regex-1.5.5 check full -0.43% 2.15x
cranelift-codegen-0.82.1 check full -0.43% 2.13x
hyper-0.14.18 check full 0.34% 1.71x
bitmaps-2 check full -0.33% 1.63x
serde_derive-1.0.136 check full 0.32% 1.59x
ripgrep-13.0.0 check full -0.31% 1.57x

and secondary (many of these are non-rustc-perf benchmarks that I have downloaded):

Benchmark Profile Scenario % Change Significance Factor?
unused-warnings check full -8.00% 40.00x
wf-projection-stress-65510 check full 4.83% 24.15x
deep-vector check full -4.59% 22.96x
ucd check full -3.82% 19.08x
coercions check full -3.57% 17.87x
issue-58319 check full 3.33% 16.63x
await-call-tree check full -3.26% 16.28x
tuple-stress check full -2.96% 14.80x
match-stress check full -2.73% 13.64x
yansi-0.5.0 check full 2.64% 13.21x
many-assoc-items check full -2.27% 11.34x
inotify-0.10.0 check full 2.22% 11.08x
memchr-2.4.1 check full 2.17% 10.87x
ctor-0.1.21 check full 1.94% 9.71x
tokio-1.17.0 check full -1.79% 8.93x
externs check full 1.78% 8.91x
issue-46449 check full -1.46% 7.32x
log-0.4.16 check full 1.38% 6.91x
deeply-nested-multi check full 1.37% 6.87x
enum-as-inner-0.4.0 check full -1.31% 6.57x
regression-31157 check full -1.28% 6.39x
enumflags2_derive-0.7.4 check full -1.27% 6.34x
issue-88862 check full -1.22% 6.12x
num-derive-0.3.3 check full -1.14% 5.70x
wayland-scanner-0.29.4 check full -1.07% 5.33x
unicode-xid-0.2.3 check full -0.96% 4.79x
diesel_derives-1.4.1 check full -0.78% 3.90x
rustfix-0.6.0 check full 0.76% 3.80x
token-stream-stress check full 0.75% 3.76x
derive check full -0.72% 3.62x
proc-macro2-1.0.38 check full -0.66% 3.30x
ostn15_phf check full -0.66% 3.28x
stdweb-derive-0.5.3 check full 0.59% 2.94x
web-sys-0.3.56 check full 0.58% 2.88x
tonic-build-0.7.0 check full 0.55% 2.76x
unify-linearly check full -0.53% 2.63x
clap_derive-3.1.7 check full -0.49% 2.45x
wasm-bindgen-backend-0.2.79 check full 0.46% 2.30x
futures-lite-1.12.0 check full -0.44% 2.22x
futures-macro-0.3.19 check full 0.43% 2.15x
pest_generator-2.1.3 check full 0.40% 2.02x
wg-grammar check full 0.40% 2.00x
projection-caching check full 0.38% 1.90x
time-macros-0.2.3 check full -0.33% 1.66x
vsdb_derive-0.21.1 check full 0.33% 1.66x
scroll_derive-0.11.0 check full -0.33% 1.64x
async-std-1.10.0 check full 0.33% 1.63x
wast-39.0.0 check full -0.32% 1.60x
nix-0.23.1 check full 0.30% 1.52x

Walltimes are generally too noisy on local runs to say much.

@lqd
Copy link
Member Author

lqd commented May 19, 2022

As discussed, this looked like a clearer win on AMD. It's unclear on Intel, so it may not be worth it to bother anymore with this PR exactly as-is. It was on our performance roadmap, we tried it, and it's not a definite win. Closing the PR.

@lqd lqd closed this May 19, 2022
@lqd lqd deleted the zoxc-hash2 branch May 19, 2022 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
perf-regression Performance regression. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants