Skip to content

resolve: Do not create NameResolutions on access unless necessary #144468

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Jul 26, 2025
Merged

resolve: Do not create NameResolutions on access unless necessary #144468

merged 2 commits into from
Jul 26, 2025

Conversation

petrochenkov
Copy link
Contributor

fn resolution now just performs the access, and fn resolution_or_default will insert a default entry if the entry is missing.

@rustbot
Copy link
Collaborator

rustbot commented Jul 25, 2025

r? @SparrowLii

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 25, 2025
@petrochenkov
Copy link
Contributor Author

This is unlikely to affect performance often, but let's check.
@bors2 try @rust-timer queue

@rust-timer

This comment has been minimized.

Sign in to view
@rust-bors
Copy link

rust-bors bot commented Jul 25, 2025

⌛ Trying commit e820112 with merge 7773a3f…

To cancel the try build, run the command @bors try cancel.

rust-bors bot added a commit that referenced this pull request Jul 25, 2025
@rust-bors
Auto merge of #144468 - petrochenkov:resolution, r=<try>
7773a3f 
resolve: Do not create `NameResolutions` on access unless necessary
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 25, 2025
@rust-bors
Copy link

rust-bors bot commented Jul 25, 2025

☀️ Try build successful (CI)
Build commit: 7773a3f (7773a3f90eb3fad8c4a3afde0beea9f91c5adef1, parent: a955f1cd09a027363729ceed919952d09f76f28e)

@rust-timer

This comment has been minimized.

Sign in to view
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (7773a3f): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking 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
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -0.2% [-0.3%, -0.0%] 3
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (secondary 0.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 3.2% [3.2%, 3.2%] 1
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -1.8% [-1.8%, -1.8%] 1
All ❌✅ (primary) - - 0

Cycles

Results (primary 2.5%, secondary 2.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)		 2.5% [2.1%, 2.9%] 2
Regressions ❌
(secondary)		 2.1% [2.1%, 2.1%] 1
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) 2.5% [2.1%, 2.9%] 2

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 467.355s -> 468.125s (0.16%)
Artifact size: 374.67 MiB -> 374.69 MiB (0.00%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 26, 2025
lqd
lqd approved these changes Jul 26, 2025
View reviewed changes
Copy link
Member

@lqd lqd left a comment

Choose a reason for hiding this comment

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

The impact of the exclusive access, even if "unnecessary" is kind of subtle, but makes sense. r=me unless you want sparrow specifically to look at it.

SparrowLii
SparrowLii approved these changes Jul 26, 2025
View reviewed changes
Copy link
Member

@SparrowLii SparrowLii left a comment

Choose a reason for hiding this comment

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

Thanks!

@SparrowLii
Copy link
Member

@bors r=lqd, SparrowLii

@bors
Copy link
Collaborator

bors commented Jul 26, 2025

📌 Commit e820112 has been approved by lqd,

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 26, 2025
@petrochenkov
Copy link
Contributor Author

@bors rollup=maybe

@jieyouxu
Copy link
Member

(You can't have spaces between multiple handles)
@bors r=lqd,SparrowLii

@bors
Copy link
Collaborator

bors commented Jul 26, 2025

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Collaborator

bors commented Jul 26, 2025

📌 Commit e820112 has been approved by lqd,SparrowLii

It is now in the queue for this repository.

@matthiaskrgr matthiaskrgr mentioned this pull request Jul 26, 2025
Merged
bors added a commit that referenced this pull request Jul 26, 2025
@bors
Auto merge of #144502 - matthiaskrgr:rollup-o81uk9m, r=matthiaskrgr
133798f 
Rollup of 10 pull requests

Successful merges:

 - #144331 (Disable non_exhaustive_omitted_patterns within matches! macro)
 - #144376 (Suggest unwrapping when private method name is available in inner type)
 - #144421 (Call `is_parsed_attribute` rather than keeping track of a list of parsed attributes manually)
 - #144424 (Allow setting `release-blog-post` label with rustbot)
 - #144427 (rename ext_tool_checks to extra_checks and use mod.rs)
 - #144435 (rustc-dev-guide subtree update)
 - #144448 (Limit defaultness query to impl of trait)
 - #144462 (Allow pretty printing paths with `-Zself-profile-events=args`)
 - #144463 (change_tracker: fix a typo)
 - #144468 (resolve: Do not create `NameResolutions` on access unless necessary)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 429cc9a into rust-lang:master Jul 26, 2025
11 checks passed
@rustbot rustbot added this to the 1.90.0 milestone Jul 26, 2025
rust-timer added a commit that referenced this pull request Jul 26, 2025
@rust-timer
Unrolled build for #144468
d0eae10 
Rollup merge of #144468 - petrochenkov:resolution, r=lqd,SparrowLii

resolve: Do not create `NameResolutions` on access unless necessary

`fn resolution` now just performs the access, and `fn resolution_or_default` will insert a default entry if the entry is missing.
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Jul 28, 2025
@bors
Auto merge of #144502 - matthiaskrgr:rollup-o81uk9m, r=matthiaskrgr
8f5abb7 
Rollup of 10 pull requests

Successful merges:

 - rust-lang/rust#144331 (Disable non_exhaustive_omitted_patterns within matches! macro)
 - rust-lang/rust#144376 (Suggest unwrapping when private method name is available in inner type)
 - rust-lang/rust#144421 (Call `is_parsed_attribute` rather than keeping track of a list of parsed attributes manually)
 - rust-lang/rust#144424 (Allow setting `release-blog-post` label with rustbot)
 - rust-lang/rust#144427 (rename ext_tool_checks to extra_checks and use mod.rs)
 - rust-lang/rust#144435 (rustc-dev-guide subtree update)
 - rust-lang/rust#144448 (Limit defaultness query to impl of trait)
 - rust-lang/rust#144462 (Allow pretty printing paths with `-Zself-profile-events=args`)
 - rust-lang/rust#144463 (change_tracker: fix a typo)
 - rust-lang/rust#144468 (resolve: Do not create `NameResolutions` on access unless necessary)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lqd lqd lqd approved these changes

@SparrowLii SparrowLii SparrowLii approved these changes

Assignees

@SparrowLii SparrowLii

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.90.0
Development

Successfully merging this pull request may close these issues.
7 participants
@petrochenkov @rustbot @rust-timer @SparrowLii @bors @jieyouxu @lqd