rustdoc: Only build extern trait impls if needed

[camelid]

View all comments

or, finally remove the BadImplStripper!

Building inlined impls is expensive, and most of them end up being
unneeded and stripped later in this function. So we should filter them
ahead of time. This requires inlining external auto traits when we
construct auto impls, since this no longer happens as a side effect.

We inline external impls when they are

• for generics (i.e., blanket impls)
• for primitive types (probably this should be handled below really)
• for a type (inlined) in the current crate
• of a trait (inlined) in the current crate
• of Deref
  These rules are based on the existing filtering rules that are applied
  after building the inlined impls.

After doing the filtering ahead of time, we can remove the downstream
filtering. Thus, we can finally remove BadImplStripper and unused
deref-following logic. It appears that all this Deref logic was pointless
in the first place since following derefs already happens elsewhere in
rustdoc, where it is actually needed.

[camelid]

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: 2706e41 (2706e41b9187e2caefe5e7291b7697fb2c00a3a3)
Base parent: d527bc9 (d527bc9bfa297ca7fd7f5ae93781eeec42073170)

[rust-timer]

Finished benchmarking commit (2706e41): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

@bors rollup=never rustc-perf
@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)	2.3%	[2.3%, 2.3%]	1
Regressions ❌ (secondary)	2.3%	[2.3%, 2.3%]	1
Improvements ✅ (primary)	-12.4%	[-37.6%, -0.5%]	19
Improvements ✅ (secondary)	-24.7%	[-38.9%, -4.9%]	24
All ❌✅ (primary)	-11.6%	[-37.6%, 2.3%]	20

Max RSS (memory usage)

Results (primary -12.4%, secondary -16.5%)

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

	mean	range	count
Regressions ❌ (primary)	1.7%	[1.7%, 1.7%]	1
Regressions ❌ (secondary)	1.8%	[1.8%, 1.8%]	1
Improvements ✅ (primary)	-13.1%	[-19.9%, -4.3%]	19
Improvements ✅ (secondary)	-17.3%	[-22.9%, -6.0%]	24
All ❌✅ (primary)	-12.4%	[-19.9%, 1.7%]	20

Cycles

Results (primary -16.3%, secondary -31.3%)

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)	-	-	0
Improvements ✅ (primary)	-16.3%	[-43.4%, -2.6%]	20
Improvements ✅ (secondary)	-31.3%	[-44.5%, -6.1%]	24
All ❌✅ (primary)	-16.3%	[-43.4%, -2.6%]	20

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 489.235s -> 486.86s (-0.49%)
Artifact size: 391.81 MiB -> 391.79 MiB (-0.01%)

[notriddle]

ship it

[rustbot]

r? @notriddle

rustbot has assigned @notriddle.
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: rustdoc
• rustdoc expanded to 8 candidates
• Random selection from GuillaumeGomez, lolbinarycat, notriddle

[camelid]

r? @notriddle
cc @GuillaumeGomez

[rustbot]

Requested reviewer is already assigned to this pull request.

Please choose another assignee.

[camelid]

I had to make a tweak to get #[doc(notable_trait)] working again (it was depending on a side effect of the old code -- Hyrum's Law!). Let me know if you want to run perf again before merging, though I expect the improvement will be pretty comparable.

[camelid]

That's weird, I could've sworn rustdoc-html was passing before, but it indeed fails locally on my machine (related to notable_trait still I think).

[GuillaumeGomez]

I think a code comment explaining what's going on here would be nice. ;)

View changes since the review

[camelid]

It's explained in the commit message, but yeah a comment would probably be good too.

[GuillaumeGomez]

Is this expected?

View changes since the review

[camelid]

Yes, previously notable_trait wasn't working properly for non-local traits. Now it consistently shows up.

[notriddle]

@bors r=notriddle,GuillaumeGomez

[rust-bors]

📌 Commit afaa983 has been approved by notriddle,GuillaumeGomez

It is now in the queue for this repository.

[JonathanBrouwer]

@bors p=6
Scheduling

[rust-bors]

☀️ Test successful - CI
Approved by: notriddle,GuillaumeGomez
Duration: 3h 18m 8s
Pushing c188565 to main...

[github-actions]

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing b803f36 (parent) -> c188565 (this PR)

Test differences

Show 3 test diffs

3 doctest diffs were found. These are ignored, as they are noisy.

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard c188565812a9b414ddf02f3c7d89e84f1ba53f7d --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

1. x86_64-gnu-debug: 1h 13m -> 1h 56m (+57.9%)
2. x86_64-gnu-gcc-core-tests: 14m 23s -> 7m 56s (-44.8%)
3. x86_64-gnu-llvm-21-1: 34m 26s -> 48m 54s (+42.1%)
4. x86_64-mingw-2: 1h 58m -> 2h 45m (+39.0%)
5. x86_64-gnu-stable: 1h 43m -> 2h 20m (+35.6%)
6. dist-i686-mingw: 2h 3m -> 2h 45m (+33.8%)
7. x86_64-gnu-next-trait-solver-polonius: 1h 2m -> 41m 17s (-33.7%)
8. i686-gnu-nopt-1: 2h 27m -> 1h 37m (-33.7%)
9. dist-i686-msvc: 2h 9m -> 1h 31m (-29.1%)
10. x86_64-gnu-miri: 1h 19m -> 56m 55s (-28.4%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

[rust-timer]

Finished benchmarking commit (c188565): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

• If the regression was expected or you think it can be justified,
  please write a comment with sufficient written justification, and add
  @rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
• If you think that you know of a way to resolve the regression, try to create
  a new PR with a fix for the regression.
• If you do not understand the regression or you think that it is just noise,
  you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
  were already notified of this PR).

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

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)	1.4%	[0.6%, 2.2%]	2
Regressions ❌ (secondary)	2.2%	[2.2%, 2.2%]	1
Improvements ✅ (primary)	-10.8%	[-30.8%, -1.7%]	18
Improvements ✅ (secondary)	-20.3%	[-31.4%, -4.9%]	24
All ❌✅ (primary)	-9.6%	[-30.8%, 2.2%]	20

Max RSS (memory usage)

Results (primary -10.8%, secondary -12.4%)

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

	mean	range	count
Regressions ❌ (primary)	1.4%	[1.4%, 1.4%]	1
Regressions ❌ (secondary)	3.9%	[1.5%, 6.2%]	2
Improvements ✅ (primary)	-11.4%	[-17.3%, -3.3%]	19
Improvements ✅ (secondary)	-13.7%	[-18.2%, -3.8%]	26
All ❌✅ (primary)	-10.8%	[-17.3%, 1.4%]	20

Cycles

Results (primary -13.8%, secondary -27.0%)

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

	mean	range	count
Regressions ❌ (primary)	1.7%	[1.7%, 1.7%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-14.6%	[-36.1%, -2.4%]	18
Improvements ✅ (secondary)	-27.0%	[-36.4%, -9.5%]	23
All ❌✅ (primary)	-13.8%	[-36.1%, 1.7%]	19

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 490.229s -> 489.502s (-0.15%)
Artifact size: 387.80 MiB -> 387.76 MiB (-0.01%)

[camelid]

@rustbot label perf-regression-triaged

Massive improvements with only small regressions (probably due to extra clean_middle_ty call as part of the early filtering). We can improve on that in future PRs.

[nnethercote]

Massive improvements

Great work!

[camelid]

Followup PR with another 7% improvement: #159721