Get rid of `clean` in rustdoc #76382

jyn514 · 2020-09-05T18:16:10Z

Currently, rustdoc has its own data structure for basically everything in the compiler: librustdoc::clean::types. Collecting these ahead of time is expensive (#74590 (comment)) and it would be better to instead calculate them on demand. This would reduce a ton of code duplication, speed up rustdoc, and hopefully fix bugs related to caching (#74879) and get rid of hacks like fake IDs (#75355).

On the other hand, it's really hard.

The basic idea is to, instead of discarding the TyCtxt after run_core, instead pass in TyCtxt to render. Then render will calculate the info it needs as it comes up - possibly still with caching in DocContext, but because this is on-demand it will be like cache.get().or_else(|| calculate_info()), not cache.get().unwrap() which is what leads to the bugs.

cc @rust-lang/rustdoc - is this something you're interested in?
cc @RDambrosio016, @Kixiron - this would break your idea to have librustdoc_render be a separate crate from librustdoc, because there would inherently be no stable interface: DocContext would be returning rustc types directly.
cc @P1n3appl3 - this would require rewriting large parts of the JSON backend.

The text was updated successfully, but these errors were encountered:

jyn514 · 2020-09-05T18:19:39Z

RE cache.get().or_else(|| calculate_info()) - some of these could be queries, but other things rustdoc uses aren't stable cross-crate and can only be cached within a single invocation: #74489 (comment)

Kixiron · 2020-09-05T19:33:48Z

I think not exposing a stable interface in some form is severely limiting the usefulness of having a decoupled backend in the first place, since then every third-party backend is reduced to the maintenance nightmare of a compiler plugin

jyn514 · 2020-09-05T19:41:49Z

since then every third-party backend is reduced to the maintenance nightmare of a compiler plugin

To be clear: this is the current situation. If you want rustdoc to have a stable, programmatic API that's a completely separate RFC.

jyn514 · 2020-09-05T19:43:15Z

That said, I'm not inherently opposed to keeping clean::Item, the thing I care about most is calculating things on-demand instead of having a 'run rustc' pass that's separate from rendering.

Get rid of `clean::Deprecation` This brings the size of `item.deprecation` from 56 to 16 bytes. Helps with rust-lang#79103 and rust-lang#76382, in the same vein as rust-lang#79957. r? `@GuillaumeGomez`

First actually useful step in rust-lang#76382 This doesn't yet compile because there's no way to get a `Lrc<Session>` from a TyCtxt, only a `&Session`.

Pass a `TyCtxt` through to `FormatRender` This is the next step after rust-lang#79957 for rust-lang#76382. Eventually I plan to use this to remove `stability`, `const_stability`, and `deprecation` from `Item`, but that needs more extensive changes (in particular, rust-lang#75355 or something like it). This has no actual changes to behavior, it's just moving types around. ccc rust-lang#80014 (comment)

…meGomez [rustdoc] Calculate stability, const_stability, and deprecation on-demand Previously, they would always be calculated ahead of time, which bloated the size of `clean::Item`. Builds on rust-lang#80090 and should not be merged before. Helps with rust-lang#79103 and rust-lang#76382. cc rust-lang#80014 (comment) This brings `Item` down to 568 bytes, down from 616.

…umeGomez Remove `DefPath` from `Visibility` and calculate it on demand Depends on rust-lang#80090 and should not be merged before. Helps with rust-lang#79103 and rust-lang#76382. cc rust-lang#80014 (comment) - `@nnethercote` I figured it out! It was simpler than I expected :) This brings the size of `clean::Visibility` down from 40 bytes to 8. Note that this does *not* remove `clean::Visibility`, even though it's now basically the same as `ty::Visibility`, because the `Invsible` variant means something different from `Inherited` and I thought it would be be confusing to merge the two. See the new comments on `impl Clean for ty::Visibility` for details.

rustdoc: Remove most fields from ExternalCrate Once rust-lang#84304 is fixed, I can get rid of ExternCrate altogether in favor of CrateNum, but in the meantime, this shrinks ExternalCrate quite a lot. This might hurt compile-times; if it does, I can add `primitive` and `keyword` queries. I expect this to improve compilemem. Helps with rust-lang#76382. r? GuillaumeGomez

rustdoc: Remove unnecessary `is_crate` field from doctree::Module and clean::Module It can be calculated on-demand even without a TyCtxt. This also changed `json::conversions::from_item_kind` to take a whole item, which avoids having to add more and more parameters. Helps with rust-lang#76382. r? `@camelid`

rustdoc: Remove unnecessary `is_crate` field from doctree::Module and clean::Module It can be calculated on-demand even without a TyCtxt. This also changed `json::conversions::from_item_kind` to take a whole item, which avoids having to add more and more parameters. Helps with rust-lang#76382. r? ``@camelid``

rustdoc: Remove unnecessary `is_crate` field from doctree::Module and clean::Module It can be calculated on-demand even without a TyCtxt. This also changed `json::conversions::from_item_kind` to take a whole item, which avoids having to add more and more parameters. Helps with rust-lang#76382. r? ```@camelid```

rustdoc: Remove unnecessary `is_crate` field from doctree::Module and clean::Module It can be calculated on-demand even without a TyCtxt. This also changed `json::conversions::from_item_kind` to take a whole item, which avoids having to add more and more parameters. Helps with rust-lang#76382. r? ``@camelid``

rustdoc: Remove unnecessary `is_crate` field from doctree::Module and clean::Module It can be calculated on-demand even without a TyCtxt. This also changed `json::conversions::from_item_kind` to take a whole item, which avoids having to add more and more parameters. Helps with rust-lang#76382. r? ```@camelid```

Calculate `span` info on-demand - Add helper `attr_span` for common reused function - Stop storing `Span`s on `Item` directly; calculate them on demand instead - Special case modules, which have different spans depending on whether you use inner or outer attributes - Special case impls with fake IDs, which can have either dummy spans (for auto traits) or the DefId of the impl block (for blanket impls) - Use a fake ID for primitives instead of the ID of the crate; this lets `source()` know that it should use a dummy span instead of the span of the crate. This shrinks `Item` from 48 to 40 bytes. Helps with rust-lang#76382.

rustdoc: Remove unnecessary `provided_trait_methods` field from Impl It can be calculated on-demand. Helps with rust-lang#76382.

…yn514 shrink doctree::Module helps rust-lang#76382

camelid · 2021-12-15T20:24:28Z

I updated the title to reflect that we actually want to get rid of all of (or most of) clean, not just Item.

camelid · 2021-12-15T20:27:05Z

this would break your idea to have librustdoc_render be a separate crate from librustdoc, because there would inherently be no stable interface: DocContext would be returning rustc types directly.

I think not exposing a stable interface in some form is severely limiting the usefulness of having a decoupled backend in the first place, since then every third-party backend is reduced to the maintenance nightmare of a compiler plugin

I really don't think rustdoc should be trying to expose a stable interface at all. Rustdoc is already quite full with features, and this would significantly add to the already high maintenance burden.

jyn514 · 2021-12-15T20:34:58Z

In practice I think the JSON backend is already serving that purpose.

jyn514 added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. labels Sep 5, 2020

jyn514 mentioned this issue Sep 5, 2020

[WIP] Remove fake IDs in rustdoc #75355

Closed

jyn514 mentioned this issue Oct 12, 2020

Rustdoc: Fix macros 2.0 and built-in derives being shown at the wrong path #77862

Merged

jyn514 added the I-compilemem Issue: Problems and improvements with respect to memory usage during compilation. label Nov 14, 2020

jyn514 mentioned this issue Nov 16, 2020

Rustdoc takes 8.5 GB of memory to document stm32h7 #79103

Closed

camelid added the I-compiletime Issue: Problems and improvements with respect to compile times. label Nov 25, 2020

jyn514 mentioned this issue Dec 12, 2020

[rustdoc] Box ItemKind::Impl to shrink the size of Item #79967

Closed

jyn514 mentioned this issue Dec 14, 2020

Get rid of clean::Deprecation #80041

Merged

jyn514 mentioned this issue Dec 16, 2020

Pass a TyCtxt through to FormatRender #80090

Merged

This was referenced Dec 16, 2020

[rustdoc] Calculate stability, const_stability, and deprecation on-demand #80095

Merged

Remove DefPath from Visibility and calculate it on demand #80099

Merged

jyn514 added a commit to jyn514/rust that referenced this issue Dec 17, 2020

[tmp] Pass TyCtxt through to the render backend

9221d4d

First actually useful step in rust-lang#76382 This doesn't yet compile because there's no way to get a `Lrc<Session>` from a TyCtxt, only a `&Session`.

jyn514 mentioned this issue Dec 23, 2020

Calculate span info on-demand #80339

Merged

Manishearth added A-rustdoc and removed A-rustdoc labels Apr 23, 2021

jyn514 added the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label Apr 29, 2021

tdelabro mentioned this issue Apr 30, 2021

shrink doctree::Module #84763

Merged

bors added a commit to rust-lang-ci/rust that referenced this issue May 1, 2021

Auto merge of rust-lang#84463 - jyn514:refactor-impl, r=CraftSpider

3d67e07

rustdoc: Remove unnecessary `provided_trait_methods` field from Impl It can be calculated on-demand. Helps with rust-lang#76382.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 1, 2021

Rollup merge of rust-lang#84763 - tdelabro:shrink-doctree-module, r=j…

71fe9df

…yn514 shrink doctree::Module helps rust-lang#76382

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 2, 2021

Rollup merge of rust-lang#84763 - tdelabro:shrink-doctree-module, r=j…

2813a01

…yn514 shrink doctree::Module helps rust-lang#76382

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 2, 2021

Rollup merge of rust-lang#84763 - tdelabro:shrink-doctree-module, r=j…

e643e2e

…yn514 shrink doctree::Module helps rust-lang#76382

camelid changed the title ~~Get rid of clean::Item in rustdoc~~ Get rid of clean in rustdoc Dec 15, 2021

fmease mentioned this issue Mar 23, 2024

rustdoc shows associated type in supertrait bound twice when both Deref and DerefMut have a Target bound #122928

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get rid of `clean` in rustdoc #76382

Get rid of `clean` in rustdoc #76382

jyn514 commented Sep 5, 2020 •

edited

Loading

jyn514 commented Sep 5, 2020

Kixiron commented Sep 5, 2020

jyn514 commented Sep 5, 2020

jyn514 commented Sep 5, 2020

camelid commented Dec 15, 2021

camelid commented Dec 15, 2021

jyn514 commented Dec 15, 2021

Get rid of clean in rustdoc #76382

Get rid of clean in rustdoc #76382

Comments

jyn514 commented Sep 5, 2020 • edited Loading

jyn514 commented Sep 5, 2020

Kixiron commented Sep 5, 2020

jyn514 commented Sep 5, 2020

jyn514 commented Sep 5, 2020

camelid commented Dec 15, 2021

camelid commented Dec 15, 2021

jyn514 commented Dec 15, 2021

Get rid of `clean` in rustdoc #76382

Get rid of `clean` in rustdoc #76382

jyn514 commented Sep 5, 2020 •

edited

Loading