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.

Sign up for GitHub

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

Jump to bottom

Rollup of 6 pull requests #107870

Merged
merged 16 commits into from
Feb 10, 2023
Merged

Rollup of 6 pull requests #107870

merged 16 commits into from
Feb 10, 2023

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

Noratrieb and others added 16 commits January 18, 2023 20:46
@Noratrieb
Support true and false as boolean flag params
a6fda3e 
Implements MCP 577.
@Noratrieb
Change codegen documentation
7605853
@nnethercote
Simplify tls::enter_context.
f7b3e39
@nnethercote
Simplify with_tlv.
18f751d
@nnethercote
Avoid some tls::with calls.
afbe167 
These are in places where a `tcx` is easily obtained.
@nnethercote
Remove QueryContext.
243944c 
There is a type `QueryCtxt`, which impls the trait `QueryContext`.
Confusingly, there is another type `QueryContext`. The latter is (like
`TyCtxt`) just a pointer to a `GlobalContext`. It's not used much, e.g.
its `impl` block has a single method.

This commit removes `QueryContext`, replacing its use with direct
`GlobalCtxt` use.
@tharunsuresh-code
Handled snap curl issue inside Rust rust-lang#107722
858a4aa
@notriddle
rustdoc: remove unused fn parameter tab
792b846
@cuviper
Release 1.67.1
6e1a28b
@compiler-errors
Multiple candidates with same response is not ambiguous
3c4e1f8
@matthiaskrgr
Rollup merge of rust-lang#107043 - Nilstrieb:true-and-false-is-false,…
8fc9ed5 
… r=wesleywiser

Support `true` and `false` as boolean flag params

Implements [MCP 577](rust-lang/compiler-team#577).
@matthiaskrgr
Rollup merge of rust-lang#107831 - nnethercote:query-refactoring, r=o…
11e1280 
…li-obk

Query refactoring

Just some cleanups I found when learning about the query system.

Best reviewed one commit at a time.

r? `@oli-obk`
@matthiaskrgr
Rollup merge of rust-lang#107841 - tharunsuresh-code:snap_curl, r=ozk…
8bfb97c 
…anonur

Handled snap curl issue inside Rust
@matthiaskrgr
Rollup merge of rust-lang#107852 - notriddle:notriddle/rm-tab, r=Guil…
d4d1c93 
…laumeGomez

rustdoc: remove unused fn parameter `tab`
@matthiaskrgr
Rollup merge of rust-lang#107861 - cuviper:relnotes-1.67.1, r=Mark-Si…
dd5e4ad 
…mulacrum

Sync release notes for 1.67.1

Closes rust-lang#107859.
@matthiaskrgr
Rollup merge of rust-lang#107863 - compiler-errors:new-solver-multipl…
9300617 
…e-candidates, r=jackh726

Allow multiple candidates with same response in new solver

Treat >1 candidates as *not* ambiguous if they return the same response.
@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-release Relevant to the release subteam, 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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) rollup A PR which is a rollup labels Feb 10, 2023
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=6

@bors
Copy link
Contributor

bors commented Feb 10, 2023

📌 Commit 9300617 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 Feb 10, 2023
@bors
Copy link
Contributor

bors commented Feb 10, 2023

⌛ Testing commit 9300617 with merge 9b8dbd5...

@bors bors mentioned this pull request Feb 10, 2023
Merged
@bors
Copy link
Contributor

bors commented Feb 10, 2023

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing 9b8dbd5 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 10, 2023
@bors bors merged commit 9b8dbd5 into rust-lang:master Feb 10, 2023
@rustbot rustbot added this to the 1.69.0 milestone Feb 10, 2023
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Perf Build Sha
#107863 5677b86c43cf70b41403958c90e2ade33123b6b3
#107861 d8afa051155387eeed7a115b9ab890a9fcdc6fc8
#107852 832a0c2e1ef6280bd7a545600fb6b76bcea4965c
#107841 e3f76e762eac9c0d81533d9fcecc9a487506800b
#107831 33084aaad8826feab0d8bbf0af5ef8c7bb416767
#107043 d0a98bb3e1f4620738a06d705765b266473c81e1

previous master: a697573463

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

This was referenced Feb 10, 2023
Closed
Merged
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (9b8dbd5): 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.7% [0.5%, 1.0%] 7
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) 0.7% [0.5%, 1.0%] 7

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)		 1.9% [1.9%, 1.9%] 1
Improvements ✅
(primary)		 -4.0% [-4.0%, -4.0%] 1
Improvements ✅
(secondary)		 -2.9% [-3.4%, -2.5%] 10
All ❌✅ (primary) -4.0% [-4.0%, -4.0%] 1

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)		 - - 0
Improvements ✅
(primary)		 -3.1% [-3.1%, -3.1%] 1
Improvements ✅
(secondary)		 -2.5% [-2.5%, -2.5%] 1
All ❌✅ (primary) -3.1% [-3.1%, -3.1%] 1

@rustbot rustbot added the perf-regression Performance regression. label Feb 10, 2023
@rylev
Copy link
Member

rylev commented Feb 14, 2023

@rust-timer build 5677b86c43cf70b41403958c90e2ade33123b6b3

@rust-timer

This comment has been minimized.

Sign in to view
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (5677b86c43cf70b41403958c90e2ade33123b6b3): 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)

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

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)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -2.6% [-2.6%, -2.6%] 1
All ❌✅ (primary) - - 0

@rustbot rustbot removed the perf-regression Performance regression. label Feb 14, 2023
@rylev
Copy link
Member

rylev commented Feb 14, 2023 •
edited
Loading

@rust-timer build 33084aaad8826feab0d8bbf0af5ef8c7bb416767

@rust-timer

This comment has been minimized.

Sign in to view
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (33084aaad8826feab0d8bbf0af5ef8c7bb416767): comparison URL.

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

mean range count
Regressions ❌
(primary)		 0.7% [0.4%, 1.0%] 7
Regressions ❌
(secondary)		 0.5% [0.4%, 0.5%] 2
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) 0.7% [0.4%, 1.0%] 7

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)		 -2.2% [-2.2%, -2.2%] 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.

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

@rustbot rustbot added the perf-regression Performance regression. label Feb 15, 2023
@rylev
Copy link
Member

rylev commented Feb 15, 2023

It seems like #107831 is the cause of the regression cc @nnethercote @oli-obk

@oli-obk
Copy link
Contributor

oli-obk commented Feb 15, 2023

Looking at the graph, I think there is no regression here. This looks like a reversal of the "improvement" in https://perf.rust-lang.org/compare.html?start=9433ba63942a736f068baacf19e5cffaff6dccc9&end=ef934d9b632b8ac00276558824664c104b92b5f0&stat=instructions:u

@nnethercote
Copy link
Contributor

The wg-grammar change is very likely noise. The bitmaps change I am less certain about. Although I wouldn't have expected this change to affect perf, I will do some local measurements to see if I can work anything out.

@nnethercote
Copy link
Contributor

I did some local measurements with Cachegrind, starting with the current trunk and then de-applying the four commits from #107831 one at a time:

Original    2,579,711,048 (100.0%)  PROGRAM TOTALS
MinusQ4     2,579,711,451 (100.0%)  PROGRAM TOTALS
MinusQ3     2,579,646,820 (100.0%)  PROGRAM TOTALS
MinusQ2     2,579,439,041 (100.0%)  PROGRAM TOTALS
MinusQ1     2,579,437,639 (100.0%)  PROGRAM TOTALS

That's a 0.02% reduction in instruction counts by de-applying the four commits, i.e. negligible. Which strongly suggests that #107831 didn't have any meaningful perf effects.

@rylev
Copy link
Member

rylev commented Feb 16, 2023

@rustbot label: perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Feb 16, 2023
@matthiaskrgr matthiaskrgr deleted the rollup-3z1q4rm branch March 16, 2024 18:18
@matthiaskrgr matthiaskrgr mentioned this pull request Apr 20, 2024
Closed
@matthiaskrgr matthiaskrgr mentioned this pull request May 6, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) 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-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-release Relevant to the release subteam, 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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Milestone
1.69.0
Development

Successfully merging this pull request may close these issues.