Skip to content

Optimize allow_unwrap_types evaluation to eliminate performance regression#16652

Merged
blyxyas merged 1 commit intorust-lang:masterfrom
sengmonkham:optimize_allow_unwrap_types
Mar 3, 2026
Merged

Optimize allow_unwrap_types evaluation to eliminate performance regression#16652
blyxyas merged 1 commit intorust-lang:masterfrom
sengmonkham:optimize_allow_unwrap_types

Conversation

@sengmonkham
Copy link
Copy Markdown
Contributor

@sengmonkham sengmonkham commented Feb 28, 2026 •
edited
Loading

Fixes #16605

This PR addresses the 6.2% compilation performance regression introduced by the original allow_unwrap_types implementation (which natively allocated AST strings on every unwrap call via bumpalo).

The implementation has been refactored to pre-resolve types securely without a performance penalty:

Pre-Resolution Cache via LateLintPass: Instead of executing ty.to_string() and lookup_path_str inside the immediate unwrap_expect_used hot check for every method call encountered, allow-unwrap-types configuration strings are now parsed exactly once per crate compilation during the LateLintPass::check_crate hook in clippy_lints/src/methods/mod.rs.
DefId Native Storage: The parsed DefId representations are stored in-memory using highly efficient caching maps directly on the main Methods struct:

  • unwrap_allowed_ids: FxHashSet<DefId>: Allows instant O(1) lookups for standard types.
  • unwrap_allowed_aliases: Vec<DefId>: Tracks type aliases requiring signature substitution checking later.
    Execution Relief: Inside unwrap_expect_used::check(), string manipulation is completely removed and ty::Adt checking uses lightning-fast .contains(&adt.did()) operations to categorically avoid regression allocation loops. This implementation strongly parallels the pre-resolution type-routing logic in clippy_config::types::create_disallowed_map without altering the core clippy.toml config requirements.

Summary Notes

Managed by @rustbot—see help for details

changelog: none

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Feb 28, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Feb 28, 2026

r? @Jarcho

rustbot has assigned @Jarcho.
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

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Feb 28, 2026
@sengmonkham sengmonkham reopened this Feb 28, 2026
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Feb 28, 2026
@sengmonkham
Copy link
Copy Markdown
Contributor Author

sengmonkham commented Feb 28, 2026

@blyxyas

@samueltardieu
Copy link
Copy Markdown
Member

samueltardieu commented Feb 28, 2026 •
edited
Loading

@rustbot label beta-nominated
@rustbot note Beta-nomination

This might need to be beta-nominated as #16605 has been synced to the Rust repository already, and the beta branch will be cut before the next sync happens.

@blyxyas
Copy link
Copy Markdown
Member

blyxyas commented Mar 1, 2026

Sorry for the late reply, I was (and sm) at a hackathon.

From my profiling in bumpalo, the performance regression is completely gone. So I'll give this a +1 if there isn't anything else to bring up.

Also we should totally backport this to beta.

@rustbot rustbot added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Mar 1, 2026
@blyxyas
Copy link
Copy Markdown
Member

blyxyas commented Mar 3, 2026

@sengmonkham Seems that CI is failing, could you look into that?

@sengmonkham
Copy link
Copy Markdown
Contributor Author

sengmonkham commented Mar 3, 2026

@sengmonkham Seems that CI is failing, could you look into that?

I have fixed all CI issues, and all checks now pass.

@samueltardieu
Copy link
Copy Markdown
Member

samueltardieu commented Mar 3, 2026

Could you please squash your commits? It would make it easier to backport to beta if we do so.

@sengmonkham sengmonkham force-pushed the optimize_allow_unwrap_types branch from d2a4fb1 to 7ac8be4 Compare March 3, 2026 15:23
blyxyas
blyxyas approved these changes Mar 3, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@blyxyas blyxyas left a comment •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

LGTM, thanks! ❤️ The regression is fully fixed in my profiling.

View changes since this review

@blyxyas blyxyas added this pull request to the merge queue Mar 3, 2026
Merged via the queue into rust-lang:master with commit 3120b98 Mar 3, 2026
11 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 3, 2026
@flip1995 flip1995 added beta-accepted Accepted for backporting to the compiler in the beta channel. and removed beta-nominated Nominated for backporting to the compiler in the beta channel. labels Apr 9, 2026
@flip1995 flip1995 mentioned this pull request Apr 9, 2026
Closed
@flip1995
Copy link
Copy Markdown
Member

flip1995 commented Apr 9, 2026

rust-lang/rust#155051

cuviper pushed a commit to cuviper/rust that referenced this pull request Apr 9, 2026
@blyxyas @cuviper
Optimize allow_unwrap_types evaluation to eliminate performance re…
b6da6b8 
…gression (rust-lang#16652)

Fixes rust-lang/rust-clippy#16605

This PR addresses the 6.2% compilation performance regression introduced
by the original `allow_unwrap_types` implementation (which natively
allocated AST strings on every `unwrap` call via `bumpalo`).

The implementation has been refactored to pre-resolve types securely
without a performance penalty:

**Pre-Resolution Cache via `LateLintPass`**: Instead of executing
`ty.to_string()` and `lookup_path_str` inside the immediate
`unwrap_expect_used` hot check for every method call encountered,
`allow-unwrap-types` configuration strings are now parsed exactly *once*
per crate compilation during the `LateLintPass::check_crate` hook in
`clippy_lints/src/methods/mod.rs`.
**`DefId` Native Storage**: The parsed `DefId` representations are
stored in-memory using highly efficient caching maps directly on the
main `Methods` struct:
- `unwrap_allowed_ids: FxHashSet<DefId>`: Allows instant O(1) lookups
for standard types.
- `unwrap_allowed_aliases: Vec<DefId>`: Tracks type aliases requiring
signature substitution checking later.
**Execution Relief**: Inside `unwrap_expect_used::check()`, string
manipulation is completely removed and `ty::Adt` checking uses
lightning-fast `.contains(&adt.did())` operations to categorically avoid
regression allocation loops. This implementation strongly parallels the
pre-resolution type-routing logic in
`clippy_config::types::create_disallowed_map` without altering the core
`clippy.toml` config requirements.

<!-- TRIAGEBOT_START -->

<!-- TRIAGEBOT_SUMMARY_START -->

### Summary Notes

-
[Beta-nomination](rust-lang/rust-clippy#16652 (comment))
by [samueltardieu](https://github.com/samueltardieu)

*Managed by `@rustbot`—see
[help](https://forge.rust-lang.org/triagebot/note.html) for details*

<!-- TRIAGEBOT_SUMMARY_END -->
<!-- TRIAGEBOT_END -->
changelog: none
@cuviper cuviper mentioned this pull request Apr 9, 2026
Merged
rust-bors bot pushed a commit to rust-lang/rust that referenced this pull request Apr 9, 2026
@bors
Auto merge of #154999 - cuviper:beta-next, r=cuviper
2c8c85d 
[beta] reverts and backports

This reverts two `dbg!` changes to avoid regressions[^1][^2] in the upcoming 1.95 release:

- std: avoid tearing `dbg!` prints #149869
- don't drop arguments' temporaries in `dbg!` #154074
  - ... which was previously backported in #154725

This also reverts a stabilization over a late issue[^3] of semantics:

- Stabilize `assert_matches` #137487

And a few other backport/reverts from `main`:

- Revert performing basic const checks in typeck on stable #154930 / #155033
- Revert "`-Znext-solver` Remove the forced ambiguity hack from search graph" #154712
- Clarify that core::range ranges do not have special syntax #155002

Clippy is backporting 2 ICE fixes and 1 perf regression (via #155051):

- rust-lang/rust-clippy#16685 already backported in #154211 to stable. This makes sure that it doesn't regress again in beta/next stable
- rust-lang/rust-clippy#16659 The ICE that is being fixed here was introduced in the 1.95 release cycle
- rust-lang/rust-clippy#16652 Perf regression introduced in the 1.95 release cycle.
  
[^1]: #153850
[^2]: #154988
[^3]: #154406
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@blyxyas blyxyas blyxyas approved these changes

Assignees

@Jarcho Jarcho

Labels

beta-accepted Accepted for backporting to the compiler in the beta channel.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@sengmonkham @rustbot @samueltardieu @blyxyas @flip1995 @Jarcho