Skip to content

Conversation

fmease
Copy link
Member

@fmease fmease commented Aug 26, 2025

Follow-up to #145747.

TODO: Migrate more variants. Presently, it's unclear if it's possible to migrate all variants to dyn lint diagnostics without regressing performance because for some early lints decorate_builtin_lint performs a bit more work (past PR #124417 has shown that eagerly decorating early lints is incredibly heavy and we had to revert back to lazily decorating in #125410). Let's see how this fares once I tackle the more 'risky' variants).

cc @joshtriplett (you can immediately unsubscribe again, I just want to prevent duplicate efforts).

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 26, 2025
@fmease fmease added the rla-silenced Silences rust-log-analyzer postings to the PR it's added on. label Aug 26, 2025
@fmease fmease force-pushed the mv-var-to-dyn-buf-lints branch 2 times, most recently from fa582a9 to 5e2b622 Compare August 27, 2025 11:48
@fmease fmease force-pushed the mv-var-to-dyn-buf-lints branch from e37e022 to 37c36c5 Compare August 27, 2025 14:31
@joshtriplett
Copy link
Member

Seems worth a try.

You might consider finishing the set you currently have and doing a perf run.

@joshtriplett
Copy link
Member

@fmease Regarding the potential performance issues, reading the previous PRs I don't think this is likely to hit the same issues. It could potentially regress for other reasons, but this change won't make the decorating any less lazy.

@fmease fmease force-pushed the mv-var-to-dyn-buf-lints branch from 37c36c5 to 5cecd4a Compare September 2, 2025 18:54
@fmease
Copy link
Member Author

fmease commented Sep 2, 2025

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rust-bors

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request Sep 2, 2025
[WIP] Move more early buffered lints to dyn lint diagnostics
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 2, 2025
@fmease
Copy link
Member Author

fmease commented Sep 2, 2025

@bors try cancel

@rust-bors
Copy link

rust-bors bot commented Sep 2, 2025

Try build cancelled. Cancelled workflows:

@fmease fmease force-pushed the mv-var-to-dyn-buf-lints branch from 5cecd4a to 6358700 Compare September 2, 2025 19:10
@fmease
Copy link
Member Author

fmease commented Sep 2, 2025

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request Sep 2, 2025
[WIP] Move more early buffered lints to dyn lint diagnostics
@rust-bors

This comment has been minimized.

@rust-bors
Copy link

rust-bors bot commented Sep 2, 2025

☀️ Try build successful (CI)
Build commit: 9d79411 (9d794119e470cc679493ce04193550fe54c76727, parent: a2c8b0b92c14b02f0b3f96a0d5296f1090dc286b)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (9d79411): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.1% [-0.2%, -0.1%] 14
Improvements ✅
(secondary)
-0.3% [-0.7%, -0.1%] 18
All ❌✅ (primary) -0.1% [-0.2%, -0.1%] 14

Max RSS (memory usage)

Results (primary 1.3%, secondary -0.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
1.3% [1.3%, 1.3%] 1
Regressions ❌
(secondary)
1.7% [1.2%, 2.2%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.0% [-2.1%, -2.0%] 2
All ❌✅ (primary) 1.3% [1.3%, 1.3%] 1

Cycles

Results (primary 2.2%, secondary 3.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

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

Binary size

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

Bootstrap: 465.026s -> 464.899s (-0.03%)
Artifact size: 388.33 MiB -> 388.40 MiB (0.02%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rla-silenced Silences rust-log-analyzer postings to the PR it's added on. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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.

4 participants