Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #1110 from pnkfelix/triage-2021-11-23
performance triage for this week.
- Loading branch information
Showing
1 changed file
with
124 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| # 2021-11-23 Triage Log | ||
|
|
||
| This week, there were a number of cases where the `incr-unchanged` variants of `inflate` went up or down by 5% to 6%; we believe these are | ||
| instances of increased noise in benchmarks documented on [rustc-perf#1105](https://github.com/rust-lang/rustc-perf/issues/1105). I was tempted to remove these from the report, but its non-trivial to re-construct the report "as if" some benchmark were omitted. | ||
|
|
||
| Otherwise, there were some nice wins for performance. For example, PR [#90996](https://github.com/rust-lang/rust/issues/90996) more than halved the compilation time for `full` builds of `diesel` by revising how we hash `ObligationCauseData`. If anyone is interested, it might be good to follow-up on the effects of PR [#90352](https://github.com/rust-lang/rust/issues/90352), "Simplify `for` loop desugar", where we have hypothesized that the increased compilation time is due to more LLVM optimizations being applied. | ||
|
|
||
| Triage done by **@pnkfelix**. | ||
| Revision range: [934624fe..22c2d9dd](https://perf.rust-lang.org/?start=934624fe5f66ce3fb8abf0597a6deb079783335f&end=22c2d9ddbf356bcdb718e88ca6ee3665e1e42690&absolute=false&stat=instructions%3Au) | ||
|
|
||
| 1 Regressions, 3 Improvements, 8 Mixed; 3 of them in rollups | ||
| 34 comparisons made in total | ||
|
|
||
| #### Regressions | ||
|
|
||
| rustdoc: Make two small cleanups [#91073](https://github.com/rust-lang/rust/issues/91073) | ||
| - Very large regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=5bc98076f37dd8c1476de4bbe0515c55a65332b7&end=02913c078849f940371eb9930754f2b0f1bc9fad&stat=instructions:u) (up to 5.3% on `incr-unchanged` builds of `inflate`) | ||
| - This was [previously triaged](https://github.com/rust-lang/rust/pull/91073#issuecomment-974880869) as spurious by the PR author. | ||
| - (*Only* inflate debug incr-unchanged was affected significantly.) | ||
|
|
||
| #### Improvements | ||
|
|
||
| Rollup of 7 pull requests [#90966](https://github.com/rust-lang/rust/issues/90966) | ||
| - Very large improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=d914f17ca71a33a89b2dc3436fca51b1a091559e&end=41301c3b2371365b753c2ad6a74528a38f3815ce&stat=instructions:u) (up to -5.9% on `incr-unchanged` builds of `inflate`) | ||
| - (This is bouncing around in the same manner that the above regression bounced around; i.e., this is a spurious *improvement*.) | ||
|
|
||
| Optimize `impl Hash for ObligationCauseData` by not hashing `ObligationCauseCode` variant fields [#90996](https://github.com/rust-lang/rust/issues/90996) | ||
| - Very large improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=e8423e6c449ad3f4b0dab442175462004554e499&end=18fa4342fc215fe9e714307db694eaa8f5dc4a0d&stat=instructions:u) (up to -58.1% on `full` builds of `diesel`) | ||
|
|
||
| Avoid documenting top-level private imports [#91094](https://github.com/rust-lang/rust/issues/91094) | ||
| - Very large improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=8d0c79d26995a973c6f33c32ffb0c827d78bb213&end=2e055d92e0d527b273d12584bd842f6527e7652c&stat=instructions:u) (up to -45.1% on `full` builds of `webrender-wrench`) | ||
| - This appears to counter-act a regression that was introduced in PR [#90769](https://github.com/rust-lang/rust/pull/90769) a rollup where the root cause is believed to be PR [#88447](https://github.com/rust-lang/rust/pull/88447), "Use computed visibility in rustdoc". | ||
|
|
||
| rustdoc: Cleanup `DocFragment` [#91034](https://github.com/rust-lang/rust/issues/91034) | ||
| - Moderate improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=18fa4342fc215fe9e714307db694eaa8f5dc4a0d&end=a77da2d454e6caa227a85b16410b95f93495e7e0&stat=instructions:u) (up to -0.7% on `full` builds of `serde`) | ||
| - It has been [pointed out](https://github.com/rust-lang/rust/pull/91034#issuecomment-974523369) that this was actually a nice -0.8% to -1.4% win on max-rss as well. | ||
| - (This is solely for rustdoc benchmarking, to be clear.) | ||
|
|
||
| libcore: assume the input of `next_code_point` and `next_code_point_reverse` is UTF-8-like [#89611](https://github.com/rust-lang/rust/issues/89611) | ||
| - Small improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=49d42325d0888a27ed50bf918d378fbf7f41a348&end=65f3f8b220f020e562c5dd848ff7319257a7ba45&stat=instructions:u) (up to -0.8% on `full` builds of `encoding`) | ||
|
|
||
|
|
||
| #### Mixed | ||
|
|
||
| Remove `DropArena`. [#90919](https://github.com/rust-lang/rust/issues/90919) | ||
| - Small improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=934624fe5f66ce3fb8abf0597a6deb079783335f&end=d914f17ca71a33a89b2dc3436fca51b1a091559e&stat=instructions:u) (up to -0.3% on `incr-patched: println` builds of `clap-rs`) | ||
| - Very large regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=934624fe5f66ce3fb8abf0597a6deb079783335f&end=d914f17ca71a33a89b2dc3436fca51b1a091559e&stat=instructions:u) (up to 6.1% on `incr-unchanged` builds of `inflate`) | ||
| - This was triaged as noise in the PR comments both [before it landed](https://github.com/rust-lang/rust/pull/90919/#issuecomment-968772257) and [after it landed](https://github.com/rust-lang/rust/pull/90919/#issuecomment-970656929). | ||
|
|
||
| std: Get the standard library compiling for wasm64 [#90382](https://github.com/rust-lang/rust/issues/90382) | ||
| - Large improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=6414e0b5b308d3ae27da83c6a25098cc8aadc1a9&end=b6f580acc0ce233d5c4d1f9680d354fded88b824&stat=instructions:u) (up to -4.3% on `incr-unchanged` builds of `clap-rs`) | ||
| - Moderate regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=6414e0b5b308d3ae27da83c6a25098cc8aadc1a9&end=b6f580acc0ce233d5c4d1f9680d354fded88b824&stat=instructions:u) (up to 1.0% on `incr-unchanged` builds of `issue-88862`) | ||
| - There were a lot of various changes in this PR, such as updates to dependencies (`compiler_builtins` and `dlmalloc`). We probably shoud have done a pre-merge rust-timer run on this PR. | ||
| - The flagged regressions of magnitude greater than 0.5% are isolated to "issue-88862 check" and "style-servo check". The improvements tended to outweigh the regressions. For the most part almost all significant performance effects are isolated to check builds. | ||
|
|
||
| Rollup of 8 pull requests [#91019](https://github.com/rust-lang/rust/issues/91019) | ||
| - Moderate improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=b6f580acc0ce233d5c4d1f9680d354fded88b824&end=cc946fcd326f7d85d4af096efdc73538622568e9&stat=instructions:u) (up to -0.6% on `incr-unchanged` builds of `style-servo`) | ||
| - Moderate regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=b6f580acc0ce233d5c4d1f9680d354fded88b824&end=cc946fcd326f7d85d4af096efdc73538622568e9&stat=instructions:u) (up to 0.8% on `full` builds of `await-call-tree`) | ||
| - The regression is entirely associated with doc builds, which led to the PR author to flag PR [#90750](https://github.com/rust-lang/rust/pull/90750) as the root cause. | ||
| - It seems to me like the extra work injected by PR [#90750](https://github.com/rust-lang/rust/pull/90750) may be unavoidable; but was it expected to be significant? | ||
|
|
||
| Implement `clone_from` for `State` [#90535](https://github.com/rust-lang/rust/issues/90535) | ||
| - Small improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=3b65165ab79f535792fc7c504b3fbadf29d7a877&end=50f2c292007f9364908e4b8344886797f0144648&stat=instructions:u) (up to -0.2% on `full` builds of `many-assoc-items`) | ||
| - Very large regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=3b65165ab79f535792fc7c504b3fbadf29d7a877&end=50f2c292007f9364908e4b8344886797f0144648&stat=instructions:u) (up to 6.1% on `incr-unchanged` builds of `inflate`) | ||
| - This was evaluated for its effect on performance [prior to merge](https://github.com/rust-lang/rust/pull/90535#issuecomment-973298904); that run returned no relevant changes. | ||
| - As noted elsewhere, for this report we should probably treat "inflate" as noisy. | ||
|
|
||
| Update stdarch [#91052](https://github.com/rust-lang/rust/issues/91052) | ||
| - Very large improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=50f2c292007f9364908e4b8344886797f0144648&end=6d48ee90f51dd5793b425c6593581fd108ead398&stat=instructions:u) (up to -5.8% on `incr-unchanged` builds of `inflate`) | ||
| - Small regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=50f2c292007f9364908e4b8344886797f0144648&end=6d48ee90f51dd5793b425c6593581fd108ead398&stat=instructions:u) (up to 0.3% on `incr-patched: println` builds of `html5ever`) | ||
| - The only benchmark that regressed is "html5ever debug" ("incr-patched: println" and "incr-unchanged"), and only by a relatively small amount. This seems acceptable to me, compared to the effort involved in figuring out how this change could be related to that effect. | ||
|
|
||
| Point at source of trait bound obligations in more places [#89580](https://github.com/rust-lang/rust/issues/89580) | ||
| - Large improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=02913c078849f940371eb9930754f2b0f1bc9fad&end=b8e5ab20ed7a7677a998a163ccf7853764b195e6&stat=instructions:u) (up to -5.0% on `incr-unchanged` builds of `inflate`) | ||
| - Small regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=02913c078849f940371eb9930754f2b0f1bc9fad&end=b8e5ab20ed7a7677a998a163ccf7853764b195e6&stat=instructions:u) (up to 1.1% on `incr-unchanged` builds of `wg-grammar`) | ||
| - (Again, we can probably ignore the change to `inflate`.) | ||
| - Other than that, there were a broad set of small regressions. Putting aside the ones tagged with `?` ("noisy"), there are 19 benchmarks that regressed by 0.10% to 0.42%. This seems like an acceptable cost. | ||
|
|
||
| Simplify `for` loop desugar [#90352](https://github.com/rust-lang/rust/issues/90352) | ||
| - Very large improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=65f3f8b220f020e562c5dd848ff7319257a7ba45&end=cebd2dda1d9071f2209079370c412f4ef9ef2b82&stat=instructions:u) (up to -6.2% on `incr-full` builds of `clap-rs`) | ||
| - Large regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=65f3f8b220f020e562c5dd848ff7319257a7ba45&end=cebd2dda1d9071f2209079370c412f4ef9ef2b82&stat=instructions:u) (up to 2.3% on `full` builds of `regex`) | ||
| - This was triaged in the PR comments both [before it landed](https://github.com/rust-lang/rust/pull/90352#issuecomment-972986518) and [after it landed](https://github.com/rust-lang/rust/pull/90352#issuecomment-975432954) with the justification "The regressions seem to all be in `-opt` builds and solely part of the time spent in LLVM, so I'm hoping it's that more optimizations apply now (and worst case some optimizations require more work but don't result in better code)." | ||
|
|
||
| Manually outline error on incremental_verify_ich [#89883](https://github.com/rust-lang/rust/issues/89883) | ||
| - Small improvement in [instruction counts](https://perf.rust-lang.org/compare.html?start=7c4be43b27993ab405beaa19738258fdd546d3db&end=22c2d9ddbf356bcdb718e88ca6ee3665e1e42690&stat=instructions:u) (up to -0.8% on `incr-unchanged` builds of `clap-rs`) | ||
| - Large regression in [instruction counts](https://perf.rust-lang.org/compare.html?start=7c4be43b27993ab405beaa19738258fdd546d3db&end=22c2d9ddbf356bcdb718e88ca6ee3665e1e42690&stat=instructions:u) (up to 1.1% on `incr-unchanged` builds of `coercions`) | ||
| - The [pre-merge rustc-timer run](https://github.com/rust-lang/rust/pull/89883#issuecomment-974927075) did not predict such a significant impact on `coercions`. | ||
| - The driving force for this change was to reduce the critical path in bootstrap time, so the most important thing to look at is the [bootstrap timing data](https://perf.rust-lang.org/compare.html?start=7c4be43b27993ab405beaa19738258fdd546d3db&end=22c2d9ddbf356bcdb718e88ca6ee3665e1e42690#bootstrap). Specifically: while there is a big mix of ups and downs on the percentages column, the crate that takes the longest to compile (`rustc_query_impl`, the laggard at over 80 seconds of compilation time). | ||
| - This PR brings the compilation time of `rustc_query_impl` from 87.1 seconds down to 85.6 seconds, a -1.8% improvement. | ||
| - That is consistent with the [predicted effect](https://github.com/rust-lang/rust/pull/89883#issue-758410955) of the PR, and justifies the isolated impact on instruction counts. | ||
|
|
||
|
|
||
| #### Untriaged Pull Requests | ||
|
|
||
| - [#91052 Update stdarch](https://github.com/rust-lang/rust/pull/91052) | ||
| - [#91019 Rollup of 8 pull requests](https://github.com/rust-lang/rust/pull/91019) | ||
| - [#90883 Rollup of 3 pull requests](https://github.com/rust-lang/rust/pull/90883) | ||
| - [#90839 Generate documentation in rustc `rustc_index::newtype_index` macro](https://github.com/rust-lang/rust/pull/90839) | ||
| - [#90821 MIRI says `reverse` is UB, so replace it with something LLVM can vectorize](https://github.com/rust-lang/rust/pull/90821) | ||
| - [#90769 Rollup of 5 pull requests](https://github.com/rust-lang/rust/pull/90769) | ||
| - [#90746 Optimize pattern matching](https://github.com/rust-lang/rust/pull/90746) | ||
| - [#90695 Rollup of 4 pull requests](https://github.com/rust-lang/rust/pull/90695) | ||
| - [#90684 Change paths for `dist` command to match the components they generate](https://github.com/rust-lang/rust/pull/90684) | ||
| - [#90645 Implement diagnostic for String conversion](https://github.com/rust-lang/rust/pull/90645) | ||
| - [#90559 Optimize bidi character detection.](https://github.com/rust-lang/rust/pull/90559) | ||
| - [#90542 Make RawVec private to alloc](https://github.com/rust-lang/rust/pull/90542) | ||
| - [#90535 Implement `clone_from` for `State`](https://github.com/rust-lang/rust/pull/90535) | ||
| - [#90489 rustdoc: Go back to loading all external crates unconditionally](https://github.com/rust-lang/rust/pull/90489) | ||
| - [#90485 Don't destructure args tuple in format_args!](https://github.com/rust-lang/rust/pull/90485) | ||
| - [#90462 [master] Fix CVE-2021-42574](https://github.com/rust-lang/rust/pull/90462) | ||
| - [#90443 Merge `DocContext.{ty,lt,ct}_substs` into one map](https://github.com/rust-lang/rust/pull/90443) | ||
| - [#90422 Rollup of 5 pull requests](https://github.com/rust-lang/rust/pull/90422) | ||
| - [#90382 std: Get the standard library compiling for wasm64](https://github.com/rust-lang/rust/pull/90382) | ||
| - [#90235 Rollup of 6 pull requests](https://github.com/rust-lang/rust/pull/90235) | ||
| - [#90183 Show all Deref implementations recursively](https://github.com/rust-lang/rust/pull/90183) | ||
| - [#90067 Rollup of 10 pull requests](https://github.com/rust-lang/rust/pull/90067) | ||
| - [#89939 Rollup of 10 pull requests](https://github.com/rust-lang/rust/pull/89939) | ||
| - [#89858 Rollup of 6 pull requests](https://github.com/rust-lang/rust/pull/89858) | ||
| - [#89695 Move top part of print_item to Tera templates](https://github.com/rust-lang/rust/pull/89695) | ||
| - [#89608 Rollup of 12 pull requests](https://github.com/rust-lang/rust/pull/89608) | ||
| - [#89580 Point at source of trait bound obligations in more places](https://github.com/rust-lang/rust/pull/89580) | ||
| - [#89551 Stabilize `const_raw_ptr_deref` for `*const T`](https://github.com/rust-lang/rust/pull/89551) | ||
| - [#89534 Introduce `tcx.get_diagnostic_name`](https://github.com/rust-lang/rust/pull/89534) | ||
| - [#89435 Rollup of 6 pull requests](https://github.com/rust-lang/rust/pull/89435) |