-
Notifications
You must be signed in to change notification settings - Fork 153
perf triage for 2021-07-20. #930
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
# 2021-07-20 Triage Log | ||
|
||
A mixed week, with some moderate regressions and moderate improvements. (I am personally wondering whether style-servo-debug needs to have a larger individual threshold for signalling a regression.) There were some notable PR's that were specifically oriented around performance enhancements. | ||
|
||
Triage done by **@pnkfelix**. | ||
Revision range: [5aff6dd07a562a2cba3c57fc3460a72acb6bef46..5c0ca08c662399c1c864310d1a20867d3ab68027](https://perf.rust-lang.org/?start=5aff6dd07a562a2cba3c57fc3460a72acb6bef46&end=5c0ca08c662399c1c864310d1a20867d3ab68027&absolute=false&stat=instructions%3Au) | ||
|
||
3 Regressions, 3 Improvements, 3 Mixed; 1 of them in rollups | ||
|
||
#### Regressions | ||
|
||
Replace associated item bound vars with placeholders when projecting [#86993](https://github.com/rust-lang/rust/issues/86993) | ||
- Moderate regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=b1f8e27b74c541d3d555149c8efa4bfe9385cd56&end=27e42058811e448b1a7dd8630d86ab247fbfcb9b&stat=instructions:u) (up to 1.5% on `incr-patched: b9b3e592dd cherry picked` builds of `style-servo-debug`) | ||
- A perf run was [done on the PR originally](https://perf.rust-lang.org/compare.html?start=fdfe819580062a441024d713b49340cd3f7d7efc&end=7baa78ec683fd14db4d4c1869dbef5cbbc5b774d). The regression flagged here seems to be on a different set of benchmarks: `style-servo-debug` and `wf-projection-stress-65510-*`, neither of which was flagged as a regression in the original runs. | ||
- Results overall seem pretty mixed and hard to interpret. Left a [comment](https://github.com/rust-lang/rust/pull/86993#issuecomment-883624762) on the PR. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure why the triage report labeled this as a regression when the results seem so mixed... I will investigate this. |
||
|
||
Remove special case for `ExprKind::Paren` in `MutVisitor` [#87284](https://github.com/rust-lang/rust/issues/87284) | ||
- Moderate regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=014026d1a7ca991f82f12efa95ef4dffb29dc8af&end=6535449a002264ee08dec8e741f1aadd97428fae&stat=instructions:u) (up to 1.0% on `full` builds of `coercions-debug`) | ||
- This is on the borderline for "significant regression", and was the *only* regression associated with this PR. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Eventually I'd like to make the logic for labeling runs as regressions to be a bit smarter in these cases. We should be able to encode that if there is only a single noisy benchmark which regressed and that regression is barely significant, we shouldn't count it as a regression. |
||
- Also, `coercions-debug` seems like it has a noisy history (that's pnkfelix's opinion from eyeballing it; it has also a single "?" on its comparison line.) | ||
|
||
Better diagnostics with mismatched types due to implicit static lifetime [#87244](https://github.com/rust-lang/rust/issues/87244) | ||
- Moderate regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=718d53b0cb7dde93499cb92950d60b412f5a3d05&end=da7d405357600a76f2b93b8aa41fe5ee5da7885d&stat=instructions:u) (up to 1.2% on `full` builds of `stm32f4-check`) | ||
- Widespread regressions that exceed the 0.2% threshold for non-noisy benchmarks. | ||
- Left a [comment](https://github.com/rust-lang/rust/pull/87244#issuecomment-883635813) asking if this was expected. But I suspect the situation does not warrant additional investment of effort, assuming there's no trivial fix identifiable from the PR author. | ||
|
||
#### Improvements | ||
|
||
Rollup of 6 pull requests [#87118](https://github.com/rust-lang/rust/issues/87118) | ||
- Moderate improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=a08f25a7ef2800af5525762e981c24d96c14febe&end=ee5ed4a88d6a869cdb152829ed697d6459650db3&stat=instructions:u) (up to -1.9% on `full` builds of `deeply-nested-async-check`) | ||
|
||
|
||
Make expansions stable for incr. comp. [#86676](https://github.com/rust-lang/rust/issues/86676) | ||
- Moderate improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=c78ebb7bdcfc924a20fd069891ffe1364d6814e7&end=68511b574ffe019a5cb3e9fa92605f80d39167bc&stat=instructions:u) (up to -2.4% on `full` builds of `externs-debug`) | ||
|
||
|
||
Some perf optimizations and logging [#87203](https://github.com/rust-lang/rust/issues/87203) | ||
- Very large improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=68511b574ffe019a5cb3e9fa92605f80d39167bc&end=c7331d65bdbab1187f5a9b8f5b918248678ebdb9&stat=instructions:u) (up to -21.7% on `full` builds of `deeply-nested-async-check`) | ||
|
||
|
||
#### Mixed | ||
|
||
Cache expansion hash globally [#87044](https://github.com/rust-lang/rust/issues/87044) | ||
- Moderate regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=3e1c75c6e25a4db968066bd2ef2dabc7c504d7ca&end=c7d6bcc788ef6b2293d2d5166a9b0339d5d03b0a&stat=instructions:u) (up to 1.9% on `full` builds of `deeply-nested-async-check`) | ||
- Moderate improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=3e1c75c6e25a4db968066bd2ef2dabc7c504d7ca&end=c7d6bcc788ef6b2293d2d5166a9b0339d5d03b0a&stat=instructions:u) (up to -1.4% on `incr-unchanged` builds of `inflate-check`) | ||
- This was an improvement for a lot of incremental cases, and a regression for a few full-compilation cases: `deeply-nested-async-{check,debug,opt}`, and `coercions-debug`. | ||
- It seems like these results were [expected](https://github.com/rust-lang/rust/pull/87044#issuecomment-879407381), and they make sense given the nature of the PR. | ||
|
||
Update Rust Float-Parsing Algorithms to use the Eisel-Lemire algorithm. [#86761](https://github.com/rust-lang/rust/issues/86761) | ||
- Moderate regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=64d171b8a419eb6cb872ab579398eff8a741bbc6&end=f502bd3abd12111bbfae0974db018c165a977c0e&stat=instructions:u) (up to 3.2% on `incr-patched: b9b3e592dd cherry picked` builds of `style-servo-debug`) | ||
- Moderate improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=64d171b8a419eb6cb872ab579398eff8a741bbc6&end=f502bd3abd12111bbfae0974db018c165a977c0e&stat=instructions:u) (up to -1.4% on `full` builds of `piston-image-opt`) | ||
- A perf run was done on the orignal PR, but the run there did not predict the regression that was now observed to `style-servo-debug` when this PR landed on nightly. | ||
- Left a [comment](https://github.com/rust-lang/rust/pull/86761#issuecomment-883645181), but I am guessing that `style-servo-debug` may need a bigger noise threshold (or rather, sensitivity to perturbations) than what we currently use. | ||
|
||
Move OnDiskCache to rustc_query_impl. [#86698](https://github.com/rust-lang/rust/issues/86698) | ||
- Moderate improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=5a8a44196b3cf099f8c9b0156bd902eaec0b4e5f&end=18073052d8c3544ccb73effd289ed3acda0d66c0&stat=instructions:u) (up to -1.4% on `incr-unchanged` builds of `ctfe-stress-4-check`) | ||
- Moderate regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=5a8a44196b3cf099f8c9b0156bd902eaec0b4e5f&end=18073052d8c3544ccb73effd289ed3acda0d66c0&stat=instructions:u) (up to 1.1% on `incr-unchanged` builds of `deeply-nested-async-opt`) | ||
- A perf run was done on the original PR that predicted a much better outcome here. | ||
- Left a [comment](https://github.com/rust-lang/rust/pull/86698#issuecomment-883647259) asking PR author if they have any idea about the discrepancy. | ||
|
||
#### Untriaged Pull Requests | ||
|
||
- [#87153 \[debuginfo\] Emit associated type bindings in trait object type names.](https://github.com/rust-lang/rust/pull/87153) | ||
- [#86993 Replace associated item bound vars with placeholders when projecting](https://github.com/rust-lang/rust/pull/86993) | ||
- [#86777 Include terminators in instance size estimate](https://github.com/rust-lang/rust/pull/86777) | ||
- [#86588 Rollup of 8 pull requests](https://github.com/rust-lang/rust/pull/86588) | ||
- [#86034 Change entry point to 🛡️ against 💥 💥-payloads](https://github.com/rust-lang/rust/pull/86034) | ||
- [#84560 Inline Iterator as IntoIterator.](https://github.com/rust-lang/rust/pull/84560) | ||
|
||
#### Nags requiring follow up |
Uh oh!
There was an error while loading. Please reload this page.