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

rustdoc: Get rid of Cache::extern_locations and calculate the info on-demand #84588

Closed
jyn514 opened this issue Apr 26, 2021 · 3 comments
Closed
Assignees
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. I-compilemem Issue: Problems and improvements with respect to memory usage during compilation. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@jyn514
Copy link
Member

jyn514 commented Apr 26, 2021

Right now, extern_locations is populated like this: https://github.com/rust-lang/rust/blob/d81d7ab203207c4110fbf1919a99345f7e9c1498/src/librustdoc/formats/cache.rs#L158-L171

All of these can be computed on-demand with only the CrateNum, TyCtxt, and extern_html_root_urls. We should do so to avoid doing more work than necessary (and taking up lots of memory).

Mentoring instructions:

  1. Add functions src_root, name, and location to ExternalCrate, using the same implementation that's currently in Cache::populate.
  2. Change all uses of cache.extern_location to use the functions instead.
  3. Delete cache.extern_location.

Originally posted by @jyn514 in #84494 (comment)

@jyn514 jyn514 added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. I-compilemem Issue: Problems and improvements with respect to memory usage during compilation. labels Apr 26, 2021
@jyn514
Copy link
Member Author

jyn514 commented Apr 26, 2021

Note that this is blocked on #84494 so you can construct an ExternalCrate with only the CrateNum.

@tdelabro
Copy link
Contributor

@rustbot claim

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Apr 30, 2021
…xtern_locations, r=jyn514

rustdoc: Only store locations in Cache::extern_locations and calculate the other info on-demand

 help rust-lang#84588
JohnTitor added a commit to JohnTitor/rust that referenced this issue May 1, 2021
…xtern_locations, r=jyn514

rustdoc: Only store locations in Cache::extern_locations and calculate the other info on-demand

 help rust-lang#84588
JohnTitor added a commit to JohnTitor/rust that referenced this issue May 1, 2021
…xtern_locations, r=jyn514

rustdoc: Only store locations in Cache::extern_locations and calculate the other info on-demand

 help rust-lang#84588
@jyn514
Copy link
Member Author

jyn514 commented May 18, 2021

The easy part of this was done in #84601, and I think the hard part is more trouble than it's worth (see #84601 (comment)).

@jyn514 jyn514 closed this as completed May 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. I-compilemem Issue: Problems and improvements with respect to memory usage during compilation. 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

2 participants