Vec refactor and optimization of panic guards - #161426
Conversation
|
|
While consistency is nice, descriptiveness is even better β as such I think
In most cases,
As it is only called in catch-pads, I don't know how relevant this is. For these kinds of things it's always a good idea to look at the assembly and see whether your intuition is correct. |
|
It seems it reduces retain function body size at least in this godbolt test: https://godbolt.org/z/vGbP9P6WT |
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Vec refactor and optimization of panic guards
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (0a61e4d): comparison URL. Overall result: β improvements - no action neededBenchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up. @rustbot label: -S-waiting-on-perf -perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 2.8%, secondary -4.2%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 0.3%, secondary 2.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 476.343s -> 473.583s (-0.58%) |
|
β The latest upstream changes (presumably #161990) made this pull request unmergeable. Please resolve the merge conflicts by rebasing. |
Hey, this is a follow-up to our PR #149784.
I want to:
FillGapOnDropofdedup_bytoPanicGuard, IMHO this is a better name and matches theretain_mut.#[inline(never)]for drop of panic guard ofretain_mutin my original PR, I've added that.dedup_byand I've added both#[cold]and#[inline(never)]As drops are only triggered for panic case, It should not affect performance for non-panic common case.
I expect slight binary size reduction.
r? joboet