Describe why size_align have not been inlined so far

[tmiasko]

although it is used only in one place.

[rust-highfive]

r? @kennytm

(rust-highfive has picked a reviewer for you, use r? to override)

[Xanewok]

@bors r+ rollup

[bors]

📌 Commit dc7325e411910d8f325af84c8ad162408249ffa7 has been approved by Xanewok

[kennytm]

@bors r-

we tried once before in #72189 and failed, please show what has changed this time.

[kennytm]

@bors rollup=never

[Xanewok]

Oh, woops, sorry for the noise. It does look innocuous; might be worth adding a comment for posterity on why it's perf-sensitive/the way it is right now.

[Mark-Simulacrum]

r=me with a comment added, fwiw

[Mark-Simulacrum]

(i.e., a comment instead of the current change)

[Xanewok]

For completeness' sake:

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion

[bors]

⌛ Trying commit dc7325e411910d8f325af84c8ad162408249ffa7 with merge 69c935bbedf41328fef53fd877e4a48df302d2f5...

[bors]

☀️ Try build successful - checks-actions
Build commit: 69c935bbedf41328fef53fd877e4a48df302d2f5 (69c935bbedf41328fef53fd877e4a48df302d2f5)

[rust-timer]

Queued 69c935bbedf41328fef53fd877e4a48df302d2f5 with parent db85512, future comparison URL.

[rust-timer]

Finished benchmarking try commit (69c935bbedf41328fef53fd877e4a48df302d2f5): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot modify labels: +S-waiting-on-review -S-waiting-on-perf

[tmiasko]

Thanks for catching this @kennytm. What prompted me to made this change in the first place, was the number of codegen items created for size_align. In retrospect, it is not surprising that it could cause a shift in partitioning. While partitioning is generally stable, when it finally changes, the change tends to be substantial. Looking again at mono-items and their partitioning in a crate I was investigating, this indeed introduced quite a shift.

Given the number of codegen item, the effect on partitioning likely generalizes, although I see no reason to except impact on quality of produced code to be in any particular direction, but since this is not captured by rustc perf benchmarks (except for rustc), leaving this as is seems like a good decision for now.

I added a comment describing why the function wasn't inlined so far.

[kennytm]

@bors rollup

[kennytm]

r=me or Mark-Simulacrum after fixing comment

[bjorn3]

I wonder if calling the size_of and align_of intrinsics directly instead of the core::mem wrappers would fix the perf regression when inlining size_align.

[kennytm]

@bors r+ rollup

[bors]

📌 Commit cf5bd26 has been approved by kennytm

[bors]

⌛ Testing commit cf5bd26 with merge 05dfaba...

[bors]

☀️ Test successful - checks-actions
Approved by: kennytm
Pushing 05dfaba to master...