perf: skip nonstandard_macro_braces work when the lint is disabled - #17125
Closed
xmakro wants to merge 1 commit into
Closed
perf: skip nonstandard_macro_braces work when the lint is disabled#17125xmakro wants to merge 1 commit into
xmakro wants to merge 1 commit into
Conversation
…sabled `NONSTANDARD_MACRO_BRACES` is a nursery (allow-by-default) lint, but `is_offending_macro` ran on every item/stmt/expr/ty unconditionally: walking the full macro backtrace (`outer_expn_data` clone + hygiene thread-local re-entry) and hashing macro names against the brace map, even though nothing could be emitted when the lint is disabled. Bail out early on two cheap checks. First, if the span isn't from a macro there is nothing to do (same as the original loop, but before any lint-level lookup so the common non-macro node stays free). Then, once we know the span came from a macro, skip everything when the lint is allowed at this node. Behaviour is unchanged when the lint is enabled. Same-binary A/B (instructions, N=12 medians) on macro-heavy code shows wasmi -8.3%; macro-light crates (tokio/syn/regex/ripgrep/serde) stay within noise. Diagnostics are byte-identical and the UI test is green.
xmakro
force-pushed
the
perf/nonstandard-macro-braces-guard
branch
from
June 1, 2026 15:42
f4e0c41 to
460f017
Compare
xmakro
marked this pull request as ready for review
June 1, 2026 15:58
rustbot
added
the
S-waiting-on-review
Collaborator
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @dswij (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
Why was this reviewer chosen?The reviewer was selected based on:
|
samueltardieu
added
the
G-performance-project
Member
|
Thanks for looking into it! ❤️ But this is thankfully Not necessary when we land #16808, and could cause more performance issues that it fixes. |
Contributor
Author
|
Closing in favor of #16808 |
rustbot
removed
the
S-waiting-on-review
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
NONSTANDARD_MACRO_BRACESis a nursery (allow-by-default) lint, butis_offending_macroran on every item, stmt, expr and ty regardless of whether the lint was enabled. For each node it walked the full macro backtrace (which clonesExpnDataand re-enters the hygiene thread-local once per level) and hashed macro names against the brace map. When the lint is disabled none of that can produce a warning, so it is all wasted.The fix adds two early returns at the top of
is_offending_macro:When the lint is enabled the second guard is never taken, so behaviour is unchanged.
Benchmarked with callgrind so the numbers are deterministic and machine-independent. Release
cargo-clippyrun undervalgrind --tool=callgrind --trace-children=yes, with dependencies pre-warmed so the measured run only re-lints the workspace crate. The figure is the summedclippy-driverinstruction count (Ir). Two drivers built against identical libs, differing only in this change:The saving is concentrated on macro-heavy code such as wasmi. On crates that use few macros it is small, which is what the guard is meant to do.
changelog: none