Prefer T::SIZE over size_of::<T>() in core#159922
Conversation
|
@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.
…, r=<try> Prefer `T::SIZE` over `size_of::<T>()` in `core`
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (6261a9a): 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 1.8%, secondary 2.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -3.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary -0.0%, secondary -0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 489.287s -> 487.746s (-0.31%) |
|
r? @JohnTitor rustbot has assigned @JohnTitor. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
r? @workingjubilee |
|
|
|
r? clarfonthey Since there appears to be a perf improvement, and there wasn't really any objection to doing this, I think we can just call this okay. @bors r+ rollup=iffy Modifies several different files, so, prone to merge conflicts. |
|
@bors r- Gonna preemptively cancel just because this isn't necessarily a big win. My thought process was that because this could have any performance impact, it would be nice to make them prevalent enough for people to know about them, but that is a kind of weak argument. Will wait for more discussion on Zulip. |
|
This pull request was unapproved. |
|
Yeah, given that the perf wins aren't really significant and that this would be quite some churn, I'll close the PR. |
Some context: #t-libs > Make more use of `SizedTypeProperties`
This only touches
coreand only changessize_of::<_>()->_::SIZEto keep the scope manageable.The motivation is mainly possible compile time wins and it's slightly more readable.
Since these were done mostly programmatically I will go over all changes again to make sure they are semantic no-ops and that no public docs were affected.