Override clone_from for some types

[a1phyr]

Override clone_from method of the Clone trait for:

• cell::RefCell
• cmp::Reverse
• io::Cursor
• mem::ManuallyDrop

This can bring performance improvements.

[rust-highfive]

r? @yaahc

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

[Xanewok]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit 9332ac3 with merge ee90cb22627d2f2c6828fc4316225d34b4b8bd1a...

[bors]

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

[rust-timer]

Queued ee90cb22627d2f2c6828fc4316225d34b4b8bd1a with parent 5c02926, future comparison URL.

[rust-timer]

Finished benchmarking try commit (ee90cb22627d2f2c6828fc4316225d34b4b8bd1a): 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

[yaahc]

So am I correct in understanding that the changes here don't represent direct optimizations, but instead stop these wrappers from ignoring clone_from impls of inner types, so if the inner type depends heavily on a custom clone_from for performance reasons these wrappers will not be in the way after this change? I was surprised that the perf test didn't show any real improvement on a change that's ostensibly all about improving perf but now I feel like I understand, assuming my assessment is correct.

If that's the case I'd be happy to accept this PR but I think it might be better to try to address this problem in the Clone derive itself, which would have much further reaching impact and would stop us from needing to do these small changes in the future.

[a1phyr]

So am I correct in understanding that the changes here don't represent direct optimizations, but instead stop these wrappers from ignoring clone_from impls of inner types, so if the inner type depends heavily on a custom clone_from for performance reasons these wrappers will not be in the way after this change?

Yes, a common example is an io::Cursor<Vec<u8>>. With this PR, usingclone_from avoids allocating a new buffer and dropping the old one.

I was surprised that the perf test didn't show any real improvement on a change that's ostensibly all about improving perf but now I feel like I understand, assuming my assessment is correct.

This optimization is a bit niche, to be fair. I don't think that rustc uses clone_from for these specific types a lot, so I didn't expect this PR to improve compile times. It could really improve some other programs though, and it has a very small cost.

If that's the case I'd be happy to accept this PR but I think it might be better to try to address this problem in the Clone derive itself, which would have much further reaching impact and would stop us from needing to do these small changes in the future.

It was proposed a long time ago (#27939), but it was closed due to the impact on compile times. With this PR, clone_from should be overridden for most of the remaining interesting types (except tuples, looks like I forgot them).

[yaahc]

It was proposed a long time ago (#27939), but it was closed due to the impact on compile times. With this PR, clone_from should be overridden for most of the remaining interesting types (except tuples, looks like I forgot them).

Aah, unfortunate. Thanks for checking the background.

@bors r+

[bors]

📌 Commit 9332ac3 has been approved by yaahc

[bors]

⌛ Testing commit 9332ac3 with merge fb84a8750d1f6cd2bdb8a2b96d4fb67a6997616c...

[rust-log-analyzer]

The job aarch64-gnu failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

* highest error code: E0783
Found 517 error codes
Found 0 error codes with no tests
Done!
thread '<unnamed>' panicked at 'cmd.exec() failed with Error during execution of `cargo metadata`:     Updating git repository `https://github.com/bjorn3/rust-ar.git`
    Updating git submodule `https://github.com/WebAssembly/WASI`
warning: spurious network error (2 tries remaining): failed to connect to github.com: Connection refused; class=Os (2)
warning: spurious network error (1 tries remaining): failed to connect to github.com: Connection refused; class=Os (2)
warning: spurious network error (1 tries remaining): failed to connect to github.com: Connection refused; class=Os (2)
error: failed to get `cranelift-codegen` as a dependency of package `rustc_codegen_cranelift v0.1.0 (/checkout/compiler/rustc_codegen_cranelift)`
Caused by:
  failed to load source for dependency `cranelift-codegen`

Caused by:
Caused by:
  Unable to update https://github.com/bytecodealliance/wasmtime/?branch=main#45bee40f

Caused by:
  failed to update submodule `WASI`

Caused by:
  failed to fetch submodule `WASI` from https://github.com/WebAssembly/WASI
Caused by:
  failed to connect to github.com: Connection refused; class=Os (2)
', src/tools/tidy/src/deps.rs:306:20
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Any', src/tools/tidy/src/main.rs:88:9


command did not execute successfully: "/checkout/obj/build/aarch64-unknown-linux-gnu/stage0-tools-bin/rust-tidy" "/checkout" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage0/bin/cargo" "/checkout/obj/build" "14"


failed to run: /checkout/obj/build/bootstrap/debug/bootstrap --stage 2 test
Build completed unsuccessfully in 0:05:25

[bors]

💔 Test failed - checks-actions

[a1phyr]

Looks like a spurious network error.
@bors retry

[bors]

@a1phyr: 🔑 Insufficient privileges: not in try users

[Xanewok]

warning: spurious network error (2 tries remaining): failed to connect to github.com: Connection refused; class=Os (2)
warning: spurious network error (1 tries remaining): failed to connect to github.com: Connection refused; class=Os (2)
warning: spurious network error (1 tries remaining): failed to connect to github.com: Connection refused; class=Os (2)

@bors retry

[bors]

⌛ Testing commit 9332ac3 with merge 3d31363...

[bors]

☀️ Test successful - checks-actions
Approved by: yaahc
Pushing 3d31363 to master...