From 9c59d04d55eccd39593844c19d3f0e988990e8ac Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 13 Apr 2022 15:01:35 +1000 Subject: [PATCH] Speed up Vec::clear(). Currently it just calls `truncate(0)`. `truncate()` is (a) not marked as `#[inline]`, and (b) more general than needed for `clear()`. This commit changes `clear()` to do the work itself. This modest change was first proposed in rust-lang#74172, where the reviewer rejected it because there was insufficient evidence that `Vec::clear()`'s performance mattered enough to justify the change. Recent changes within rustc have made `Vec::clear()` hot within `macro_parser.rs`, so the change is now clearly worthwhile. Although it doesn't show wins on CI perf runs, this seems to be because they use PGO. But not all platforms currently use PGO. Also, local builds don't use PGO, and `truncate` sometimes shows up in an over-represented fashion in local profiles. So local profiling will be made easier by this change. Note that this will also benefit `String::clear()`, because it just calls `Vec::clear()`. Finally, the commit removes the `vec-clear.rs` codegen test. It was added in #52908. From before then until now, `Vec::clear()` just called `Vec::truncate()` with a zero length. The body of Vec::truncate() has changed a lot since then. Now that `Vec::clear()` is doing actual work itself, and not just calling `Vec::truncate()`, it's not surprising that its generated code includes a load and an icmp. I think it's reasonable to remove this test. --- library/alloc/src/vec/mod.rs | 13 ++++++++++++- src/test/codegen/vec-clear.rs | 11 ----------- 2 files changed, 12 insertions(+), 12 deletions(-) delete mode 100644 src/test/codegen/vec-clear.rs diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index 74bcac2b541..8c2f52172ee 100644 --- a/library/alloc/src/vec/mod.rs +++ b/library/alloc/src/vec/mod.rs @@ -1881,7 +1881,18 @@ pub fn drain(&mut self, range: R) -> Drain<'_, T, A> #[inline] #[stable(feature = "rust1", since = "1.0.0")] pub fn clear(&mut self) { - self.truncate(0) + let elems: *mut [T] = self.as_mut_slice(); + + // SAFETY: + // - `elems` comes directly from `as_mut_slice` and is therefore valid. + // - Setting `self.len` before calling `drop_in_place` means that, + // if an element's `Drop` impl panics, the vector's `Drop` impl will + // do nothing (leaking the rest of the elements) instead of dropping + // some twice. + unsafe { + self.len = 0; + ptr::drop_in_place(elems); + } } /// Returns the number of elements in the vector, also referred to diff --git a/src/test/codegen/vec-clear.rs b/src/test/codegen/vec-clear.rs deleted file mode 100644 index 15bfe421e9d..00000000000 --- a/src/test/codegen/vec-clear.rs +++ /dev/null @@ -1,11 +0,0 @@ -// compile-flags: -O - -#![crate_type = "lib"] - -// CHECK-LABEL: @vec_clear -#[no_mangle] -pub fn vec_clear(x: &mut Vec) { - // CHECK-NOT: load - // CHECK-NOT: icmp - x.clear() -}