From c5fa7776dff913dd75fed6f4e7ed483f0e75e367 Mon Sep 17 00:00:00 2001 From: whitequark Date: Sun, 20 Sep 2015 11:04:01 +0300 Subject: [PATCH 1/3] Do not drop_in_place elements of Vec if T doesn't need dropping With -O2, LLVM's inliner can remove this code, but this does not happen with -O1 and lower. As a result, dropping Vec was linear with length, resulting in abysmal performance for large buffers. --- src/libcollections/vec.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/libcollections/vec.rs b/src/libcollections/vec.rs index c99460a55c952..fac846179430e 100644 --- a/src/libcollections/vec.rs +++ b/src/libcollections/vec.rs @@ -65,7 +65,7 @@ use alloc::heap::EMPTY; use core::cmp::Ordering; use core::fmt; use core::hash::{self, Hash}; -use core::intrinsics::{arith_offset, assume, drop_in_place}; +use core::intrinsics::{arith_offset, assume, drop_in_place, needs_drop}; use core::iter::FromIterator; use core::mem; use core::ops::{Index, IndexMut, Deref}; @@ -1322,8 +1322,10 @@ impl Drop for Vec { // OK because exactly when this stops being a valid assumption, we // don't need unsafe_no_drop_flag shenanigans anymore. if self.buf.unsafe_no_drop_flag_needs_drop() { - for x in self.iter_mut() { - unsafe { drop_in_place(x); } + if unsafe { needs_drop::() } { + for x in self.iter_mut() { + unsafe { drop_in_place(x); } + } } } // RawVec handles deallocation From 6beb4ba1aaecb59fdfdc676434b5a3b17a4b5bfc Mon Sep 17 00:00:00 2001 From: whitequark Date: Sun, 20 Sep 2015 22:15:39 +0300 Subject: [PATCH 2/3] Fix style. --- src/libcollections/vec.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/libcollections/vec.rs b/src/libcollections/vec.rs index fac846179430e..ddab16493f013 100644 --- a/src/libcollections/vec.rs +++ b/src/libcollections/vec.rs @@ -1322,9 +1322,11 @@ impl Drop for Vec { // OK because exactly when this stops being a valid assumption, we // don't need unsafe_no_drop_flag shenanigans anymore. if self.buf.unsafe_no_drop_flag_needs_drop() { - if unsafe { needs_drop::() } { - for x in self.iter_mut() { - unsafe { drop_in_place(x); } + unsafe { + if needs_drop::() { + for x in self.iter_mut() { + drop_in_place(x); + } } } } From 77f5da77a91af88130fcff3e726931934c71ba3f Mon Sep 17 00:00:00 2001 From: whitequark Date: Sun, 20 Sep 2015 22:19:30 +0300 Subject: [PATCH 3/3] Add comment. --- src/libcollections/vec.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libcollections/vec.rs b/src/libcollections/vec.rs index ddab16493f013..4110faa41b324 100644 --- a/src/libcollections/vec.rs +++ b/src/libcollections/vec.rs @@ -1323,6 +1323,8 @@ impl Drop for Vec { // don't need unsafe_no_drop_flag shenanigans anymore. if self.buf.unsafe_no_drop_flag_needs_drop() { unsafe { + // The branch on needs_drop() is an -O1 performance optimization. + // Without the branch, dropping Vec takes linear time. if needs_drop::() { for x in self.iter_mut() { drop_in_place(x);