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 5 pull requests #108640

Merged
merged 11 commits into from
Mar 2, 2023
Merged

Rollup of 5 pull requests #108640

merged 11 commits into from
Mar 2, 2023

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

compiler-errors and others added 11 commits February 28, 2023 17:29
…mpiler-errors

Restrict `#[rustc_box]` to `Box::new` calls

Currently, `#[rustc_box]` can be applied to any call expression with a single argument. This PR only allows it to be applied to calls to `Box::new`
Erase **all** regions when probing for associated types on ambiguity in astconv

Fixes rust-lang#108562
…nner, r=tmandry

Run compiler test suite in parallel on Fuchsia

This also adds file locking around calls to `pm publish` as these calls are not thread-safe.
…mpiler-errors

Add test case for mismatched open/close delims

Fixes rust-lang#104367
Fixes rust-lang#105209

After landing rust-lang#108297, these issues are resolved.
Highlight whole expression for E0599

Fixes rust-lang#108603

This adds a secondary label to highlight the whole expression leading to the error. It also prevents empty labels being recognised as 'unexpected' by compiletest - otherwise, tests with NOTE annotations would pick up empty labels.

`@rustbot` label +A-diagnostics
@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc 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-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Mar 2, 2023
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=5

@bors
Copy link
Contributor

bors commented Mar 2, 2023

📌 Commit 832987b has been approved by matthiaskrgr

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 2, 2023
@bors
Copy link
Contributor

bors commented Mar 2, 2023

⌛ Testing commit 832987b with merge 4912485f15667799e847fd3ae601e2e3d6837b50...

@bors
Copy link
Contributor

bors commented Mar 2, 2023

💥 Test timed out

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 2, 2023
@matthiaskrgr
Copy link
Member Author

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 2, 2023
@bors
Copy link
Contributor

bors commented Mar 2, 2023

⌛ Testing commit 832987b with merge 7e966bc...

@bors
Copy link
Contributor

bors commented Mar 2, 2023

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing 7e966bc to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 2, 2023
@bors bors merged commit 7e966bc into rust-lang:master Mar 2, 2023
@rustbot rustbot added this to the 1.69.0 milestone Mar 2, 2023
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Perf Build Sha
#108609 7a4b1a20b900dfffaca19ba6ef85c73965cab5b0
#108606 9ab93fea0a96e573a837990b60e655ea7c4ee216
#108585 0638a6d5124ae688ac3e35f4f54eeead662f777b
#108575 920d37d09964cf017aa3a0ccb49d4e3dd34e134b
#108516 e1c2a35058d45644d8ad0f3a09dd85cfe382c15e

previous master: 18caf88956

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 (7e966bc): comparison URL.

Overall result: ❌ regressions - 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.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
3.2% [2.1%, 4.4%] 8
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

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.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.0% [-3.5%, -2.6%] 2
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.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
4.7% [2.3%, 6.2%] 10
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

@rustbot rustbot added the perf-regression Performance regression. label Mar 2, 2023
@lqd
Copy link
Member

lqd commented Mar 2, 2023

#108575 is supposed to be on the error path and shouldn't be the cause here, but the regressions do contain more query traffic for associated_items, and it seems to be one of the few related things in this rollup.

So let's see just in case:
@rust-timer build 920d37d09964cf017aa3a0ccb49d4e3dd34e134b

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (920d37d09964cf017aa3a0ccb49d4e3dd34e134b): comparison URL.

Overall result: no relevant changes - 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-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

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.

mean range count
Regressions ❌
(primary)
3.1% [3.1%, 3.1%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 3.1% [3.1%, 3.1%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

@rustbot rustbot removed the perf-regression Performance regression. label Mar 2, 2023
@lqd
Copy link
Member

lqd commented Mar 2, 2023

@rustbot label: +perf-regression

@rustbot rustbot added the perf-regression Performance regression. label Mar 2, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (920d37d09964cf017aa3a0ccb49d4e3dd34e134b): comparison URL.

Overall result: no relevant changes - 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-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

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.

mean range count
Regressions ❌
(primary)
3.1% [3.1%, 3.1%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 3.1% [3.1%, 3.1%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

@rustbot rustbot removed the perf-regression Performance regression. label Mar 2, 2023
@lqd
Copy link
Member

lqd commented Mar 3, 2023

Come on, bot.

@rustbot label: +perf-regression

@rustbot rustbot added the perf-regression Performance regression. label Mar 3, 2023
@nnethercote
Copy link
Contributor

Oh dear: I suspect deep-vector has succumbed to a similar problem as keccak and cranelift-codegen, i.e. bimodal codegen of process_obligations.

I was just doing some Cachegrind profiling of deep-vector, and noticed that process_obligations was the hottest function, which was not the case last week. I did a cg_diff, the changes are all in and around process_obligations. For me the difference with a check-full build is only 2%, not 4%, but I think this is the explanation :(

@rylev
Copy link
Member

rylev commented Mar 7, 2023

@nnethercote checking for triage: is there anything we should do to try to address this or do we just take this as another test case we should expect to see occasional perf swings in?

@nnethercote
Copy link
Contributor

keccak and cranelift-codegen are always bimodal in sync. This deep-vector regression involves the same code but appears to be a once-off. I won't pretend to understand why it's behaving differently. But I am confident it's a question of code generation quality rather than, say, some change in the compiler that caused it to call the relevant functions more times. (I can tell this because in the Cachegrind profiles a lot of the lines in the relevant functions have the same number of instruction counts, but then others have higher instruction counts.)

@rylev
Copy link
Member

rylev commented Mar 8, 2023

I think that's enough justification to call this triaged. While we don't have a good explanation, there's no real point in trying to find code changes in this rollup that are to blame.

@rustbot label +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Mar 8, 2023
@matthiaskrgr matthiaskrgr deleted the rollup-rii4t5t branch March 16, 2024 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. 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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure 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