specialize slice::clone_from_slice() for T: Copy

[the8472]

No description provided.

[rust-highfive]

r? @Mark-Simulacrum

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

[tmiasko]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit 066a9e06aa0769beb3704ade5f2e1daa64ebddfc with merge 972d79cb1c7b66c63a2524437f366c15734b3234...

[bors]

☀️ Try build successful - checks-actions
Build commit: 972d79cb1c7b66c63a2524437f366c15734b3234 (972d79cb1c7b66c63a2524437f366c15734b3234)

[rust-timer]

Queued 972d79cb1c7b66c63a2524437f366c15734b3234 with parent 36ecbc9, future comparison URL.

[rust-timer]

Finished benchmarking try commit (972d79cb1c7b66c63a2524437f366c15734b3234): 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 label: +S-waiting-on-review -S-waiting-on-perf

[Mark-Simulacrum]

Could you squash the two commits into one?

The perf results are not entirely positive but I think that the PR here is worth its added complexity for the particular edge cases where this is likely to come up.

r=me with that done

[the8472]

The perf results are not entirely positive

We could try adding a few #[inline] and see if that makes a difference if the current results are concerning.

[Mark-Simulacrum]

Hm, I would expect the compiler to already inline - I'm not opposed to trying it though. I also noted that the copy and clone from slice impls use different assertions - copy calls a function on mismatch with track caller and cold; clone doesn't. I'm not actually sure that the track caller there is useful as copy_from_slice isn't annotated itself...

Would you be up for trying to add inline attributes to see what happens? It might be worthwhile to just manually inline the implementation of copy_from_slice and outline the length check from clone/copy cases perhaps, too...

[the8472]

Hm, I would expect the compiler to already inline - I'm not opposed to trying it though

According to the docs #[inline] has some impact on CGU partitioning which in turn has an impact on perf results.

https://doc.rust-lang.org/stable/nightly-rustc/rustc_mir/monomorphize/partitioning/index.html#a-note-on-inlining

Would you be up for trying to add inline attributes to see what happens?

Ok, I have added them in a way that should restore the previous state of things. I.e. the small wrapper methods get inlined and from a caller's perspective it would now look again like they would be calling the fat methods as they used to, which then may or may not get inlined too.

I also noted that the copy and clone from slice impls use different assertions - copy calls a function on mismatch with track caller and cold; clone doesn't.

According to the git history that's an optimization to make the hot path generate less assembly, so it hopefully wouldn't make things slower.

[Mark-Simulacrum]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit 4d71b1979ebc3573cee4e0c470169f6d59a12e57 with merge 6c8b6ee1c0ff2064726c1633a3d7505606398b29...

[bors]

☀️ Try build successful - checks-actions
Build commit: 6c8b6ee1c0ff2064726c1633a3d7505606398b29 (6c8b6ee1c0ff2064726c1633a3d7505606398b29)

[rust-timer]

Queued 6c8b6ee1c0ff2064726c1633a3d7505606398b29 with parent ea09825, future comparison URL.

[rust-timer]

Finished benchmarking try commit (6c8b6ee1c0ff2064726c1633a3d7505606398b29): 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 label: +S-waiting-on-review -S-waiting-on-perf

[the8472]

I must have rebased in the meantime so a direct comparison of the commits doesn't seem possible. But it doesn't look significantly different anyway. Should I remove the latest commit?

[the8472]

Removed the last commit since it had no significant impact.

A bit odd that CI runs again on the same hash.

[Mark-Simulacrum]

Ok, I'm going to conclude that the perf results are noise if anything - they seem pretty minor in terms of changes, and the wall times are similarly largely unchanged. @bors r+ rollup=never

[bors]

📌 Commit 130fb24 has been approved by Mark-Simulacrum

[bors]

⌛ Testing commit 130fb24 with merge 3158857...

[bors]

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 3158857 to master...