rustc/rustdoc: Perform name resolver cleanups enabled by #94857 #107765

petrochenkov · 2023-02-07T15:14:53Z

rustbot · 2023-02-07T15:15:05Z

Some changes occurred in src/librustdoc/clean/types.rs

These commits modify the Cargo.lock file. Random changes to Cargo.lock can be introduced when switching branches and rebasing PRs.
This was probably unintentional and should be reverted before this PR is merged.

If this was intentional then you can ignore this comment.

compiler/rustc_resolve/src/rustdoc.rs

petrochenkov · 2023-02-11T16:06:59Z

@rustbot ready

And remove `Clone` impls and `Lrc`s that are no longer necessary

oli-obk · 2023-02-13T10:52:21Z

@bors try @rust-timer queue

bors · 2023-02-13T10:52:30Z

⌛ Trying commit fd73d01 with merge 6035315a51746de277112909a1da649ef4e0d40b...

bors · 2023-02-13T13:41:33Z

☀️ Try build successful - checks-actions
Build commit: 6035315a51746de277112909a1da649ef4e0d40b (6035315a51746de277112909a1da649ef4e0d40b)

rust-timer · 2023-02-13T16:13:42Z

Finished benchmarking commit (6035315a51746de277112909a1da649ef4e0d40b): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

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)	13.4%	[13.4%, 13.4%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.4%	[-0.4%, -0.3%]	3
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.1%	[-0.4%, 13.4%]	4

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)	0.4%	[0.4%, 0.4%]	1
Regressions ❌ (secondary)	1.4%	[1.4%, 1.4%]	1
Improvements ✅ (primary)	-4.1%	[-4.5%, -3.7%]	6
Improvements ✅ (secondary)	-2.5%	[-4.7%, -0.9%]	37
All ❌✅ (primary)	-3.4%	[-4.5%, 0.4%]	7

Cycles

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)	8.7%	[8.7%, 8.7%]	1
Regressions ❌ (secondary)	7.5%	[5.2%, 10.4%]	6
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	8.7%	[8.7%, 8.7%]	1

oli-obk · 2023-02-13T16:48:02Z

The "regression" is in the bitmaps crate. I say "regression" because it's actually more things getting documented. I haven't looked into the details, but I think it now just got more data from dependencies to include in the docs

@bors r+ rollup=never

bors · 2023-02-13T16:48:04Z

📌 Commit fd73d01 has been approved by oli-obk

It is now in the queue for this repository.

petrochenkov · 2023-02-13T18:33:14Z

it's actually more things getting documented ... I think it now just got more data from dependencies to include in the docs

It's not something that should happen with this PR.
I'll try to investigate the bitmaps case some time later this week.
It's especially interesting that bitmaps is the only regression and other crates are not affected.

bors · 2023-02-14T05:59:48Z

⌛ Testing commit fd73d01 with merge e9ab787...

bors · 2023-02-14T09:30:19Z

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing e9ab787 to master...

rust-timer · 2023-02-14T10:47:52Z

Finished benchmarking commit (e9ab787): comparison URL.

Overall result: ❌✅ regressions and improvements - 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)	13.4%	[13.4%, 13.4%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.4%	[-0.4%, -0.4%]	3
Improvements ✅ (secondary)	-0.2%	[-0.2%, -0.2%]	1
All ❌✅ (primary)	3.1%	[-0.4%, 13.4%]	4

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

Cycles

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)	9.1%	[9.1%, 9.1%]	1
Regressions ❌ (secondary)	3.0%	[3.0%, 3.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	9.1%	[9.1%, 9.1%]	1

petrochenkov · 2023-03-20T15:57:38Z

The regression should be fixed by #109399.

…Gomez rustdoc: Optimize impl sorting during rendering This should fix the perf regression on [bitmaps-3.1.0](https://github.com/rust-lang/rustc-perf/tree/master/collector/compile-benchmarks/bitmaps-3.1.0) from rust-lang#107765. The bitmaps crate has a lot of impls: ```rust impl Bits for BitsImpl<1> { ... } impl Bits for BitsImpl<2> { ... } // ... impl Bits for BitsImpl<1023> { ... } impl Bits for BitsImpl<1024> { ... } ``` and the logic in `fn print_item` sorts them in natural order. Before rust-lang#107765 the impls came in source order, which happened to be already sorted in the necessary way. So the comparison function was called fewer times. After rust-lang#107765 the impls came in "stable" order (based on def path hash). So the comparison function was called more times to sort them. The comparison function was terribly inefficient, so it caused a large perf regression. This PR attempts to make it more efficient by using cached keys during sorting.

rustdoc: Optimize impl sorting during rendering This should fix the perf regression on [bitmaps-3.1.0](https://github.com/rust-lang/rustc-perf/tree/master/collector/compile-benchmarks/bitmaps-3.1.0) from rust-lang/rust#107765. The bitmaps crate has a lot of impls: ```rust impl Bits for BitsImpl<1> { ... } impl Bits for BitsImpl<2> { ... } // ... impl Bits for BitsImpl<1023> { ... } impl Bits for BitsImpl<1024> { ... } ``` and the logic in `fn print_item` sorts them in natural order. Before rust-lang/rust#107765 the impls came in source order, which happened to be already sorted in the necessary way. So the comparison function was called fewer times. After rust-lang/rust#107765 the impls came in "stable" order (based on def path hash). So the comparison function was called more times to sort them. The comparison function was terribly inefficient, so it caused a large perf regression. This PR attempts to make it more efficient by using cached keys during sorting.

…ulacrum cstore: Remove unnecessary locking from `CrateMetadata` Locks and atomics in `CrateMetadata` fields were necessary before rust-lang#107765 when `CStore` was cloneable, but now they are not necessary and can be removed after restructuring the code a bit to please borrow checker. All remaining locked fields in `CrateMetadata` are lazily populated caches.

Rollup merge of rust-lang#119589 - petrochenkov:cdatalock, r=Mark-Simulacrum cstore: Remove unnecessary locking from `CrateMetadata` Locks and atomics in `CrateMetadata` fields were necessary before rust-lang#107765 when `CStore` was cloneable, but now they are not necessary and can be removed after restructuring the code a bit to please borrow checker. All remaining locked fields in `CrateMetadata` are lazily populated caches.

rustbot assigned oli-obk Feb 7, 2023

kadiwa4 reviewed Feb 7, 2023

View reviewed changes

compiler/rustc_resolve/src/rustdoc.rs Outdated Show resolved Hide resolved

petrochenkov added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 7, 2023

This comment was marked as resolved.

Sign in to view

petrochenkov force-pushed the nomoclone branch from 00dc400 to 02faf83 Compare February 11, 2023 16:06

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Feb 11, 2023

This comment was marked as resolved.

Sign in to view

petrochenkov added 2 commits February 13, 2023 00:10

rustdoc: Eliminate remaining uses of resolver

9080b79

rustc_resolve: Remove Resolver::clone_output

fd73d01

And remove `Clone` impls and `Lrc`s that are no longer necessary

petrochenkov force-pushed the nomoclone branch from 02faf83 to fd73d01 Compare February 12, 2023 20:14

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 13, 2023

This comment has been minimized.

Sign in to view

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Feb 13, 2023

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 Feb 13, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 14, 2023

bors merged commit e9ab787 into rust-lang:master Feb 14, 2023

rustbot added this to the 1.69.0 milestone Feb 14, 2023

bors mentioned this pull request Feb 14, 2023

Make traits_in_crate and impls_in_crate proper queries #100601

Closed

4 tasks

petrochenkov mentioned this pull request Feb 14, 2023

give the resolver access to TyCtxt #105462

Merged

petrochenkov mentioned this pull request Mar 20, 2023

rustdoc: Optimize impl sorting during rendering #109399

Merged

petrochenkov mentioned this pull request Jan 4, 2024

cstore: Remove unnecessary locking from CrateMetadata #119589

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustc/rustdoc: Perform name resolver cleanups enabled by #94857 #107765

rustc/rustdoc: Perform name resolver cleanups enabled by #94857 #107765

petrochenkov commented Feb 7, 2023 •

edited

Loading

rustbot commented Feb 7, 2023

This comment was marked as resolved.

petrochenkov commented Feb 11, 2023

This comment was marked as resolved.

oli-obk commented Feb 13, 2023

This comment has been minimized.

bors commented Feb 13, 2023

bors commented Feb 13, 2023

This comment has been minimized.

rust-timer commented Feb 13, 2023

oli-obk commented Feb 13, 2023

bors commented Feb 13, 2023

petrochenkov commented Feb 13, 2023

bors commented Feb 14, 2023

bors commented Feb 14, 2023

rust-timer commented Feb 14, 2023

petrochenkov commented Mar 20, 2023

rustc/rustdoc: Perform name resolver cleanups enabled by #94857 #107765

rustc/rustdoc: Perform name resolver cleanups enabled by #94857 #107765

Conversation

petrochenkov commented Feb 7, 2023 • edited Loading

rustbot commented Feb 7, 2023

This comment was marked as resolved.

petrochenkov commented Feb 11, 2023

This comment was marked as resolved.

oli-obk commented Feb 13, 2023

This comment has been minimized.

bors commented Feb 13, 2023

bors commented Feb 13, 2023

This comment has been minimized.

rust-timer commented Feb 13, 2023

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Instruction count

Max RSS (memory usage)

Cycles

oli-obk commented Feb 13, 2023

bors commented Feb 13, 2023

petrochenkov commented Feb 13, 2023

bors commented Feb 14, 2023

bors commented Feb 14, 2023

rust-timer commented Feb 14, 2023

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Instruction count

Max RSS (memory usage)

Cycles

petrochenkov commented Mar 20, 2023

petrochenkov commented Feb 7, 2023 •

edited

Loading