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

More linting tweaks #105416

Merged
merged 5 commits into from
Dec 10, 2022
Merged

More linting tweaks #105416

merged 5 commits into from
Dec 10, 2022

Conversation

nnethercote
Copy link
Contributor

Squeeze a little more blood from this stone.

r? @cjgillot

Because it makes more sense that way.
These each have a single call site, due to being called from a
"combined" lint pass.
@rustbot rustbot added 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. labels Dec 7, 2022
@nnethercote
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Dec 7, 2022
@bors
Copy link
Contributor

bors commented Dec 7, 2022

⌛ Trying commit d049be3 with merge 858ff1cdb6da6c41638d404e35a962f37e2fc512...

@bors
Copy link
Contributor

bors commented Dec 7, 2022

☀️ Try build successful - checks-actions
Build commit: 858ff1cdb6da6c41638d404e35a962f37e2fc512 (858ff1cdb6da6c41638d404e35a962f37e2fc512)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (858ff1cdb6da6c41638d404e35a962f37e2fc512): comparison URL.

Overall result: ❌ regressions - 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 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)
1.6% [1.6%, 1.6%] 1
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)
-3.3% [-3.3%, -3.3%] 1
Improvements ✅
(secondary)
-2.2% [-2.2%, -2.2%] 2
All ❌✅ (primary) -3.3% [-3.3%, -3.3%] 1

Cycles

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

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Dec 7, 2022
@cjgillot
Copy link
Contributor

cjgillot commented Dec 7, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Dec 7, 2022

📌 Commit d049be3 has been approved by cjgillot

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 Dec 7, 2022
@nnethercote
Copy link
Contributor Author

nnethercote commented Dec 7, 2022

Disappoining perf results, I saw improvements locally. I guess with PGO/BOLT the version compiled on CI was already inlining those functions. Though they may still help platforms that aren't built with PGO/BOLT.

@bors
Copy link
Contributor

bors commented Dec 7, 2022

⌛ Testing commit d049be3 with merge 85868c6419b82d9c77d6f17ddd82339a2827a375...

@bors
Copy link
Contributor

bors commented Dec 7, 2022

💔 Test failed - checks-actions

@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 Dec 7, 2022
@nnethercote
Copy link
Contributor Author

Seems to be a network error, the failing log ends with several repetitions of this:

fatal: could not read Username for 'https://github.com/': No such device or address
Sleeping for 3 seconds before retrying push
From https://github.com/rust-lang-nursery/rust-toolstate
 * branch            master     -> FETCH_HEAD
HEAD is now at 496fb40 (windows CI update)
[master e3015cb] (linux CI update)
 1 file changed, 1 insertion(+)

@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 Dec 7, 2022
@bors
Copy link
Contributor

bors commented Dec 7, 2022

⌛ Testing commit d049be3 with merge 910b4af6a0a20dcd18974e9196a195f8daf5ecd6...

@bors
Copy link
Contributor

bors commented Dec 7, 2022

💔 Test failed - checks-actions

@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 Dec 7, 2022
@nnethercote
Copy link
Contributor Author

@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 Dec 8, 2022
@rust-log-analyzer

This comment was marked as outdated.

@rust-log-analyzer

This comment was marked as outdated.

@bors
Copy link
Contributor

bors commented Dec 10, 2022

⌛ Testing commit d049be3 with merge c6fcdb6...

@bors
Copy link
Contributor

bors commented Dec 10, 2022

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing c6fcdb6 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 10, 2022
@bors bors merged commit c6fcdb6 into rust-lang:master Dec 10, 2022
@rustbot rustbot added this to the 1.68.0 milestone Dec 10, 2022
@bors bors mentioned this pull request Dec 10, 2022
5 tasks
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (c6fcdb6): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -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
Regressions ❌
(secondary)
1.4% [1.4%, 1.4%] 1
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)
2.0% [0.9%, 3.4%] 3
Improvements ✅
(primary)
-2.9% [-2.9%, -2.9%] 1
Improvements ✅
(secondary)
-3.0% [-4.7%, -1.7%] 6
All ❌✅ (primary) -2.9% [-2.9%, -2.9%] 1

Cycles

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

@nnethercote nnethercote deleted the more-linting-tweaks branch December 11, 2022 22:49
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 12, 2022
…s, r=cjgillot

Fix lint perf regressions

rust-lang#104863 caused small but widespread regressions in lint performance. I tried to improve things in rust-lang#105291 and rust-lang#105416 with minimal success, before fully understanding what caused the regression. This PR effectively reverts all of rust-lang#105291 and part of rust-lang#104863 to fix the perf regression.

r? `@cjgillot`
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
…jgillot

More linting tweaks

Squeeze a little more blood from this stone.

r? `@cjgillot`
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. 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants