Skip to content

resolve: Use interior mutability for extern module map #143550

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 4 commits into from
Jul 17, 2025
Merged

resolve: Use interior mutability for extern module map #143550

merged 4 commits into from
Jul 17, 2025
+140 −129

Conversation

petrochenkov
Copy link
Contributor

@petrochenkov petrochenkov commented Jul 6, 2025 •
edited
Loading

Module map for extern modules is a lazily populated cache, it's not significantly mutable.
If some logic in name resolver is parallelized, then this cache can be populated from any thread, and without affecting results of any speculative resolution.

Unblocks #143884.
This is a part of #gsoc > Project: Parallel Macro Expansion.
cc @LorrensP-2158466

@rustbot
Copy link
Collaborator

rustbot commented Jul 6, 2025

r? @lcnr

rustbot has assigned @lcnr.
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 6, 2025
@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 6, 2025
@lcnr
Copy link
Contributor

lcnr commented Jul 7, 2025

d540290, 3ed7e6f, 27bb8f1, 92dea43

lgtm, r=me after #143458

@rustbot blocked

@rustbot rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 7, 2025
@LorrensP-2158466 LorrensP-2158466 mentioned this pull request Jul 13, 2025
Open
@bors

This comment was marked as resolved.

Sign in to view
@petrochenkov
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

Sign in to view
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 16, 2025
@bors
Copy link
Collaborator

bors commented Jul 16, 2025

⌛ Trying commit 11498e6 with merge b91aa9a...

bors added a commit that referenced this pull request Jul 16, 2025
@bors
Auto merge of #143550 - petrochenkov:lessmutres, r=<try>
b91aa9a 
[WIP] resolve: Use interior mutability for extern module map

Based on #143458.

Module map for extern modules is a lazily populated cache, it's not *significantly* mutable.
If some logic in name resolver is parallelized, then this cache can be populated from any thread, and without affecting results of any speculative resolution.

cc `@LorrensP-2158466`
@bors
Copy link
Collaborator

bors commented Jul 16, 2025

☀️ Try build successful - checks-actions
Build commit: b91aa9a (b91aa9a16e716abbba82b400143c6e129888ca01)

@rust-timer

This comment has been minimized.

Sign in to view
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (b91aa9a): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

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.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@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.4% [0.1%, 0.7%] 3
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -0.2% [-0.4%, -0.1%] 12
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary 0.9%, secondary 0.6%)

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

mean range count
Regressions ❌
(primary)		 1.9% [1.4%, 2.2%] 3
Regressions ❌
(secondary)		 4.3% [4.3%, 4.3%] 1
Improvements ✅
(primary)		 -2.2% [-2.2%, -2.2%] 1
Improvements ✅
(secondary)		 -3.1% [-3.1%, -3.1%] 1
All ❌✅ (primary) 0.9% [-2.2%, 2.2%] 4

Cycles

Results (secondary 3.4%)

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.4% [3.4%, 3.4%] 1
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 464.891s -> 464.924s (0.01%)
Artifact size: 374.78 MiB -> 374.80 MiB (0.01%)

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jul 16, 2025
@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Jul 16, 2025
@petrochenkov petrochenkov changed the title [WIP] resolve: Use interior mutability for extern module map resolve: Use interior mutability for extern module map Jul 16, 2025
@petrochenkov
Copy link
Contributor Author

@bors r=lcnr rollup=maybe

@bors
Copy link
Collaborator

bors commented Jul 16, 2025

📌 Commit 6849f81 has been approved by lcnr

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 16, 2025
@matthiaskrgr matthiaskrgr mentioned this pull request Jul 17, 2025
Merged
bors added a commit that referenced this pull request Jul 17, 2025
@bors
Auto merge of #144058 - matthiaskrgr:rollup-xezozsk, r=matthiaskrgr
bf5e6cc 
Rollup of 11 pull requests

Successful merges:

 - #143326 (Remove deprecated `Error::description` impl from `c_str::FromBytesWithNulError`)
 - #143431 (Use relative visibility when noting sealed trait to reduce false positive)
 - #143550 (resolve: Use interior mutability for extern module map)
 - #143631 (update to literal-escaper-0.0.5)
 - #143793 (Opaque type collection: Guard against endlessly recursing free alias types)
 - #143880 (tests: Test line debuginfo for linebreaked function parameters)
 - #143914 (Reword mismatched-lifetime-syntaxes text based on feedback)
 - #143926 (Remove deprecated fields in bootstrap)
 - #143955 (Make frame spans appear on a separate trace line)
 - #143975 (type_id_eq: check that the hash fully matches the type)
 - #143984 (Fix ice for feature-gated `cfg` attributes applied to the crate)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 52a405b into rust-lang:master Jul 17, 2025
11 checks passed
@rustbot rustbot added this to the 1.90.0 milestone Jul 17, 2025
rust-timer added a commit that referenced this pull request Jul 17, 2025
@rust-timer
Unrolled build for #143550
240c39a 
Rollup merge of #143550 - petrochenkov:lessmutres, r=lcnr

resolve: Use interior mutability for extern module map

Module map for extern modules is a lazily populated cache, it's not *significantly* mutable.
If some logic in name resolver is parallelized, then this cache can be populated from any thread, and without affecting results of any speculative resolution.

Unblocks #143884.
This is a part of [#gsoc > Project: Parallel Macro Expansion](https://rust-lang.zulipchat.com/#narrow/channel/421156-gsoc/topic/Project.3A.20Parallel.20Macro.20Expansion/with/527348747).
cc `@LorrensP-2158466`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@lcnr lcnr

Labels
perf-regression Performance regression. 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.
5 participants
@petrochenkov @rustbot @lcnr @bors @rust-timer