give the resolver access to TyCtxt #105462

oli-obk · 2022-12-08T16:27:11Z

The resolver is now created after TyCtxt is created. Then macro expansion and name resolution are run and the results fed into queries just like before this PR.

Since the resolver had (before this PR) mutable access to the CStore and the source span table, these two datastructures are now behind a RwLock. To ensure that these are not mutated anymore after the resolver is done, a read lock to them is leaked right after the resolver finishes.

PRs split out of this one and leading up to it:

bors · 2022-12-08T16:27:36Z

⌛ Trying commit 9b27fe51e70e1ef01326b1b1ffd1e399f51dea39 with merge 150b43a8f421dbd33945aa017aafcb12a093bbcb...

jyn514

this is amazing ❤️

compiler/rustc_interface/src/queries.rs

src/librustdoc/core.rs

…ation if it fails instead of bubbling out an error

oli-obk · 2023-02-20T15:29:09Z

@bors r=cjgillot,petrochenkov

bors · 2023-02-20T15:29:11Z

📌 Commit 0847b79 has been approved by cjgillot,petrochenkov

It is now in the queue for this repository.

bors · 2023-02-21T01:19:29Z

⌛ Testing commit 0847b79 with merge 2deff71...

bors · 2023-02-21T04:22:16Z

☀️ Test successful - checks-actions
Approved by: cjgillot,petrochenkov
Pushing 2deff71 to master...

rust-timer · 2023-02-21T06:51:42Z

Finished benchmarking commit (2deff71): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.4%	[0.2%, 1.4%]	94
Regressions ❌ (secondary)	0.6%	[0.2%, 2.6%]	25
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.4%	[0.2%, 1.4%]	94

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.2%	[0.8%, 1.6%]	3
Regressions ❌ (secondary)	2.9%	[1.2%, 5.5%]	12
Improvements ✅ (primary)	-1.5%	[-1.5%, -1.4%]	2
Improvements ✅ (secondary)	-2.3%	[-3.5%, -1.4%]	5
All ❌✅ (primary)	0.1%	[-1.5%, 1.6%]	5

Cycles

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

oli-obk · 2023-02-21T07:52:44Z

the 2.6% regression (deep-vector opt) is noise (it shows LLVM changes, which this PR can't cause in opt builds).

The other regressions are expected due to the additional locking, I am already working on removing some of the locking, but we decided to land this PR regardless as

it opens up using queries in the resolver, getting us better caching and thus incremental wins
we're actively working on reducing the locking, but that seems an orthogonal change that should not be part of this refactoring.

Use a lock-free datastructure for source_span follow up to the perf regression in rust-lang#105462 The main regression is likely the CStore, but let's evaluate the perf impact of this on its own

the8472 · 2023-02-23T16:09:35Z

Note that, compared to instructions, this lead to a much bigger (real) wall-time regression in incremental builds. So you might want to keep an eye on that in PRs that try to recover performance.

oli-obk · 2023-02-23T16:23:25Z

Ah! that's where the failed parallelism for dep-graph loading disappeared to. Will look at wall times in the follow up PRs.

rustc/rustdoc: Perform name resolver cleanups enabled by #94857 Unblocks rust-lang/rust#105462. r? `@oli-obk`

This comment was marked as resolved.

Sign in to view