Skip to content

Commit

Permalink
Speed up Vec::clear().
Browse files Browse the repository at this point in the history
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 rust-lang#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.
  • Loading branch information
nnethercote committed Apr 13, 2022
1 parent 52ca603 commit 9c59d04
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 12 deletions.
13 changes: 12 additions & 1 deletion library/alloc/src/vec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 0 additions & 11 deletions src/test/codegen/vec-clear.rs

This file was deleted.

0 comments on commit 9c59d04

Please sign in to comment.