Uh oh!
There was an error while loading. Please reload this page.
librustdoc: create a helper for separating elements of an iterator instead of implementing it multiple times - #136244
Conversation
rustbot
commented
Jan 29, 2025
rustbot has assigned @GuillaumeGomez. Use |
GuillaumeGomez
commented
Jan 29, 2025
Seems very promising, thanks! Please tell me when you want to start the perf run. |
yotamofek
commented
Jan 29, 2025
Thanks! I'll ping you once CI checks are done (or can it be queued before they're finished? if so - go ahead please) |
GuillaumeGomez
commented
Jan 29, 2025
Unfortunately it's not queue, it's done on a separate server. So waiting for your ping then! |
yotamofek
commented
Jan 29, 2025
@GuillaumeGomez ping 😁 |
GuillaumeGomez
commented
Jan 29, 2025
Let's go! @bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
[WIP: perf] attempt to prevent regressions in that originally happened in rust-lang#135494 This implements something similar to [`Itertools::format`](https://docs.rs/itertools/latest/itertools/trait.Itertools.html#method.format), but on `Fn`s returning iterators instead of directly on iterators, to allow implementing `Display` without the use of a `Cell` (to handle the possibility of `fmt` being called multiple times while receiving `&self`). This is WIP, I just want to get a perf run first to see if the regression I saw in rust-lang#135494 is fixed
bors
commented
Jan 29, 2025
bors
commented
Jan 29, 2025
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
rust-timer
commented
Jan 29, 2025
Finished benchmarking commit (f5cefbb): comparison URL. Overall result: ❌ regressions - please read the text belowBenchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @bors rollup=never Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (secondary -1.3%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (secondary -2.3%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 775.532s -> 776.62s (0.14%) |
yotamofek
commented
Jan 29, 2025
Still regresses 😓 |
GuillaumeGomez
commented
Jan 29, 2025
Please ping me whenever you want to give it another try. 😉 |
yotamofek
commented
Jan 29, 2025
Ping 🙏 |
GuillaumeGomez
commented
Jan 29, 2025
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
bors
commented
Jan 29, 2025
[WIP: perf] attempt to prevent regressions in that originally happened in rust-lang#135494 This implements something similar to [`Itertools::format`](https://docs.rs/itertools/latest/itertools/trait.Itertools.html#method.format), but on `Fn`s returning iterators instead of directly on iterators, to allow implementing `Display` without the use of a `Cell` (to handle the possibility of `fmt` being called multiple times while receiving `&self`). This is WIP, I just want to get a perf run first to see if the regression I saw in rust-lang#135494 is fixed
bors
commented
Jan 29, 2025
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
rust-timer
commented
Jan 29, 2025
Finished benchmarking commit (e38cd65): comparison URL. Overall result: ❌ regressions - please read the text belowBenchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @bors rollup=never Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (primary 2.8%, secondary -1.3%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (secondary 9.2%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 776.896s -> 776.333s (-0.07%) |
GuillaumeGomez
commented
Feb 3, 2025
Thanks! @bors r+ rollup=iffy |
bors
commented
Feb 3, 2025
bors
commented
Feb 4, 2025
☔ The latest upstream changes (presumably #136507) made this pull request unmergeable. Please resolve the merge conflicts. |
…stead of implementing it multiple times
eb2635e to
cb028dcCompareyotamofek
commented
Feb 4, 2025
@rustbot review |
This comment was marked as resolved.
This comment was marked as resolved.
fmease
commented
Feb 4, 2025
Lol, oops. @bors r=GuillaumeGomez |
bors
commented
Feb 4, 2025
bors
commented
Feb 5, 2025
bors
commented
Feb 5, 2025
☀️ Test successful - checks-actions |
rust-timer
commented
Feb 5, 2025
Finished benchmarking commit (8df89d1): comparison URL. Overall result: ✅ improvements - no action needed@rustbot label: -perf-regression Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (primary 0.2%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 777.31s -> 776.632s (-0.09%) |
…<try> librustdoc: more usages of `Joined::joined` Some missed opportunities from rust-lang#136244 r? `@GuillaumeGomez` since you reviewed the last one (feel free to re-assign, of course 😊) First two commits are just drive-by cleanups
…r=GuillaumeGomez librustdoc: more usages of `Joined::joined` Some missed opportunities from rust-lang#136244 r? `@GuillaumeGomez` since you reviewed the last one (feel free to re-assign, of course 😊) First two commits are just drive-by cleanups
…r=GuillaumeGomez librustdoc: more usages of `Joined::joined` Some missed opportunities from rust-lang#136244 r? ``@GuillaumeGomez`` since you reviewed the last one (feel free to re-assign, of course 😊) First two commits are just drive-by cleanups
…r=GuillaumeGomez librustdoc: more usages of `Joined::joined` Some missed opportunities from rust-lang#136244 r? ```@GuillaumeGomez``` since you reviewed the last one (feel free to re-assign, of course 😊) First two commits are just drive-by cleanups
Rollup merge of rust-lang#136599 - yotamofek:pr/rustdoc-more-joined, r=GuillaumeGomez librustdoc: more usages of `Joined::joined` Some missed opportunities from rust-lang#136244 r? ```@GuillaumeGomez``` since you reviewed the last one (feel free to re-assign, of course 😊) First two commits are just drive-by cleanups
This implements something similar to
Itertools::format, but onFns returning iterators instead of directly on iterators, to allow implementingDisplaywithout the use of aCell(to handle the possibility offmtbeing called multiple times while receiving&self).This is WIP, I just want to get a perf run first to see if the regression I saw in #135494 is fixedThis was originally part of #135494 , but originally caused a perf regression that was since fixed:
rust/src/librustdoc/html/format.rs
Line 507 in 7d5ae18