Copy link

Contributor

nikic commented 24 days ago

rust-timer commented 24 days ago

Awaiting bors try build completion.

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

bors commented 24 days ago

Trying commit cd8e450 with merge cc23adb...

This comment has been hidden.

This comment has been hidden.

bors commented 24 days ago

Test failed - checks-actions

nikic commented 24 days ago

@bors try

bors commented 24 days ago

Trying commit 07dfa39 with merge 610f099...

This comment has been hidden.

This comment has been hidden.

bors commented 24 days ago

Test failed - checks-actions

nikic commented 24 days ago

@bors try

bors commented 24 days ago

Trying commit 34f8765 with merge 2d4ed6f...

This comment has been hidden.

bors commented 24 days ago

Test failed - checks-actions

nikic commented 23 days ago

@bors try

bors commented 23 days ago

Trying commit de2b6aa with merge b87df8d...

bors commented 23 days ago

Try build successful - checks-actions
Build commit: b87df8d (b87df8d2c7c5d9ac448c585de10927ab2ee1b864)

rust-timer commented 23 days ago

rust-timer commented 23 days ago

Finished benchmarking commit (b87df8d): comparison url.

Summary: This benchmark run shows 134 relevant improvements but 58 relevant regressions to instruction counts.

• Average relevant regression: 0.6%
• Average relevant improvement: -1.1%
• Largest improvement in instruction counts: -3.3% on full builds of ctfe-stress-4 check
• Largest regression in instruction counts: 2.1% on incr-full builds of piston-image opt

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 led 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

nikic commented 9 days ago

@bors retry LLVM rebuild timeout

It looks like bors failed to kill the build here and it actually succeeded afterwards, and now bors is just stuck...

bors commented 9 days ago

Testing commit 75636bb with merge 30b3f35...

bors commented 8 days ago

Test successful - checks-actions
Approved by: nagisa
Pushing 30b3f35 to master...

glaubitz commented 8 days ago

Nice ;).

rust-timer commented 8 days ago

Finished benchmarking commit (30b3f35): comparison url.

Summary: This benchmark run shows 153 relevant improvements but 51 relevant regressions to instruction counts.

• Average relevant regression: 0.7%
• Average relevant improvement: -1.3%
• Largest improvement in instruction counts: -3.8% on full builds of projection-caching check
• Largest regression in instruction counts: 3.0% on incr-patched: add static arr item builds of coercions debug

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

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression

LLVM 14.0.0 final planned for Mar 15.
Rust 1.60.0 planned for Apr 7.

If there isn't an urgent reason to upgrade to LLVM 14 then maybe it is worth waiting for the release? IMO three weeks between when LLVM 14 final is expected to ship is not enough window. There's a good chance that the LLVM 14 release won't be fully rolled out or people will otherwise have trouble upgrading to LLVM 14.

When Rust upgrades to an as-yet-unreleased LLVM version, this breaks some (many?) people's CI/CD code coverage measurement, as often the LLVM tools version has to match Rust's LLVM version exactly in order for the coverage data to be accurate. It is pretty difficult to acquire a pre-release version of LLVM tools on certain platforms. Not sure if this aspect was overlooked or if it is expected breakage.

nikic commented 7 days ago

@briansmith I took a look at your CI config, and it looks like switching https://github.com/rustls/rustls/blob/5bda754ac18f37eb39132f89fb5522494b6202eb/.github/workflows/build.yml#L185 to llvm-toolchain-bionic-14 should work fine? This is available for pre-releases as well.

We always update LLVM for nightly well in advance of the release to make sure that any issues we discover can actually make it into the release. Our timing is chosen such that a stable LLVM release is available by the time the update makes it into a stable Rust release.

In this particular instance, we may want to revert the upgrade from the beta branch once promotion happens, because it landed very close to promotion this time, and we may want to give it more time to bake in nightly.

By the way, I believe -C instrument-coverage was recently stabilized, so you should be able to use a stable toolchain (with a stable LLVM release) for coverage in the future.

rylev commented 2 days ago

Upgrading LLVM is always likely to produce performance changes. Luckily the perf improvements seem to outweigh the perf regressions considerably both in number and magnitude.

@rustbot label: +perf-regression-triaged