Skip to content

Conversation

@yotamofek
Copy link
Contributor

@yotamofek yotamofek commented Oct 20, 2025

cc @GuillaumeGomez and @notriddle

please disregard the actual changes in my fork on stringdex, it's all "wip" commits and a bunch of different changes I was experimenting with..
I just wanna see if it actually has an effect on the rustc benchmarks (not just the stringdex benchmarks) to see if it's worth pursuing

@rustbot rustbot added A-tidy Area: The tidy tool S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) 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 Oct 20, 2025
@Kobzol
Copy link
Member

Kobzol commented Oct 20, 2025

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request Oct 20, 2025
[PERF] see if my fork of `stringdex` affects perf
@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 Oct 20, 2025
@rust-bors
Copy link

rust-bors bot commented Oct 21, 2025

💥 Test timed out after 21600s

@notriddle
Copy link
Contributor

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rust-bors

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request Oct 21, 2025
[PERF] see if my fork of `stringdex` affects perf
@rust-bors
Copy link

rust-bors bot commented Oct 21, 2025

☀️ Try build successful (CI)
Build commit: cc4f770 (cc4f7703e067e4e62f404b76a3a1bd7c1d67b4ec, parent: c7a635f33c5fbd1ead110243a2f4a5f0561d79b0)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (cc4f770): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

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.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.6% [0.6%, 0.6%] 1
Improvements ✅
(primary)
-0.4% [-0.9%, -0.1%] 12
Improvements ✅
(secondary)
-0.5% [-0.9%, -0.3%] 3
All ❌✅ (primary) -0.4% [-0.9%, -0.1%] 12

Max RSS (memory usage)

Results (secondary -4.0%)

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)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-4.0% [-4.0%, -4.0%] 1
All ❌✅ (primary) - - 0

Cycles

Results (primary -2.4%, secondary 2.0%)

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.0% [2.0%, 2.0%] 1
Improvements ✅
(primary)
-2.4% [-2.4%, -2.4%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -2.4% [-2.4%, -2.4%] 1

Binary size

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

Bootstrap: 472.842s -> 473.067s (0.05%)
Artifact size: 388.69 MiB -> 388.71 MiB (0.01%)

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Oct 21, 2025
@JonathanBrouwer
Copy link
Contributor

Performance looks excellent, the coercions result is clearly bimodal noise

@JonathanBrouwer JonathanBrouwer removed the perf-regression Performance regression. label Oct 21, 2025
@GuillaumeGomez
Copy link
Member

@yotamofek: Do you want to merge this one already?

@yotamofek
Copy link
Contributor Author

Nah, my changes to stringdex are a complete mess at the moment, I think it'll be better to first get a few of them up as PRs and have @notriddle go over them, and then released to crates.io. No point in depending on a fork when it's a crate that's owned by a project member.
(unless they don't have the capacity to review and/or work on stringdex)

@notriddle
Copy link
Contributor

It's fine. Send me the PR on GitLab, and I'll look at them ASAP.

@yotamofek
Copy link
Contributor Author

Still a WIP, but I've started PR-ing my changes and notriddle has merged a few of them already, and I'm interested in the interim perf results. Would love another perf run please 🙏
(the stringdex dep now points at the upstream repo, and not my fork)

@JonathanBrouwer
Copy link
Contributor

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rust-bors

This comment has been minimized.

@rust-bors
Copy link

rust-bors bot commented Nov 5, 2025

Unknown argument "cance". Run @bors2 help to see available commands.

@yotamofek
Copy link
Contributor Author

@bors try cancel

@rust-bors
Copy link

rust-bors bot commented Nov 5, 2025

Try build cancelled. Cancelled workflows:

@yotamofek
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rust-bors

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request Nov 5, 2025
[PERF] see if my fork of `stringdex` affects perf
@rust-bors
Copy link

rust-bors bot commented Nov 5, 2025

☀️ Try build successful (CI)
Build commit: d73a1bd (d73a1bddf44384c37e074411122571b40f44c339, parent: 8e0b68e63cd2b7b6d18474fe6f49df6fb1570c25)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (d73a1bd): comparison URL.

Overall result: ✅ improvements - 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

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.5% [-1.1%, -0.2%] 13
Improvements ✅
(secondary)
-0.5% [-1.3%, -0.1%] 4
All ❌✅ (primary) -0.5% [-1.1%, -0.2%] 13

Max RSS (memory usage)

Results (secondary -3.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)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.4% [-3.4%, -3.4%] 1
All ❌✅ (primary) - - 0

Cycles

Results (secondary 2.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)
3.9% [2.1%, 7.2%] 3
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.1% [-2.1%, -2.1%] 1
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 475.57s -> 475.357s (-0.04%)
Artifact size: 390.76 MiB -> 390.77 MiB (0.00%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Nov 5, 2025
@yotamofek
Copy link
Contributor Author

yotamofek commented Nov 5, 2025

I'm quite happy with these results, they're even better than the first perf run, but now all my changes to stringdex are properly separated into commits, and mostly merged.

@notriddle , once you approve the last PR, I think it would be a good point to release a new version of stringdex and call it a win. :)

@JonathanBrouwer
Copy link
Contributor

Yeah this performance improvement is awesome to see, thanks a lot for your effort!

@yotamofek
Copy link
Contributor Author

@bors try @rust-timer queue

Pushed a new version that takes a branch of stringdex that merges both PRs that are currently open (!11 and !12). Also added another commit to !11 since last perf run, which required some adaptations on the rustdoc side.

Gonna take advantage of the fact that the perf queue is really quiet right now to get another reading. Mostly for the sake of my curiosity, wanna see if I managed to squeeze out a few more green numbers until notriddle gets a chance to properly review everything :)

@rust-timer

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request Nov 6, 2025
[PERF] see if my fork of `stringdex` affects perf
@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 Nov 6, 2025
@rust-bors
Copy link

rust-bors bot commented Nov 6, 2025

☀️ Try build successful (CI)
Build commit: 5d905a8 (5d905a831657432c7b62345e6e710cc0ada801ef, parent: 642c19bfc3a5c1de985bf5d0cc8207ac9d22708a)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (5d905a8): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

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.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.1% [0.1%, 0.1%] 2
Improvements ✅
(primary)
-0.5% [-1.1%, -0.2%] 13
Improvements ✅
(secondary)
-0.5% [-1.3%, -0.1%] 4
All ❌✅ (primary) -0.5% [-1.1%, -0.2%] 13

Max RSS (memory usage)

Results (primary -1.7%, secondary -3.2%)

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)
3.4% [3.4%, 3.4%] 1
Improvements ✅
(primary)
-1.7% [-1.7%, -1.7%] 1
Improvements ✅
(secondary)
-3.8% [-5.3%, -1.7%] 11
All ❌✅ (primary) -1.7% [-1.7%, -1.7%] 1

Cycles

Results (secondary -1.8%)

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)
7.1% [3.6%, 10.6%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-6.2% [-8.7%, -2.6%] 4
All ❌✅ (primary) - - 0

Binary size

Results (secondary 0.0%)

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)
0.0% [0.0%, 0.0%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Bootstrap: 474.467s -> 474.025s (-0.09%)
Artifact size: 390.76 MiB -> 390.90 MiB (0.03%)

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Nov 6, 2025
@yotamofek
Copy link
Contributor Author

yotamofek commented Nov 6, 2025

Looks a tiny bit better when only looking at "Doc" profile (changes in other profiles are probably noise):

Previous run (avg. -0.49) vs last run (avg. -0.50).

@bors
Copy link
Collaborator

bors commented Nov 6, 2025

☔ The latest upstream changes (presumably #148573) made this pull request unmergeable. Please resolve the merge conflicts.

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

Labels

A-tidy Area: The tidy tool perf-regression Performance regression. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) 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.

8 participants