Skip to content

Conversation

@GuillaumeGomez
Copy link
Member

A few things I had sitting around for a while. Let's check what perf says about them

r? @yotamofek

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. labels Jan 19, 2026
@GuillaumeGomez
Copy link
Member Author

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

rust-bors bot pushed a commit that referenced this pull request Jan 19, 2026
@rust-bors

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 19, 2026
@rust-bors
Copy link
Contributor

rust-bors bot commented Jan 19, 2026

☀️ Try build successful (CI)
Build commit: aa5a68b (aa5a68b1f547922fb3ea589e3327c1cafd0e76b1, parent: 158ae9ee50d82b415fc9479575bc33f8985f4f1a)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (aa5a68b): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (secondary -0.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.6% [2.6%, 2.6%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.4% [-3.4%, -3.4%] 1
All ❌✅ (primary) - - 0

Cycles

Results (primary 1.9%, secondary -2.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
1.9% [1.9%, 1.9%] 1
Regressions ❌
(secondary)
2.4% [2.4%, 2.4%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.8% [-5.8%, -2.5%] 4
All ❌✅ (primary) 1.9% [1.9%, 1.9%] 1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 474.261s -> 473.66s (-0.13%)
Artifact size: 383.28 MiB -> 383.28 MiB (-0.00%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 19, 2026
@GuillaumeGomez
Copy link
Member Author

But it makes sense to allocate directly what you need and there is still one less string comparison. Well, whatever you prefer @yotamofek. Applied your suggestion as well.

@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez
Copy link
Member Author

Reverted the suggestion.

@@ -488,7 +490,15 @@ impl Hierarchy {
let files = self.elems.borrow();
let name = OrderedJson::serialize(self.elem.to_str().expect("invalid osstring conversion"))
.unwrap();
let mut out = Vec::from([name]);
let needed_capacity = if !files.is_empty() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change seems very verbose, considering it doesn't show on perf. Other two changes are IMHO improvements, regardless of perf.
Maybe discard just this one?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok!

@GuillaumeGomez
Copy link
Member Author

Removed the pre-computed vec size.

if let Some(parent) = h.parent.upgrade() {
h = parent;

while let Some(component) = components.next() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just noticed there's a very small change in behavior here.
previous version would panic if path.components() doesn't yield any items.
not sure if it's worth preserving? (you could add a assert!(components.peek().is_some()) before the loop)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Yeah let's go with an assert.

h.elems.borrow_mut().insert(s.to_owned());
break;
}
let next_h = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: you could just do h = { ... }, instead of creating the new binding. up to you.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One less variable and line, sounds good to me. :)

Comment on lines 534 to 538
Component::ParentDir => {
if let Some(parent) = h.parent.upgrade() {
h = parent;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, up to you:

Suggested change
Component::ParentDir => {
if let Some(parent) = h.parent.upgrade() {
h = parent;
}
}
Component::ParentDir if let Some(parent) = h.parent.upgrade() => {
h = parent;
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shorter, I like it.

@rustbot
Copy link
Collaborator

rustbot commented Jan 26, 2026

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@GuillaumeGomez
Copy link
Member Author

Applied improved code suggestions. :)

@yotamofek
Copy link
Contributor

r=me once CI passes, sorry for the long response time with this one :D

@GuillaumeGomez
Copy link
Member Author

@bors r=yotamofek rollup

@rust-bors
Copy link
Contributor

rust-bors bot commented Jan 26, 2026

📌 Commit bd45311 has been approved by yotamofek

It is now in the queue for this repository.

@rust-bors rust-bors bot 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 Jan 26, 2026
rust-bors bot pushed a commit that referenced this pull request Jan 26, 2026
…uwer

Rollup of 7 pull requests

Successful merges:

 - #151290 (Recover from struct literals with placeholder or empty path)
 - #148187 (Remove uses of `&mut CmResolver`)
 - #151368 (Rustdoc performance improvements)
 - #151374 (some more rustc_borrowck cleanups)
 - #151536 (Fix sanitizer target builds on CI)
 - #151626 (Remove `Deref<Target = TyCtxt>` from `QueryCtxt`)
 - #151661 (Suggest changing `iter`/`into_iter` when the other was meant)
rust-bors bot pushed a commit that referenced this pull request Jan 26, 2026
…uwer

Rollup of 7 pull requests

Successful merges:

 - #151290 (Recover from struct literals with placeholder or empty path)
 - #148187 (Remove uses of `&mut CmResolver`)
 - #151368 (Rustdoc performance improvements)
 - #151374 (some more rustc_borrowck cleanups)
 - #151536 (Fix sanitizer target builds on CI)
 - #151626 (Remove `Deref<Target = TyCtxt>` from `QueryCtxt`)
 - #151661 (Suggest changing `iter`/`into_iter` when the other was meant)
rust-bors bot pushed a commit that referenced this pull request Jan 26, 2026
…uwer

Rollup of 7 pull requests

Successful merges:

 - #151290 (Recover from struct literals with placeholder or empty path)
 - #148187 (Remove uses of `&mut CmResolver`)
 - #151368 (Rustdoc performance improvements)
 - #151374 (some more rustc_borrowck cleanups)
 - #151536 (Fix sanitizer target builds on CI)
 - #151626 (Remove `Deref<Target = TyCtxt>` from `QueryCtxt`)
 - #151661 (Suggest changing `iter`/`into_iter` when the other was meant)
rust-bors bot pushed a commit that referenced this pull request Jan 26, 2026
…uwer

Rollup of 7 pull requests

Successful merges:

 - #151290 (Recover from struct literals with placeholder or empty path)
 - #148187 (Remove uses of `&mut CmResolver`)
 - #151368 (Rustdoc performance improvements)
 - #151374 (some more rustc_borrowck cleanups)
 - #151536 (Fix sanitizer target builds on CI)
 - #151626 (Remove `Deref<Target = TyCtxt>` from `QueryCtxt`)
 - #151661 (Suggest changing `iter`/`into_iter` when the other was meant)
@rust-bors rust-bors bot merged commit 621c712 into rust-lang:main Jan 27, 2026
11 checks passed
@rustbot rustbot added this to the 1.95.0 milestone Jan 27, 2026
rust-timer added a commit that referenced this pull request Jan 27, 2026
Rollup merge of #151368 - GuillaumeGomez:librustdoc-perf, r=yotamofek

Rustdoc performance improvements

A few things I had sitting around for a while. Let's check what perf says about them

r? @yotamofek
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants