perf: cache the most recent MSRV attribute scan - #17139
Closed
xmakro wants to merge 2 commits into
Closed
Conversation
The early `MsrvStack` passes scan every node's attributes for `#[clippy::msrv]` on both enter and exit, but most nodes carry none. Returning early when the attribute slice is empty avoids building the filter iterator in that common case. changelog: none
The early `MsrvStack` passes are registered separately, so each node's attribute slice is scanned once per pass on both enter and exit. Those calls arrive back to back with the same slice, so a single-slot cache keyed on the slice pointer collapses them to one scan. changelog: none
samueltardieu
added
the
G-performance-project
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
2 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.
Summary
Follow-up to #17138 (skip MSRV attribute parsing for nodes without attributes). This PR is stacked on it, so the first commit is shared; if #17138 lands first, rebasing leaves only the cache commit here.
The five early
MsrvStackpasses are registered separately, so for each AST node the same attribute slice is scanned once per pass on both enter and exit. Those calls arrive back to back with the same slice. This adds a single-slot thread-local cache, keyed on the slice's pointer and length, so the repeated scans of one node collapse to a single parse. Only the early AST path goes through the cache, so it never mixesastandhirattributes.Lint output is unchanged.
Measurements
callgrind Ir (sum of clippy-driver Ir over a workspace re-lint), master vs the fast path alone (#17138) vs this PR, back to back in one session:
On top of the fast path the cache is roughly break-even: it helps wasmi, syn and regex a little and costs serde and ripgrep about as much, all within noise. I'm opening it for completeness on top of #17138, but it may not be worth the extra complexity over the fast path alone.
Diagnostics are byte-identical against master on serde, wasmi, ripgrep, regex, syn and tokio.
changelog: none