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 #130281

Merged
merged 13 commits into from
Sep 12, 2024
Merged

Rollup of 5 pull requests #130281

merged 13 commits into from
Sep 12, 2024

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

RalfJung and others added 13 commits September 8, 2024 08:45
`rustc_resolve` allocates many things in `ResolverArenas`. The lifetime
used for references into the arena is mostly `'a`, and sometimes `'b`.

This commit changes it to `'ra`, which is much more descriptive. The
commit also changes the order of lifetimes on a couple of structs so
that '`ra` is second last, before `'tcx`, and does other minor
renamings such as `'r` to `'a`.
some const cleanup: remove unnecessary attributes, add const-hack indications

I learned that we use `FIXME(const-hack)` on top of the "const-hack" label. That seems much better since it marks the right place in the code and moves around with the code. So I went through the PRs with that label and added appropriate FIXMEs in the code. IMO this means we can then remove the label -- Cc ``@rust-lang/wg-const-eval.``

I also noticed some const stability attributes that don't do anything useful, and removed them.

r? ``@fee1-dead``
…chenkov

Introduce `'ra` lifetime name.

`rustc_resolve` allocates many things in `ResolverArenas`. The lifetime used for references into the arena is mostly `'a`, and sometimes `'b`.

This commit changes it to `'rslv`, which is much more descriptive. The commit also changes the order of lifetimes on a couple of structs so that '`rslv` is second last, before `'tcx`, and does other minor renamings such as `'r` to `'a`.

r? ``@petrochenkov``
cc ``@oli-obk``
coverage: Simplify creation of sum counters

A small and self-contained improvement, extracted from some larger changes that I'm still working on.

Ultimately I want to avoid creating these sum counter-expressions in some cases (in favour of just adding physical counters directly to the nodes we care about), so a good incremental move towards that is splitting the “gather edge counters” step out from the ”build a sum of those counters” step.

Creating an extra intermediate vector should have negligible cost (and coverage isn't exercised by the benchmark suite anyway). The removed logging is redundant with the `#[instrument(..)]` logging we already have on the underlying method calls.
…mpiler-errors

more eagerly discard constraints on overflow

We always discard the results of overflowing goals inside of the trait solver. We previously did so when instantiating the response in `evaluate_goal`. Canonicalizing results only to later discard them is also  inefficient 🤷

It's simpler and nicer to debug to eagerly discard constraints inside of the query itself.

r? ``@compiler-errors``
Add test for nalgebra hang in coherence

r? lcnr
@rustbot rustbot added O-windows Operating system: Windows 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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) rollup A PR which is a rollup labels Sep 12, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=5

@bors
Copy link
Contributor

bors commented Sep 12, 2024

📌 Commit ed1602e 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 Sep 12, 2024
@bors
Copy link
Contributor

bors commented Sep 12, 2024

⌛ Testing commit ed1602e with merge 2e8db5e...

@bors
Copy link
Contributor

bors commented Sep 12, 2024

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing 2e8db5e to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 12, 2024
@bors bors merged commit 2e8db5e into rust-lang:master Sep 12, 2024
7 checks passed
@rustbot rustbot added this to the 1.83.0 milestone Sep 12, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#130101 some const cleanup: remove unnecessary attributes, add cons… 29a231167dee8a7031b88855203405aa18f24792 (link)
#130208 Introduce 'ra lifetime name. 820de58f1d957e6b4209d5b4b75f0c08735a6b04 (link)
#130263 coverage: Simplify creation of sum counters 10f5c64d92460b2adcda13fa024f57f85b9d6664 (link)
#130273 more eagerly discard constraints on overflow 55063c34e1393b3383ef2793372728b503f4a932 (link)
#130276 Add test for nalgebra hang in coherence c886379ffc38c264ed782551a9debdac9636c215 (link)

previous master: 8c0ec05f7d

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 (2e8db5e): 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.

mean range count
Regressions ❌
(primary)
0.6% [0.6%, 0.6%] 1
Regressions ❌
(secondary)
0.2% [0.2%, 0.2%] 1
Improvements ✅
(primary)
-0.2% [-0.2%, -0.2%] 1
Improvements ✅
(secondary)
-1.8% [-2.0%, -1.6%] 6
All ❌✅ (primary) 0.2% [-0.2%, 0.6%] 2

Max RSS (memory usage)

Results (primary 0.8%, secondary 1.1%)

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)
5.7% [2.8%, 9.7%] 3
Regressions ❌
(secondary)
3.1% [2.6%, 4.0%] 3
Improvements ✅
(primary)
-4.2% [-8.1%, -0.5%] 3
Improvements ✅
(secondary)
-1.9% [-2.7%, -1.1%] 2
All ❌✅ (primary) 0.8% [-8.1%, 9.7%] 6

Cycles

Results (secondary -2.2%)

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.3%, -2.1%] 3
All ❌✅ (primary) - - 0

Binary size

Results (primary 0.2%, secondary 0.0%)

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.2% [0.0%, 0.5%] 49
Regressions ❌
(secondary)
0.1% [0.1%, 0.2%] 4
Improvements ✅
(primary)
-0.1% [-0.5%, -0.0%] 6
Improvements ✅
(secondary)
-0.0% [-0.0%, -0.0%] 18
All ❌✅ (primary) 0.2% [-0.5%, 0.5%] 55

Bootstrap: 757.007s -> 755.803s (-0.16%)
Artifact size: 341.21 MiB -> 341.20 MiB (-0.00%)

@rustbot rustbot added the perf-regression Performance regression. label Sep 12, 2024
@Mark-Simulacrum Mark-Simulacrum added the perf-regression-triaged The performance regression has been triaged. label Sep 16, 2024
@Mark-Simulacrum
Copy link
Member

Regressions are minor enough that given the rollup and lack of obviously relevant PRs I'm just marking as triaged. The primary regressed benchmark is webrender and only in full LLVM builds which doesn't seem like it maps well onto the contained PRs.

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. O-windows Operating system: Windows 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-libs Relevant to the library 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
Development

Successfully merging this pull request may close these issues.