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

Rollup of 6 pull requests #101439

Merged
merged 19 commits into from
Sep 5, 2022
Merged

Rollup of 6 pull requests #101439

merged 19 commits into from
Sep 5, 2022

Conversation

Dylan-DPC
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

nnethercote and others added 19 commits August 29, 2022 06:35
This is based on `-Zprint-type-sizes` which does the same thing. It
makes the output provenance clearer, and helps with post-processing.
E.g. if you have `-Zhir-stats` output from numerous compiler invocations
you can now easily extract the pre-expansion stats separately from the
post-expansion stats.
For consistency, and because it makes HIR measurement simpler and more
accurate.
For consistency, and because it makes HIR measurement simpler and more
accurate.
For consistency, and because it makes HIR measurement simpler and more
accurate.
This comment on the HIR `visit_path_segment` is supposed be on the AST
`visit_path_segment`.
Because `hir_id` is the standard name for methods that return a `HirId`
from a HIR node.
Adds and removes some `visit_*` methods accordingly, improving
coverage, and avoiding some double counting. Brings it in line with the
AST stats collector.
…avidtwco

Improve HIR stats

rust-lang#100398 improve the AST stats collection done by `-Zhir-stats`. This PR does the same for HIR stats collection.

r? `@davidtwco`
…cloned, r=lcnr

Suggest `{Option,Result}::{copied,clone}()` to satisfy type mismatch

Fixes rust-lang#100699, but in the opposite direction (instead of suggesting to fix the signature, it fixes the body)
…ty_no_warn_in_2021_crates, r=TaKO8Ki

Don't fire `rust_2021_incompatible_closure_captures` in `edition = 2021` crates

Fixes rust-lang#101284
Fix `hir::Local` doc to match with the variable name used: `init`
Don't suggest reborrow if usage is inside a closure

I can't think of why we would ever be able to *successfully* suggest a mutable reborrow `&mut *` due to a move happening due to a closure, so just suppress it.

Fixes rust-lang#101119
@rustbot rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Sep 5, 2022
@Dylan-DPC
Copy link
Member Author

@bors r+ rollup=never p=5

@bors
Copy link
Contributor

bors commented Sep 5, 2022

📌 Commit d2fdb5d has been approved by Dylan-DPC

It is now in the queue for this repository.

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Sep 5, 2022
@bors
Copy link
Contributor

bors commented Sep 5, 2022

⌛ Testing commit d2fdb5d with merge 6e4a9ab...

@bors
Copy link
Contributor

bors commented Sep 5, 2022

☀️ Test successful - checks-actions
Approved by: Dylan-DPC
Pushing 6e4a9ab to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 5, 2022
@bors bors merged commit 6e4a9ab into rust-lang:master Sep 5, 2022
@rustbot rustbot added this to the 1.65.0 milestone Sep 5, 2022
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Perf Build Sha
#101429 1692b9599d9e3d535b975197a9a2ed1a69c19656
#101420 eff96fea00a76f4d0a933c51db381c7762c4d6f5
#101409 47093b0902bf68606f249d4fed363fe60ff8c34b
#101391 0fbbf672012383c11ca93a3290bdcc8dbb1ebf71
#101367 e3f98bf4b560316107dadf882e3995a79df7b0f4
#101142 bfa6968e0b1354384b6b1ec3cfe5e2bd3e77d4fd

previous master: 5b4bd154de

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (6e4a9ab): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

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
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean1 range count2
Regressions ❌
(primary)
0.9% [0.9%, 1.0%] 2
Regressions ❌
(secondary)
2.1% [1.8%, 2.4%] 6
Improvements ✅
(primary)
-2.9% [-29.7%, -0.3%] 18
Improvements ✅
(secondary)
-1.2% [-1.8%, -0.7%] 9
All ❌✅ (primary) -2.5% [-29.7%, 1.0%] 20

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean1 range count2
Regressions ❌
(primary)
1.5% [1.0%, 2.0%] 2
Regressions ❌
(secondary)
2.6% [2.1%, 3.9%] 5
Improvements ✅
(primary)
-6.9% [-13.9%, -2.7%] 4
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -4.1% [-13.9%, 2.0%] 6

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean1 range count2
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.5% [2.1%, 2.8%] 2
Improvements ✅
(primary)
-16.4% [-35.7%, -4.6%] 3
Improvements ✅
(secondary)
-2.6% [-2.6%, -2.6%] 1
All ❌✅ (primary) -16.4% [-35.7%, -4.6%] 3

Footnotes

  1. the arithmetic mean of the percent change 2 3

  2. number of relevant changes 2 3

@rustbot rustbot added the perf-regression Performance regression. label Sep 6, 2022
@nnethercote
Copy link
Contributor

The regressions are mostly on keccak, which has been noisy recently, and so can be ignored.

But there are plenty of improvements here, including some huge ones on cargo patch incremental runs. I wonder if #101409 is the cause:
@rust-timer build 7093b0902bf68606f249d4fed363fe60ff8c34b

@nnethercote
Copy link
Contributor

Whoops, I chopped off the first digit in the SHA:
@rust-timer build 47093b0902bf68606f249d4fed363fe60ff8c34b

@rust-timer
Copy link
Collaborator

Queued 47093b0902bf68606f249d4fed363fe60ff8c34b with parent 5b4bd15, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (47093b0902bf68606f249d4fed363fe60ff8c34b): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

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

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean1 range count2
Regressions ❌
(primary)
1.0% [0.9%, 1.0%] 2
Regressions ❌
(secondary)
2.1% [1.9%, 2.4%] 6
Improvements ✅
(primary)
-13.7% [-29.6%, -4.1%] 3
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -7.8% [-29.6%, 1.0%] 5

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean1 range count2
Regressions ❌
(primary)
1.5% [0.9%, 2.1%] 2
Regressions ❌
(secondary)
2.2% [1.6%, 3.5%] 6
Improvements ✅
(primary)
-7.1% [-13.8%, -3.2%] 4
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -4.2% [-13.8%, 2.1%] 6

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean1 range count2
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-16.7% [-36.3%, -5.0%] 3
Improvements ✅
(secondary)
-2.8% [-2.8%, -2.8%] 1
All ❌✅ (primary) -16.7% [-36.3%, -5.0%] 3

Footnotes

  1. the arithmetic mean of the percent change 2 3

  2. number of relevant changes 2 3

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 6, 2022
@nnethercote
Copy link
Contributor

Ok, so #101409 is responsible for the cargo improvements, but it doesn't seem to explain a lot of the small improvements. I'm not sure which other PR might be responsible for those.

@mati865
Copy link
Contributor

mati865 commented Sep 6, 2022

Maybe one of changes in #101391 has touched hot code?

@nnethercote
Copy link
Contributor

@rust-timer build 0fbbf672012383c11ca93a3290bdcc8dbb1ebf71

@rust-timer
Copy link
Collaborator

Queued 0fbbf672012383c11ca93a3290bdcc8dbb1ebf71 with parent 5b4bd15, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (0fbbf672012383c11ca93a3290bdcc8dbb1ebf71): comparison URL.

Overall result: ✅ improvements - no action needed

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.

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

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean1 range count2
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.8% [-1.3%, -0.3%] 14
Improvements ✅
(secondary)
-1.1% [-1.8%, -0.5%] 11
All ❌✅ (primary) -0.8% [-1.3%, -0.3%] 14

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean1 range count2
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
4.2% [4.2%, 4.2%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.5% [-3.5%, -3.5%] 1
All ❌✅ (primary) - - 0

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean1 range count2
Regressions ❌
(primary)
2.4% [2.4%, 2.4%] 1
Regressions ❌
(secondary)
2.3% [2.3%, 2.3%] 1
Improvements ✅
(primary)
-2.3% [-2.3%, -2.3%] 1
Improvements ✅
(secondary)
-2.8% [-2.8%, -2.8%] 1
All ❌✅ (primary) 0.1% [-2.3%, 2.4%] 2

Footnotes

  1. the arithmetic mean of the percent change 2 3

  2. number of relevant changes 2 3

@rustbot rustbot removed the perf-regression Performance regression. label Sep 6, 2022
@nnethercote
Copy link
Contributor

I have since learned that we have some very noisy/bimodal benchmarks at the moment, particularly keccak, html5ever, serde_derive, and tt-muncher... which account for almost all of the non-cargo changes. So I think #101409 is the only PR in this rollup that genuinely affected perf, and that was in a good way.

@rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. perf-regression-triaged The performance regression has been triaged. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants