rustdoc: optimize impl sorting#157179
Conversation
Currently impls printed in alternate mode use plain text for the first part and HTML for the where clause. This commit fixes the where clause so it's also plain text.
These sorts are hot, so it's significantly faster to use concise plain text rather than verbose HTML.
We don't need to because the char ordering works out naturally.
|
LLM disclosure: I wrote the three commits by hand after viewing profiling data. I used an LLM to review the commits before submitting. The LLM identified that |
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
rustdoc: optimize impl sorting
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (e444e2a): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking 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. @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (secondary -3.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -2.2%, secondary -4.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 513.315s -> 511.874s (-0.28%) |
|
Nice wins! Have you tried replacing Btw I noticed at https://doc.rust-lang.org/std/cell/struct.Cell.html#trait-implementations, then in the bar on the left, |
Yes... this approach gets about 90-95% of the improvement of replacing
Good question, I'm not sure. I just checked with my local builds. The orders are unchanged by this PR, so I think I've successfully preserved the existing behaviour. Which seems reasonable for an optimization PR. Any bugs should probably be fixed in a separate PR. |
|
Looks great, thanks! @bors r+ rollup |
|
Woups, perf sensitive. @bors rollup=never |
This comment has been minimized.
This comment has been minimized.
I meant combining this PR with replacing compare_names with just cmp. |
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 9e293ae (parent) -> 1ce45a0 (this PR) Test differencesShow 4 test diffs4 doctest diffs were found. These are ignored, as they are noisy. Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 1ce45a011a3fe99ab08643d8eb7229bfc3b40bda --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (1ce45a0): comparison URL. Overall result: ✅ improvements - no action needed@rustbot label: -perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -3.2%, secondary -5.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesThis perf run didn't have relevant results for this metric. Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 508.947s -> 513.507s (0.90%) |
I have a fix for this in #157233. |
Currently rustdoc sorts impls in a couple of places using long HTML string representations of the impls. Using plain text representations instead speeds things up. Details in individual commits.