about summary refs log tree commit diff
diff options
context:
space:
mode:
authorNicholas Nethercote <n.nethercote@gmail.com>2022-04-13 15:01:35 +1000
committerNicholas Nethercote <n.nethercote@gmail.com>2022-04-13 15:39:35 +1000
commit9c59d04d55eccd39593844c19d3f0e988990e8ac (patch)
tree441077d22bf220e174de9119718eaa8cae1b9143
parent52ca603da73ae9eaddf96f77953b33ad8c47cc8e (diff)
downloadrust-9c59d04d55eccd39593844c19d3f0e988990e8ac.tar.gz
rust-9c59d04d55eccd39593844c19d3f0e988990e8ac.zip
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.
-rw-r--r--library/alloc/src/vec/mod.rs13
-rw-r--r--src/test/codegen/vec-clear.rs11
2 files changed, 12 insertions, 12 deletions
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 @@ impl<T, A: Allocator> Vec<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<u32>) {
-    // CHECK-NOT: load
-    // CHECK-NOT: icmp
-    x.clear()
-}