[rustdoc] Calculate span information on demand instead of storing it ahead of time#79957

jyn514 · 2020-12-12T04:11:55Z

This brings size_of<clean::types::Span>() down from over 100 bytes (!!) to only 12, the same as rustc. It brings Item down even more, from 784 to 680.

TODO: I need to figure out how to do this for the JSON backend too. That uses From impls everywhere, which don't allow passing in the Session as an argument. @P1n3appl3, @tmandry, maybe one of you have ideas? Figured it out, fortunately only two functions needed to be changed. I like the convert_x() format better than From everywhere but I'm open to feedback.

Helps with #79103

rust-highfive · 2020-12-12T04:11:59Z

r? @GuillaumeGomez

(rust-highfive has picked a reviewer for you, use r? to override)

jyn514 · 2020-12-12T04:25:02Z

This brings stm32h7 to 2425472 KB used (2.42 GB) in 2m 41s, down from 2565508 KB (2.56 GB) in 2m 36s. I think the change in time is just noise from other programs on my system, rustc-perf will have more accurate times.

jyn514 · 2020-12-12T04:26:17Z

Not sure when it went from 8.5 GB to 2.5 GB ... maybe I measured wrong at some point?

jyn514 · 2020-12-12T04:31:21Z

2611304 (2.6 GB) on beta, 2645148 (2.6 GB) on stable. I think I'm measuring this wrong somehow.

jyn514 · 2020-12-12T04:36:49Z

@bors try @rust-timer queue

rust-timer · 2020-12-12T04:36:50Z

Awaiting bors try build completion

bors · 2020-12-12T04:36:59Z

⌛ Trying commit e852bb4bf1a8a460240a01474aaba58221cd9df1 with merge 7a00fa80319b87268f2e011e4d760b5ceabade92...

jyn514 · 2020-12-12T04:56:00Z

Ok yes, I was measuring this wrong - the allocation in check_mod_items is allocated and deallocated so quickly that looking at /proc/self/mem with polling misses it. Heaptrack picks up on it though.

Without the changes (39b841d):

bytes allocated in total (ignoring deallocations): 39.31GB (104.62MB/s)
calls to allocation functions: 177032707 (471157/s)
temporary memory allocations: 34929957 (92963/s)
peak heap memory consumption: 2.33GB
peak RSS (including heaptrack overhead): 26.92GB

With the changes:

bytes allocated in total (ignoring deallocations): 39.15GB (112.73MB/s)
calls to allocation functions: 177801926 (512030/s)
temporary memory allocations: 34890948 (100478/s)
peak heap memory consumption: 2.19GB
peak RSS (including heaptrack overhead): 26.08GB

This should *vastly* reduce memory usage.

This was simpler than expected.

bjorn3 · 2020-12-12T09:06:38Z

Rustfmt fails and I think your push canceled the try run.

src/librustdoc/clean/types.rs

GuillaumeGomez

This is an awesome PR! Thanks a lot for working on it. I just have one suggestion but you can ignore it if you feel that the current code is better, I have no strong preference here.

- Use a tuple struct instead of a single field - Enforce calling `source_callsite()` by making the inner span private - Rename `empty` to `dummy`

GuillaumeGomez · 2020-12-12T14:44:12Z

The current code looks good to me. I repeat myself but this is really an amazing PR, thanks a lot! r=me when CI pass. :)

jyn514 · 2020-12-12T14:45:02Z

The current code looks good to me. I repeat myself but this is really an amazing PR, thanks a lot! r=me when CI pass. :)

I want to see the perf improvement first, I like seeing green numbers 😆

rust-timer · 2020-12-12T15:52:22Z

Finished benchmarking try commit (c6c84e4efa0153524a3de2b6c65eac7adf1cc6e7): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot modify labels: +S-waiting-on-review -S-waiting-on-perf

jyn514 · 2020-12-12T15:53:32Z

Up to -2.4% on instructions, up to -3% on max-rss 🎉 🎉

@bors r=GuillaumeGomez

bors · 2020-12-12T15:53:34Z

📌 Commit 9684557 has been approved by GuillaumeGomez

bors · 2020-12-12T19:42:01Z

⌛ Testing commit 9684557 with merge 7efc097...

bors · 2020-12-12T22:13:03Z

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing 7efc097 to master...

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`

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)

jyn514 added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. I-compilemem Issue: Problems and improvements with respect to memory usage during compilation. labels Dec 12, 2020

rust-highfive assigned GuillaumeGomez Dec 12, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 12, 2020

This comment has been minimized.

Sign in to view

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 12, 2020

This comment has been minimized.

Sign in to view

jyn514 force-pushed the smaller-span branch from d7b4127 to e852bb4 Compare December 12, 2020 04:36

jyn514 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 12, 2020

jyn514 added 3 commits December 12, 2020 00:00

Pass Session into renderer

af6aa9f

Calculate span info on-demand instead of ahead of time

4fa95b3

This should *vastly* reduce memory usage.

Fix the JSON backend

0e574fb

This was simpler than expected.

jyn514 marked this pull request as ready for review December 12, 2020 05:04

jyn514 force-pushed the smaller-span branch from 325fc7a to 0e574fb Compare December 12, 2020 12:56

GuillaumeGomez reviewed Dec 12, 2020

View reviewed changes

src/librustdoc/clean/types.rs Outdated Show resolved Hide resolved

GuillaumeGomez approved these changes Dec 12, 2020

View reviewed changes

Small cleanups

9684557

- Use a tuple struct instead of a single field - Enforce calling `source_callsite()` by making the inner span private - Rename `empty` to `dummy`

GuillaumeGomez approved these changes Dec 12, 2020

View reviewed changes

jyn514 mentioned this pull request Dec 12, 2020

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

Closed

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 Dec 12, 2020

jyn514 mentioned this pull request Dec 12, 2020

[rustdoc] Box some more ItemKind variants #79975

Closed

bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 12, 2020

bors merged commit 7efc097 into rust-lang:master Dec 12, 2020

rustbot added this to the 1.50.0 milestone Dec 12, 2020

This was referenced Dec 12, 2020

Add Sized trait display when implemented on type #78181

Merged

Don't look for blanket impls in intra-doc links #79682

Merged

Refractor a few more types to rustc_type_ir #79951

Merged

jyn514 deleted the smaller-span branch December 12, 2020 22:48

jyn514 mentioned this pull request Dec 14, 2020

Get rid of clean::Deprecation #80041

Merged

This was referenced Dec 16, 2020

[rustdoc] Box ItemKind to reduce the size of Item #80014

Merged

Pass a TyCtxt through to FormatRender #80090

Merged

Uh oh!

Conversation

jyn514 commented Dec 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented Dec 12, 2020

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

jyn514 commented Dec 12, 2020

Uh oh!

jyn514 commented Dec 12, 2020

Uh oh!

jyn514 commented Dec 12, 2020

Uh oh!

This comment has been minimized.

This comment has been minimized.

jyn514 commented Dec 12, 2020

Uh oh!

rust-timer commented Dec 12, 2020

Uh oh!

bors commented Dec 12, 2020

Uh oh!

jyn514 commented Dec 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bjorn3 commented Dec 12, 2020

Uh oh!

Uh oh!

GuillaumeGomez left a comment

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomez commented Dec 12, 2020

Uh oh!

jyn514 commented Dec 12, 2020

Uh oh!

rust-timer commented Dec 12, 2020

Uh oh!

jyn514 commented Dec 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bors commented Dec 12, 2020

Uh oh!

bors commented Dec 12, 2020

Uh oh!

bors commented Dec 12, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

jyn514 commented Dec 12, 2020 •

edited

Loading

jyn514 commented Dec 12, 2020 •

edited

Loading

jyn514 commented Dec 12, 2020 •

edited

Loading