rustdoc: Merge source code pages HTML elements together #100429

GuillaumeGomez · 2022-08-11T21:43:17Z

We realized that the HTML generated for the source code pages could be improved quite a lot. This PR is a first pass toward this goal. Some explanations: it merges similar classes elements (even when there are white characters in between).

There is an exception to this: if this is an ident, I also merged it with "unclassified" elements. This part is up to debate and can be very easily removed as the check is performed in one place (in the can_merge function).

EDIT: The ident is now only kept in the code for the span it contains but it is not rendered into the HTML.

So now some numbers:

For these ones, on each page, I run this JS: document.getElementsByTagName('*').length. The goal is to count the number of DOM elements. I took some pages that seemed big, but don't hesitate to check some others.

file name	before this PR	with this PR	diff	without ident	diff
std/lib.rs.html (source link on std crate page)	3455	2776	20.7%	2387	31%
alloc/vec/mod.rs.html (source on Vec type page)	11012	8084	26.6%	6682	39.4%
alloc/string.rs.html (source on String type page)	10800	8214	24%	6712	37.9%
std/sync/mutex.rs.html (source on Mutex type page)	2953	2403	18.7%	2139	27.6%

You can test it here.

cc @jsha
r? @notriddle

notriddle · 2022-08-11T21:53:11Z

Is this blocked on #100409, or does it replace that PR?

GuillaumeGomez · 2022-08-12T08:38:48Z

Is this blocked on #100409, or does it replace that PR?

It's a separate one, but I'll need to fix conflicts on the tests if it is merged first.

GuillaumeGomez · 2022-08-12T09:42:07Z

Fixed the format issue.

src/librustdoc/html/highlight/fixtures/union.html

rustbot · 2022-08-12T14:38:58Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

jsha · 2022-08-12T14:40:37Z

Are there some unrelated changes in here related to type aliases?

compiler/rustc_const_eval/src/interpret/intrinsics.rs

GuillaumeGomez · 2022-08-12T14:49:36Z

Yes completely, I was working on something else and pushed to the wrong branch. It is now fixed.

rustbot · 2022-08-12T16:18:34Z

A change occurred in the Ayu theme.

cc @Cldfire

GuillaumeGomez · 2022-08-12T16:21:51Z

I updated the online demo, added the new numbers. So in short: we need to keep Class::Ident around because we need the Span but I don't render it into the HTML anymore. The difference in the DOM size is quite big.

bors · 2022-08-12T20:05:43Z

☔ The latest upstream changes (presumably #100456) made this pull request unmergeable. Please resolve the merge conflicts.

GuillaumeGomez · 2022-08-12T20:37:17Z

Fixed the merge conflict with the op class removal.

notriddle · 2022-08-13T13:42:52Z

@bors r+ rollup=never

bors · 2022-08-13T13:42:54Z

📌 Commit 8ed674c0f5161ff189d8a073a405ba8610056545 has been approved by notriddle

It is now in the queue for this repository.

GuillaumeGomez · 2022-08-14T12:10:43Z

@bors r=notriddle

bors · 2022-08-14T12:10:45Z

📌 Commit 6e574e1 has been approved by notriddle

It is now in the queue for this repository.

bors · 2022-08-14T14:37:21Z

⌛ Testing commit 6e574e1 with merge 801821d...

bors · 2022-08-14T16:54:53Z

☀️ Test successful - checks-actions
Approved by: notriddle
Pushing 801821d to master...

rust-timer · 2022-08-14T18:11:16Z

Finished benchmarking commit (801821d): comparison url.

Instruction count

Primary benchmarks: ❌ relevant regressions found
Secondary benchmarks: ❌ relevant regressions found

	mean¹	max	count²
Regressions ❌ (primary)	0.6%	1.4%	12
Regressions ❌ (secondary)	1.8%	3.1%	8
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.6%	1.4%	12

Max RSS (memory usage)

Results

Primary benchmarks: no relevant changes found
Secondary benchmarks: mixed results

	mean¹	max	count²
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	1.5%	1.5%	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.3%	-2.3%	1
All ❌✅ (primary)	-	-	0

Cycles

Results

Primary benchmarks: ❌ relevant regression found
Secondary benchmarks: ❌ relevant regression found

	mean¹	max	count²
Regressions ❌ (primary)	3.7%	3.7%	1
Regressions ❌ (secondary)	3.1%	3.1%	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.7%	3.7%	1

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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

the arithmetic mean of the percent change ↩ ↩² ↩³
number of relevant changes ↩ ↩² ↩³

…otriddle rustdoc: Merge source code pages HTML elements together v2 This is the follow-up of rust-lang#100429. I strongly recommend to review it one commit at a time because otherwise it's a lot at once. For these ones, on each page, I run this JS: `document.getElementsByTagName('*').length`. The goal is to count the number of DOM elements. I took some pages that seemed big, but don't hesitate to check some others. I also added the "starting point" because it's quite nice to see how much the page was reduced thanks to these two PRs. | file name | before rust-lang#100429 | before this PR | with this PR | diff | |-|-|-|-|-| | std/lib.rs.html (source link on std crate page) | 3455 | 2332 | 1772 | 24% | | alloc/vec/mod.rs.html (source on Vec type page) | 11012 | 5982 | 5833 | 2.5% | | alloc/string.rs.html (source on String type page) | 10800 | 6010 | 5822 | 3.2% | | std/sync/mutex.rs.html (source on Mutex type page) | 2953 | 2041 | 2038 | 0.1% | So unsurprisingly, the more attributes you have, the bigger the difference. You can test it [here](https://rustdoc.crud.net/imperio/reduce-span-v2/src/std/lib.rs.html). cc `@jsha` r? `@notriddle`

…otriddle rustdoc: Merge source code pages HTML elements together v2 This is the follow-up of rust-lang#100429. I strongly recommend to review it one commit at a time because otherwise it's a lot at once. For these ones, on each page, I run this JS: `document.getElementsByTagName('*').length`. The goal is to count the number of DOM elements. I took some pages that seemed big, but don't hesitate to check some others. I also added the "starting point" because it's quite nice to see how much the page was reduced thanks to these two PRs. | file name | before rust-lang#100429 | before this PR | with this PR | diff | |-|-|-|-|-| | std/lib.rs.html (source link on std crate page) | 3455 | 2332 | 1772 | 24% | | alloc/vec/mod.rs.html (source on Vec type page) | 11012 | 5982 | 5833 | 2.5% | | alloc/string.rs.html (source on String type page) | 10800 | 6010 | 5822 | 3.2% | | std/sync/mutex.rs.html (source on Mutex type page) | 2953 | 2041 | 2038 | 0.1% | So unsurprisingly, the more attributes you have, the bigger the difference. You can test it [here](https://rustdoc.crud.net/imperio/reduce-span-v2/src/std/lib.rs.html). cc ``@jsha`` r? ``@notriddle``

…otriddle rustdoc: Merge source code pages HTML elements together v2 This is the follow-up of rust-lang#100429. I strongly recommend to review it one commit at a time because otherwise it's a lot at once. For these ones, on each page, I run this JS: `document.getElementsByTagName('*').length`. The goal is to count the number of DOM elements. I took some pages that seemed big, but don't hesitate to check some others. I also added the "starting point" because it's quite nice to see how much the page was reduced thanks to these two PRs. | file name | before rust-lang#100429 | before this PR | with this PR | diff | |-|-|-|-|-| | std/lib.rs.html (source link on std crate page) | 3455 | 2332 | 1772 | 24% | | alloc/vec/mod.rs.html (source on Vec type page) | 11012 | 5982 | 5833 | 2.5% | | alloc/string.rs.html (source on String type page) | 10800 | 6010 | 5822 | 3.2% | | std/sync/mutex.rs.html (source on Mutex type page) | 2953 | 2041 | 2038 | 0.1% | So unsurprisingly, the more attributes you have, the bigger the difference. You can test it [here](https://rustdoc.crud.net/imperio/reduce-span-v2/src/std/lib.rs.html). cc ```@jsha``` r? ```@notriddle```

…otriddle rustdoc: Merge source code pages HTML elements together v2 This is the follow-up of rust-lang#100429. I strongly recommend to review it one commit at a time because otherwise it's a lot at once. For these ones, on each page, I run this JS: `document.getElementsByTagName('*').length`. The goal is to count the number of DOM elements. I took some pages that seemed big, but don't hesitate to check some others. I also added the "starting point" because it's quite nice to see how much the page was reduced thanks to these two PRs. | file name | before rust-lang#100429 | before this PR | with this PR | diff | |-|-|-|-|-| | std/lib.rs.html (source link on std crate page) | 3455 | 2332 | 1772 | 24% | | alloc/vec/mod.rs.html (source on Vec type page) | 11012 | 5982 | 5833 | 2.5% | | alloc/string.rs.html (source on String type page) | 10800 | 6010 | 5822 | 3.2% | | std/sync/mutex.rs.html (source on Mutex type page) | 2953 | 2041 | 2038 | 0.1% | So unsurprisingly, the more attributes you have, the bigger the difference. You can test it [here](https://rustdoc.crud.net/imperio/reduce-span-v2/src/std/lib.rs.html). cc ````@jsha```` r? ````@notriddle````

…otriddle rustdoc: Merge source code pages HTML elements together v2 This is the follow-up of rust-lang#100429. I strongly recommend to review it one commit at a time because otherwise it's a lot at once. For these ones, on each page, I run this JS: `document.getElementsByTagName('*').length`. The goal is to count the number of DOM elements. I took some pages that seemed big, but don't hesitate to check some others. I also added the "starting point" because it's quite nice to see how much the page was reduced thanks to these two PRs. | file name | before rust-lang#100429 | before this PR | with this PR | diff | |-|-|-|-|-| | std/lib.rs.html (source link on std crate page) | 3455 | 2332 | 1772 | 24% | | alloc/vec/mod.rs.html (source on Vec type page) | 11012 | 5982 | 5833 | 2.5% | | alloc/string.rs.html (source on String type page) | 10800 | 6010 | 5822 | 3.2% | | std/sync/mutex.rs.html (source on Mutex type page) | 2953 | 2041 | 2038 | 0.1% | So unsurprisingly, the more attributes you have, the bigger the difference. You can test it [here](https://rustdoc.crud.net/imperio/reduce-span-v2/src/std/lib.rs.html). cc `````@jsha````` r? `````@notriddle`````

…otriddle rustdoc: Merge source code pages HTML elements together v2 This is the follow-up of rust-lang#100429. I strongly recommend to review it one commit at a time because otherwise it's a lot at once. For these ones, on each page, I run this JS: `document.getElementsByTagName('*').length`. The goal is to count the number of DOM elements. I took some pages that seemed big, but don't hesitate to check some others. I also added the "starting point" because it's quite nice to see how much the page was reduced thanks to these two PRs. | file name | before rust-lang#100429 | before this PR | with this PR | diff | |-|-|-|-|-| | std/lib.rs.html (source link on std crate page) | 3455 | 2332 | 1772 | 24% | | alloc/vec/mod.rs.html (source on Vec type page) | 11012 | 5982 | 5833 | 2.5% | | alloc/string.rs.html (source on String type page) | 10800 | 6010 | 5822 | 3.2% | | std/sync/mutex.rs.html (source on Mutex type page) | 2953 | 2041 | 2038 | 0.1% | So unsurprisingly, the more attributes you have, the bigger the difference. You can test it [here](https://rustdoc.crud.net/imperio/reduce-span-v2/src/std/lib.rs.html). cc ``````@jsha`````` r? ``````@notriddle``````

…m-size, r=notriddle Reduce right-side DOM size This is another follow-up of rust-lang#100429 but not in code blocks this time. So the idea is: if there is only one element in the `.rightside` element, there is no need to wrap it, we can just create one node. On each page, I run this JS: `document.getElementsByTagName('*').length`. Important to note: the bigger the number of elements inside the page, the greater the gain. It also doesn't work very nicely on std docs because there are a lot of version annotations. So with this PR, It allows to get the following results: | file name | before this PR | with this PR | diff | |-|-|-|-| | std/default/trait.Default.html | 2189 | 1331 | 39.2% | | std/vec/struct.Vec.html | 14073 | 13842 | 1.7% | | std/fmt/trait.Debug.html | 5313 | 4907 | 7.7% | | std/ops/trait.Index.html | 642 | 630 | 1.9% | | gtk4/WidgetExt | 3269 | 3061 | 6.4% | You can test it [here](https://rustdoc.crud.net/imperio/reduce-rightsize-dom-size/gtk4/prelude/trait.WidgetExt.html). r? `@notriddle`

GuillaumeGomez added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-rustdoc-ui Area: Rustdoc UI (generated HTML) labels Aug 11, 2022

rust-highfive assigned notriddle Aug 11, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 11, 2022

GuillaumeGomez changed the title ~~rustdoc: Merge HTML elements together~~ rustdoc: Merge source code pages HTML elements together Aug 11, 2022

This comment has been minimized.

# to view

GuillaumeGomez force-pushed the merge-html-elements-together branch from 1271f1d to 6e5d1dc Compare August 12, 2022 08:39

notriddle requested changes Aug 12, 2022

View reviewed changes

src/librustdoc/html/highlight/fixtures/union.html Show resolved Hide resolved

GuillaumeGomez force-pushed the merge-html-elements-together branch from 6e5d1dc to 617d6fc Compare August 12, 2022 14:38

RalfJung reviewed Aug 12, 2022

View reviewed changes

compiler/rustc_const_eval/src/interpret/intrinsics.rs Outdated Show resolved Hide resolved

GuillaumeGomez force-pushed the merge-html-elements-together branch from 617d6fc to 6e5d1dc Compare August 12, 2022 14:49

GuillaumeGomez force-pushed the merge-html-elements-together branch from 6e5d1dc to 4dd7276 Compare August 12, 2022 16:18

GuillaumeGomez force-pushed the merge-html-elements-together branch 2 times, most recently from e799af3 to efa9532 Compare August 12, 2022 18:46

GuillaumeGomez force-pushed the merge-html-elements-together branch from efa9532 to 3b2ef32 Compare August 12, 2022 20:37

GuillaumeGomez force-pushed the merge-html-elements-together branch from 3b2ef32 to 8ed674c Compare August 12, 2022 20:38

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 Aug 13, 2022

bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Aug 14, 2022

GuillaumeGomez added 2 commits August 14, 2022 11:30

Merge HTML elements in highlighting when they can be merged together

68f327b

Don't generate ident elements as DOM nodes

33df8a9

GuillaumeGomez force-pushed the merge-html-elements-together branch from 8ed674c to 6caff4b Compare August 14, 2022 09:32

This comment has been minimized.

# to view

Update rustdoc tests

6e574e1

GuillaumeGomez force-pushed the merge-html-elements-together branch from 6caff4b to 6e574e1 Compare August 14, 2022 11:32

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 Aug 14, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 14, 2022

bors merged commit 801821d into rust-lang:master Aug 14, 2022

rustbot added this to the 1.65.0 milestone Aug 14, 2022

GuillaumeGomez deleted the merge-html-elements-together branch August 14, 2022 16:58

rustbot added the perf-regression Performance regression. label Aug 14, 2022

GuillaumeGomez mentioned this pull request Aug 19, 2022

rustdoc: Merge source code pages HTML elements together v2 #100775

Merged

GuillaumeGomez mentioned this pull request Aug 24, 2022

Reduce right-side DOM size #100956

Merged

GuillaumeGomez mentioned this pull request Nov 4, 2022

HTML generated for code blocks can be improved (and shortened) #74180

Closed

matthiaskrgr mentioned this pull request Aug 27, 2023

ICE: no name for HirId #115264

Closed

rustdoc: Merge source code pages HTML elements together #100429

rustdoc: Merge source code pages HTML elements together #100429

Uh oh!

Conversation

GuillaumeGomez commented Aug 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

notriddle commented Aug 11, 2022

Uh oh!

This comment has been minimized.

GuillaumeGomez commented Aug 12, 2022

Uh oh!

GuillaumeGomez commented Aug 12, 2022

Uh oh!

Uh oh!

rustbot commented Aug 12, 2022

Uh oh!

jsha commented Aug 12, 2022

Uh oh!

Uh oh!

GuillaumeGomez commented Aug 12, 2022

Uh oh!

rustbot commented Aug 12, 2022

Uh oh!

GuillaumeGomez commented Aug 12, 2022

Uh oh!

bors commented Aug 12, 2022

Uh oh!

GuillaumeGomez commented Aug 12, 2022

Uh oh!

notriddle commented Aug 13, 2022

Uh oh!

bors commented Aug 13, 2022

Uh oh!

This comment has been minimized.

GuillaumeGomez commented Aug 14, 2022

Uh oh!

bors commented Aug 14, 2022

Uh oh!

bors commented Aug 14, 2022

Uh oh!

bors commented Aug 14, 2022

Uh oh!

rust-timer commented Aug 14, 2022

Instruction count

Max RSS (memory usage)

Cycles

Footnotes

Uh oh!

Uh oh!

GuillaumeGomez commented Aug 11, 2022 •

edited

Loading