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: sidebar generation code duplicates logic #138576

Open
lolbinarycat opened this issue Mar 16, 2025 · 2 comments
Open

rustdoc: sidebar generation code duplicates logic #138576

lolbinarycat opened this issue Mar 16, 2025 · 2 comments
Assignees
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@lolbinarycat
Copy link
Contributor

I recently discovered this in #138574, when I fixed the bug for the main page, but the erroneous items were still in the sidebar.

If we simply collected a list of sidebar items during rendering of the main page, almost the entirety of sidebar.rs could be removed. This could even be done incrementally, with each sidebar_* function being replaced with a field on Context.

@lolbinarycat lolbinarycat 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. labels Mar 16, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 16, 2025
@yotamofek
Copy link
Contributor

IIUC, it would also mean we won't need interior mutability for deref_id_map, which might improve performance by a little bit.
And consequently, might also allow us to "lazily" render the content and sidebar into the same buffer, which might improve perf, maybe even substantially.

So - good idea! LMK if I can help out with this.

@lolbinarycat lolbinarycat self-assigned this Mar 18, 2025
@lolbinarycat
Copy link
Contributor Author

lolbinarycat commented Mar 18, 2025

I'm thinking we'll add a field to Context, like sidebar_map: HashMap<ItemId, SidebarLinks>, where SidebarLinks is a struct that has fields for each different type of sidebar item.

The one downside is that Link will likely be unable to effectively use Cow, which will have some impact due to some extra cloning being required.

We'll have to do some profiling to see if not recalculating things makes up for the extra allocations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. 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

3 participants