Skip to content

Optimize nonstandard_macro_braces by 99.9683% [1.1b -> 351K] - #16808

Merged
ada4a merged 1 commit into
rust-lang:masterfrom
blyxyas:perf-nonstandard_macro_braces
Jul 10, 2026
Merged

Optimize nonstandard_macro_braces by 99.9683% [1.1b -> 351K]#16808
ada4a merged 1 commit into
rust-lang:masterfrom
blyxyas:perf-nonstandard_macro_braces

Conversation

@blyxyas

@blyxyas blyxyas commented Apr 4, 2026

Copy link
Copy Markdown
Member

View all comments

I know that we shouldn't move lints to pre-expansion, but nonstandard_macro_braces was taking up almost a 25% of the Clippy-exclusive runtime in wasmi-0.35.0, and moving it to pre-expansion lets us not bother as much with expansion data.


Previously, we had to get expansion data via outer_expn_data, which does atomic operations, SessionGlobals operations (so, locking threads), and was generally a very bumpy way of doing things. For example in wasmi we were calling scoped_tls::ScopedKey<rustc_span::SessionGlobals>::with<<rustc_span::hygiene::HygieneData>::with<rustc_span::hygiene::ExpnData, <rustc_span::hygiene::SyntaxContext>::outer_expn_data::{closure#0}>::{closure#0}, rustc_span::hygiene::ExpnData> 5,879,804 times. Thus locking the thread 11,759,608 times.


So personally I think that moving this lint to pre-expansion is worth.

changelog:[nonstandard_macro_braces]: Optimize by 99.97%

@blyxyas blyxyas added the G-performance-project Goal: For issues and PRs related to the Clippy Performance Project label Apr 4, 2026
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 4, 2026
@rustbot

rustbot commented Apr 4, 2026

Copy link
Copy Markdown
Collaborator

r? @dswij

rustbot has assigned @dswij.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: 7 candidates
  • 7 candidates expanded to 7 candidates
  • Random selection from Jarcho, dswij, llogiq, samueltardieu

@github-actions

github-actions Bot commented Apr 4, 2026

Copy link
Copy Markdown

Lintcheck changes for 5f34b6d

Lint Added Removed Changed
clippy::nonstandard_macro_braces 0 2 1

This comment will be updated if you push new changes

@blyxyas

blyxyas commented Apr 4, 2026

Copy link
Copy Markdown
Member Author

I didn't notice those lintcheck differences, I'll fix them

Comment thread clippy_lints/src/nonstandard_macro_braces.rs
@rustbot

This comment has been minimized.

@xmakro

xmakro commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Very nice optimization. Is the intention to change this lint from nursery to correctness?

#17125 only recovers the performance if the lint is disabled. But @blyxyas PR also keeps the performance if this lint is turned on by default.

@blyxyas
blyxyas force-pushed the perf-nonstandard_macro_braces branch from efddb12 to 4a457cc Compare June 3, 2026 08:59
@rustbot

This comment has been minimized.

@ada4a ada4a left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work! Left some comments, mostly stylistic

Let me take over the review if that's okay

r? @ada4a

View changes since this review

Comment thread clippy_lints/src/nonstandard_macro_braces.rs Outdated
Comment thread clippy_lints/src/nonstandard_macro_braces.rs Outdated
Comment thread clippy_lints/src/nonstandard_macro_braces.rs Outdated
Comment thread clippy_lints/src/nonstandard_macro_braces.rs Outdated
Comment thread clippy_lints/src/nonstandard_macro_braces.rs Outdated
Comment thread clippy_lints/src/nonstandard_macro_braces.rs
Comment thread clippy_lints/src/nonstandard_macro_braces.rs Outdated
@rustbot rustbot assigned ada4a and unassigned dswij Jun 6, 2026
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Jun 6, 2026
@rustbot

This comment has been minimized.

@blyxyas
blyxyas force-pushed the perf-nonstandard_macro_braces branch from 61386b9 to 54496c9 Compare June 14, 2026 17:05
@rustbot

This comment has been minimized.

Comment thread clippy_lints/src/nonstandard_macro_braces.rs Outdated
Comment thread clippy_lints/src/nonstandard_macro_braces.rs Outdated
Comment thread clippy_lints/src/nonstandard_macro_braces.rs
@blyxyas
blyxyas force-pushed the perf-nonstandard_macro_braces branch from 3631298 to a7406b6 Compare June 15, 2026 09:17
@rustbot

This comment has been minimized.

@blyxyas
blyxyas force-pushed the perf-nonstandard_macro_braces branch from a7406b6 to 86c00a2 Compare June 25, 2026 20:16
@rustbot

This comment has been minimized.

@rustbot

This comment has been minimized.

@blyxyas
blyxyas force-pushed the perf-nonstandard_macro_braces branch from d8d7901 to 489bf44 Compare June 29, 2026 16:10
@rustbot

rustbot commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@ada4a ada4a left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some last naming nits..

View changes since this review

Comment thread clippy_lints/src/nonstandard_macro_braces.rs Outdated
Comment thread clippy_lints/src/nonstandard_macro_braces.rs Outdated
Comment thread clippy_lints/src/nonstandard_macro_braces.rs Outdated
Comment thread clippy_lints/src/nonstandard_macro_braces.rs Outdated
@rustbot

This comment has been minimized.

@rust-cloud-vms
rust-cloud-vms Bot force-pushed the perf-nonstandard_macro_braces branch from 0fdca09 to c00ffe5 Compare July 9, 2026 23:31

@ada4a ada4a left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking great now, thank you for your patience with me:) Could you please squash everything down?

View changes since this review

mod unnecessary_clippy_cfg;
mod useless_attribute;
mod utils;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks spurious.. could you please remove this change?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, I've also squashed

@rust-cloud-vms
rust-cloud-vms Bot force-pushed the perf-nonstandard_macro_braces branch from b3b24d4 to 5f34b6d Compare July 10, 2026 13:40
@ada4a
ada4a added this pull request to the merge queue Jul 10, 2026
Merged via the queue into rust-lang:master with commit e5251c9 Jul 10, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

G-performance-project Goal: For issues and PRs related to the Clippy Performance Project S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants