Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[tracking-ish issue] Doc pages are very slow to render #55900

Closed
1 of 3 tasks
GuillaumeGomez opened this issue Nov 12, 2018 · 71 comments
Closed
1 of 3 tasks

[tracking-ish issue] Doc pages are very slow to render #55900

GuillaumeGomez opened this issue Nov 12, 2018 · 71 comments
Assignees
Labels
regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Nov 12, 2018

Message from discord:

the https://doc.rust-lang.org/std/iter/trait.Iterator.html page(and only that page?) seems to have a broken search bar that manifests as infinite? loading and high CPU usage in both Chrome and firefox, across stable, beta, nightly, and local versions.

Related:

@GuillaumeGomez GuillaumeGomez added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Nov 12, 2018
@GuillaumeGomez GuillaumeGomez self-assigned this Nov 12, 2018
@ollie27
Copy link
Member

ollie27 commented Nov 12, 2018

The HTML for that page is currently about 12.5MB thanks to #51885. We're now rendering a copy of every trait method for every impl. We could revert #51885 or only render methods that have documentation in the impl.

@JeanMertz
Copy link

Just to confirm this issue: I wanted to add that even loading that page is next to impossible on Safari. It takes a very long time, and after it loads, scrolling is so slow that the page is next to useless. I often have to switch to Firefox (my secondary browser) only to read this specific page in the docs.

@crumblingstatue
Copy link
Contributor

Not only is it very slow to render initially, but due to the sheer amount of content on the page, it is also very sluggish to scroll through it on my system.

I also agree that we should show less information by default. A 12.5 MB doc page for a single item sounds ridiculous.

I'd very much prefer if implementors were just listed plainly like so: Implementors: std::ascii::EscapeDefault, EscapeDebug, std::char::EscapeDefault, EscapeUnicode, ToLowercase, ToUppercase, ..., with them being links so you can click on them to go to their doc page.

This amount of duplication is a lot of wasted data. It makes the documentation bloated and slow.

@Stargateur
Copy link
Contributor

Stargateur commented Dec 5, 2018

Also it would be good to fix this as fast as possible (I have not idea how still a newbie), Iterator are a lot used and be able to read the doc is primordial.

Freeze on Chrome Version 70.0.3538.110 (Build officiel) (64 bits)
Load quite fast on Firefox 63.0.3 (64 bits) ;)

@GuillaumeGomez
Copy link
Member Author

The PR is open and waiting for reviews.

bors added a commit that referenced this issue Dec 15, 2018
…reavus

Greatly improve rustdoc rendering speed issues

Fixes #55900.

So a few improvements here:

* we're switching to `DOMTokenList` API when available providing a replacement if it isn't (should only happen on safari and IE I think...)
* hide doc sections by default to allow the whole HTML generation to happen in the background to avoid triggering DOM redraw all the times (which killed the performances)

r? @QuietMisdreavus
@nocduro
Copy link

nocduro commented Jan 8, 2019

Has this been deployed? I'm still having really sluggish performance on https://doc.rust-lang.org/std/iter/trait.Iterator.html

Related issue #57281

@steveklabnik
Copy link
Member

steveklabnik commented Jan 8, 2019 via email

@nocduro
Copy link

nocduro commented Jan 8, 2019

Just tried beta and nightly in incognito and it works about the same. The page doesn't freeze or anything, but scrolling is a bit laggy. Chrome uses ~30-40% cpu when scrolling.

I think this patch is live though, because nightly shows some loading text while the sections are loaded instead of instantly trying to render?

@Stargateur
Copy link
Contributor

yeah, that doesn't freeze but the page still load tons of information, is it possible to have the page that only load what user required, for example, a plus bottom that will only load content when opened ? That would allow to see method, without load everything.

Also, all theses informations are them really needed ? Why did we do this change ?

@GuillaumeGomez
Copy link
Member Author

The rendering was slow in any case so whatever changes we want to perform afterwards, we'll still gain from them. We're debating about what information could/should be removed on the pages alongside making the HTML/JS lighter. It's moving forward.

@crumblingstatue
Copy link
Contributor

I just checked the nightly doc page. On my computer, it's just as slow as it was before. It takes quite a lot of seconds before it loads, and even then the scrolling is sluggish.

@GuillaumeGomez
Copy link
Member Author

I don't see such behavior (either on firefox or chrome) so the problem could come from your computer configuration?

@crumblingstatue
Copy link
Contributor

I don't see such behavior (either on firefox or chrome) so the problem could come from your computer configuration?

I have a pretty old computer, but reading documentation shouldn't require a beefy computer. No other documentation I ever used had this problem. In fact, old versions of the Iterator page posed no problem either. But the new versions are way too large.

@JeanMertz
Copy link

I know this is being worked on, so my post is just "another datapoint". Ignore this if it doesn't add anything you don't already know.

Are we talking about this page? https://doc.rust-lang.org/nightly/std/iter/trait.Iterator.html

If so, that one is (still) slow for me as well, on a 2017 MBP. It takes about 5 seconds to load, and once it does, scrolling down is jerky from time to time (but not as often as before), and shows white screens until rendering catches up.

I do believe it's quite a bit faster than before, but still not great.

It's also worth considering what information people want to get from that page. Anything above "Implementations on Foreign Types" is only a fraction of the contents of the page (understandably, as the rest is mostly about other implementors), while it contains all you need to know on how to use Iterator (what that section lacks, is what implements Iterator, which is the bulk of the page) so perhaps it makes sense to put a cut there, and load the rest of the page lazily on request.

@Stargateur
Copy link
Contributor

Stargateur commented Jan 9, 2019

there is no need to prove the page is slow:

curl "https://doc.rust-lang.org/nightly/std/iter/trait.Iterator.html" > /dev/null
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 13.1M  100 13.1M    0     0  3375k      0  0:00:04  0:00:04 --:--:-- 2761k

13.1M, for a html page ? there is a problem. Even, C11 standard only does 2M, https://port70.net/~nsz/c/c11/n1570.html, and it's already too big.

@GuillaumeGomez
Copy link
Member Author

We agree on this and minification is being worked on like said previously: #56869

@Stargateur
Copy link
Contributor

Stargateur commented Jan 9, 2019

I don't think micro optimization will fix the problem, we need a real solution that change how doc is working. 13 - 1 => 12 M. That will not change much. Also, that not a problem for me to download heavy thing, the problem is to render too many thing in the page.

We could argue that the problem is "only" in iterator, but iterator just show there is a problem, that could happen on other lib.

@DianaNites
Copy link

DianaNites commented Jan 9, 2019

Wouldn't a better fix would be for pages to just not to repeat their own trait documentation in Implementors, just a list of what implements it and for what Item?(Or more generally whatever associated constants) Cut it out entirely.

Then the Iterator page wouldn't repeat it's entire documentation for everything. That seems to me to be where the bulk of the page is.

Plus the nightly page, while faster, is significantly less usable since theres no content until it finishes loading everything. On the current page you can scroll and read to what you need while it loads the huge implementors section. On nightly, you only get "loading content" until the entire page finishes.

@0xpr03
Copy link

0xpr03 commented Jan 10, 2019

The iter site works pretty fine on my side.
Yeah it takes some seconds but scrolling is fine on my laptop. I don't think we can do too much about this without removing relevant information. Sometimes it's just that: A pretty huge amount of stuff to display. Just as in high LoC files.

@GuillaumeGomez
Copy link
Member Author

I don't think micro optimization will fix the problem, we need a real solution that change how doc is working. 13 - 1 => 12 M.

This is just one amongst others. There are PRs to reduce DOM size and other things. When we'll reach the end of those optimizations, if it's still not enough, we'll check for other solutions at this time.

@matthieu-m
Copy link
Contributor

I would second the opinion that repeating trait methods for each and every implementor is overkill. The implementor's page will already provide those methods in full.

Beyond the performance issues, it also requires scrolling a WHOLE lot more, unless one uses the minimized mode, which I find even more concerning.

Sometimes less is more.

@steveklabnik
Copy link
Member

There's always tension between different use-cases. Sometimes, people want docs for individual implementations, to talk about what they do, etc. That can only happen if things are repeated.

@DianaNites
Copy link

What use-case does the Implementors section on Traits solve? Right now all it does is bloat pages and repeat the methods already listed above. Note that it doesn't repeat their documentation. Just a straight list of documentation-less method signatures that are already listed above, for everything that uses Iterator.

The Trait Implementations section, which appears on structs, is fine as-is, and already does something different than the Implementors section, it has documentation.

@ebarnard
Copy link
Contributor

When opening the Iterator page in the latest nightly docs (which include #56005) it takes about 45 seconds for "Loading content..." to disappear and the Iterator docs to become usable. This does not include network time. (Late 2013 MPB running Safari 12 and Firefox 64).

This renders the Iterator docs page unusable unless I am careful not to use the search feature, click on any links, or navigate away from the page.

I'm not sure whether others are experiencing similarly long load times but if they are it seems a convincing reason to revert #51885 until the performance issues can be fixed.

I'm also unsure of the intentions of the Implementors section. Is it meant to show docs written on methods in an impl block rather than on the methods in the trait declaration.

@matthieu-m
Copy link
Contributor

I think that somewhat makes sense for common traits like Iterator, but not in general.

One useful side of the expanded doc view is being able to see what methods you have available. Often these are through traits.

There are two distinct use cases though:

  1. Showing which traits are implemented by the current struct/trait A on the page of A, and the methods available.
  2. Showing all implementations of a trait A on the page of A, and duplicate the trait methods for each implementation.

I agree with you that (1) is very useful, however (2) is a completely separate usecase and the cause of the issue for Iterator.

I find that @wafflespeanut adequately solves both (2) and the bloat issue: it shows all implementers, yet conveniently sidesteps the performance issue by referring to the implementer's page for further information.

@Manishearth
Copy link
Member

Oh, I see. Hmm. That seems okay.

(Though I'll note the performance issues are due to various bugs and not the actual page content. Browsers are pretty good about not processing display:none things)

@wafflespeanut
Copy link
Contributor

One useful side of the expanded doc view is being able to see what methods you have available. Often these are through traits.

This is true and this is the case for "type" pages. I never suggested to remove anything there. I was talking about removing the stuff in "trait" pages, because we already have all the trait methods up top (so why duplicate everything?). I'm also suggesting to provide a link (next to the trait impl in a trait's page) for the user to see that impl in the corresponding type's page.

Though I'll note the performance issues are due to various bugs and not the actual page content.

I agree that browsers are very good, but I still think the current state of rustdoc (specifically for Iterator trait) is an overkill, especially when we don't have a good reason for listing all trait methods for all trait impls.

@sinesc
Copy link

sinesc commented Apr 26, 2019

As soon as the last "Loading content..." appears the browser has processed the entire HTML. After that it is "just" layouting the blocks that were previously display: none.
One could flip those blocks individually from hidden to visible with a setTimeout(.. ,0) inbetween to allow the browser to layout the blocks individually. Total rendering time would go up slightly, but the stuff at the top would appear faster.

@Manishearth
Copy link
Member

I agree that browsers are very good, but I still think the current state of rustdoc (specifically for Iterator trait) is an overkill, especially when we don't have a good reason for listing all trait methods for all trait impls.

Yeah I agree with that.

I'm more saying that this isn't the cause of the performance issues.

@GuillaumeGomez
Copy link
Member Author

It could be part of it. That's something that has been suggested a while ago and that I wanted to give a try to. I'll try to find some time to check locally if that improves things.

@bendem
Copy link

bendem commented May 31, 2019

Speaking of doc performance, you might want to fix the server's config to compress html documents. Those 13MB are 692K gzip'd, 74K brotli'd.

@GuillaumeGomez
Copy link
Member Author

That's not up to rustdoc unfortunately. Please open an issue on docs.rs repository.

@ebarnard
Copy link
Contributor

ebarnard commented Jun 1, 2019

The Iterator docs are hosted on https://doc.rust-lang.org. Presumably that isn't hosted by docs.rs? I'm not sure where the right place to open an issue would be.

@GuillaumeGomez
Copy link
Member Author

Like I said, on their repository: https://github.com/rust-lang/docs.rs

@SimonSapin
Copy link
Contributor

I believe the hosting of rust-lang.org (including doc.rust-lang.org) is the responsibility of the @rust-lang/infra team. Maybe a new issue on https://github.com/rust-lang/infra-team/issues would be the appropriate place to discuss server configuration?

@GuillaumeGomez
Copy link
Member Author

Arf sorry, I kept reading docs.rs instead of doc.rust-lang.org...

@pietroalbini
Copy link
Member

Speaking of doc performance, you might want to fix the server's config to compress html documents. Those 13MB are 692K gzip'd, 74K brotli'd.

We already have automatic compression configured on the CloudFront side, but CloudFront won't compress files bigger than 10MB, so fixing this is not a simple configuration change.

@ebarnard
Copy link
Contributor

ebarnard commented Jun 3, 2019

Can I suggest a minimal change, as far as I can see without downsides, that should significantly reduce the size of the Iterator doc page:

In the Implementors and Implementations on Foreign Types sections, only show methods that appear in the impl block for that type. This has the benefit of

  • Reducing the size of the Iterator page, and other large trait documentation pages.
  • Retaining documentation on the impl blocks and functions in the impl blocks.
  • Indicating which provided methods are overridden.
  • Making the documentation match the structure of the code being documented.
  • Being a small change that can be easily backed out if issues arise.

What this would look like for the Read trait:
Screenshot 2019-06-03 at 16 41 27

This should be an easy change to make so I'll try and have a PR ready soon so we can see what impact it would have on page sizes and load times.

@ebarnard
Copy link
Contributor

ebarnard commented Jun 3, 2019

A set of Rust stdlib docs build with this change are available here.

The Iterator doc page is now 724kB.

@memoryruins
Copy link
Contributor

Thank you for trying these changes and providing a demo!

Immediate impression:

  • Current nightly Iterator docs: similar to my previous report [tracking-ish issue] Doc pages are very slow to render #55900 (comment),
    except causes my mobile browser to become unresponsive and crash.
  • @ebarnard 's iterator docs: loads nearly immediately on laptop 🎉, and it scrolls smoothly without page flickering or laptop fans becoming loud. It loaded quickly on my phone too :)
    It appears to make docs pages clearer and easier to navigate in general.

Another example trait page that should benefit, which I'm unable to load on my laptop:
https://docs.rs/uom/0.23.1/uom/trait.Conversion.html

@GuillaumeGomez
Copy link
Member Author

@ebarnard: Looks great! Can you open a PR with your changes please? It'll also allow to see if other people like this change or not.

@ebarnard
Copy link
Contributor

ebarnard commented Jun 4, 2019

@GuillaumeGomez It's open at #61505

Centril added a commit to Centril/rust that referenced this issue Jun 18, 2019
Only show methods that appear in `impl` blocks in the Implementors sections of trait doc pages

In the "Implementors" and "Implementations on Foreign Types" sections, only show methods that appear in the `impl` block for that type. This has the benefit of
- Reducing the size of the Iterator page, and other large trait documentation pages.
- Retaining documentation on the `impl` blocks and functions in the `impl` blocks.
- Indicating which provided methods are overridden.
- Making the documentation match the structure of the code being documented.
- Being a small change that can be easily backed out if issues arise.

A set of Rust stdlib docs build with this change are [available here](https://ebarnard.github.io/2019-06-03-rust-smaller-trait-implementers-docs/).

The size of the [`Iterator` doc page](https://ebarnard.github.io/2019-06-03-rust-smaller-trait-implementers-docs/std/iter/trait.Iterator.html) is reduced from 14.4MB (latest nightly) to 724kB.

Before:
<img width="1411" alt="Screenshot 2019-06-03 at 23 12 17" src="https://user-images.githubusercontent.com/1059683/58837971-1722a780-8655-11e9-8d81-51e48130951d.png">

After:
<img width="1428" alt="Screenshot 2019-06-03 at 16 41 27" src="https://user-images.githubusercontent.com/1059683/58814907-84ffac80-861e-11e9-8692-79be473a5299.png">

cc rust-lang#55900
Centril added a commit to Centril/rust that referenced this issue Jun 18, 2019
Only show methods that appear in `impl` blocks in the Implementors sections of trait doc pages

In the "Implementors" and "Implementations on Foreign Types" sections, only show methods that appear in the `impl` block for that type. This has the benefit of
- Reducing the size of the Iterator page, and other large trait documentation pages.
- Retaining documentation on the `impl` blocks and functions in the `impl` blocks.
- Indicating which provided methods are overridden.
- Making the documentation match the structure of the code being documented.
- Being a small change that can be easily backed out if issues arise.

A set of Rust stdlib docs build with this change are [available here](https://ebarnard.github.io/2019-06-03-rust-smaller-trait-implementers-docs/).

The size of the [`Iterator` doc page](https://ebarnard.github.io/2019-06-03-rust-smaller-trait-implementers-docs/std/iter/trait.Iterator.html) is reduced from 14.4MB (latest nightly) to 724kB.

Before:
<img width="1411" alt="Screenshot 2019-06-03 at 23 12 17" src="https://user-images.githubusercontent.com/1059683/58837971-1722a780-8655-11e9-8d81-51e48130951d.png">

After:
<img width="1428" alt="Screenshot 2019-06-03 at 16 41 27" src="https://user-images.githubusercontent.com/1059683/58814907-84ffac80-861e-11e9-8692-79be473a5299.png">

cc rust-lang#55900
@GuillaumeGomez
Copy link
Member Author

Little update: I think I'll investigate wasm at some point and see if it improves the situation. But first, I want to check if there is a way to improve how we store the information in the search-index.js file.

@ollie27
Copy link
Member

ollie27 commented Apr 16, 2020

I think this issue should be closed. The original regression caused by #51885 has long since been fixed by #61505. I think we can track specific performance problems in dedicated issues like #56545.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests