Move more early buffered lints to dyn lint diagnostics#145881
Move more early buffered lints to dyn lint diagnostics#145881bors merged 5 commits intorust-lang:masterfrom
Conversation
e37e022 to
37c36c5
Compare
|
Seems worth a try. You might consider finishing the set you currently have and doing a perf run. |
|
@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. |
37c36c5 to
5cecd4a
Compare
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
[WIP] Move more early buffered lints to dyn lint diagnostics
|
@bors try cancel |
|
Try build cancelled. Cancelled workflows: |
5cecd4a to
6358700
Compare
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
[WIP] Move more early buffered lints to dyn lint diagnostics
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (9d79411): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking 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 Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
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.
CyclesResults (primary 2.2%, secondary 3.6%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 465.026s -> 464.899s (-0.03%) |
This comment was marked as resolved.
This comment was marked as resolved.
30e999b to
7d9c1e2
Compare
|
@fmease This is looking great. It seems worth merging the ones you have so far, to avoid the PR developing conflicts. r=me as soon as it passes CI and you're ready to merge it. |
7d9c1e2 to
5e2defc
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Dropped in favor a hard error in RUST-127907.
5e2defc to
ec7ad59
Compare
|
@bors r=joshtriplett |
|
☀️ Test successful - checks-actions |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing ddaf123 (parent) -> 52618eb (this PR) Test differencesShow 902 test diffsStage 0
Stage 1
(and 790 additional test diffs) Additionally, 12 doctest diffs were found. These are ignored, as they are noisy. Job group index Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 52618eb338609df44978b0ca4451ab7941fd1c7a --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (52618eb): comparison URL. Overall result: ❌ regressions - no action needed@rustbot label: -perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -0.9%, secondary 0.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -2.3%, secondary -4.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 473.077s -> 474.182s (0.23%) |
…elmann Move even more early buffered lints to dyn lint diagnostics Follow-up to #145881 and #145747. I originally wanted to migrate most if not the entire rest of the early buffered lints. However, when trying to migrate the buffered lints used by check-cfg I encountered a roadblock. Namely, it depends on `TyCtxt` (well, `Option<TyCtxt>`) which makes it quite hard to migrate (see also #147634 (comment), #147634 (comment) & #149215). So for now, I won't migrate it (maybe #149215 will find a solution), nor will I migrate the rest since it's quite tedious to migrate these. I'll leave them for future me.
…elmann Move even more early buffered lints to dyn lint diagnostics Follow-up to rust-lang/rust#145881 and rust-lang/rust#145747. I originally wanted to migrate most if not the entire rest of the early buffered lints. However, when trying to migrate the buffered lints used by check-cfg I encountered a roadblock. Namely, it depends on `TyCtxt` (well, `Option<TyCtxt>`) which makes it quite hard to migrate (see also rust-lang/rust#147634 (comment), rust-lang/rust#147634 (comment) & rust-lang/rust#149215). So for now, I won't migrate it (maybe rust-lang/rust#149215 will find a solution), nor will I migrate the rest since it's quite tedious to migrate these. I'll leave them for future me.
Follow-up to #145747.
Presently, it's unclear to me if it's possible to migrate all variants to dyn lint diagnostics without regressing performance because for some early lints
decorate_builtin_lintperforms 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).