From 6ce76acae455a32113116cd2f95f8076388fc2d3 Mon Sep 17 00:00:00 2001 From: MaloJaffre Date: Wed, 22 Aug 2018 10:10:07 +0200 Subject: [PATCH 1/5] Slightly refactor VecDeque implementation --- src/liballoc/collections/vec_deque.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/liballoc/collections/vec_deque.rs b/src/liballoc/collections/vec_deque.rs index 571f35a2031d2..d49cb9857740d 100644 --- a/src/liballoc/collections/vec_deque.rs +++ b/src/liballoc/collections/vec_deque.rs @@ -1024,7 +1024,7 @@ impl VecDeque { iter: Iter { tail: drain_tail, head: drain_head, - ring: unsafe { self.buffer_as_mut_slice() }, + ring: unsafe { self.buffer_as_slice() }, }, } } @@ -2593,8 +2593,8 @@ impl From> for Vec { let mut right_offset = 0; for i in left_edge..right_edge { right_offset = (i - left_edge) % (cap - right_edge); - let src: isize = (right_edge + right_offset) as isize; - ptr::swap(buf.add(i), buf.offset(src)); + let src = right_edge + right_offset; + ptr::swap(buf.add(i), buf.add(src)); } let n_ops = right_edge - left_edge; left_edge += n_ops; From 11e488b64fed181820280d494d4fcc157ee1adc5 Mon Sep 17 00:00:00 2001 From: MaloJaffre Date: Wed, 22 Aug 2018 13:18:51 +0200 Subject: [PATCH 2/5] Optimize VecDeque::append --- src/liballoc/collections/vec_deque.rs | 29 +++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/src/liballoc/collections/vec_deque.rs b/src/liballoc/collections/vec_deque.rs index d49cb9857740d..7c16258e84ef0 100644 --- a/src/liballoc/collections/vec_deque.rs +++ b/src/liballoc/collections/vec_deque.rs @@ -202,6 +202,22 @@ impl VecDeque { len); } + /// Copies all values from `src` to `self`, wrapping around if needed. + /// Assumes capacity is sufficient. + #[inline] + unsafe fn copy_slice(&mut self, src: &[T]) { + let dst_high_ptr = self.ptr().add(self.head); + let dst_high_len = self.cap() - self.head; + + let split = cmp::min(src.len(), dst_high_len); + let (src_high, src_low) = src.split_at(split); + + ptr::copy_nonoverlapping(src_high.as_ptr(), dst_high_ptr, src_high.len()); + ptr::copy_nonoverlapping(src_low.as_ptr(), self.ptr(), src_low.len()); + + self.head = self.wrap_add(self.head, src.len()); + } + /// Copies a potentially wrapping block of memory len long from src to dest. /// (abs(dst - src) + len) must be no larger than cap() (There must be at /// most one continuous overlapping region between src and dest). @@ -1834,8 +1850,17 @@ impl VecDeque { #[inline] #[stable(feature = "append", since = "1.4.0")] pub fn append(&mut self, other: &mut Self) { - // naive impl - self.extend(other.drain(..)); + // Guarantees there is space in `self` for `other + self.reserve(other.len()); + + unsafe { + let (src_high, src_low) = other.as_slices(); + self.copy_slice(src_low); + self.copy_slice(src_high); + } + + // Some values now exist in both `other` and `self` but are made inaccessible in `other`. + other.tail = other.head; } /// Retains only the elements specified by the predicate. From 1a1a7f6167edf18b8a0ab488e651f7748cc2e9d3 Mon Sep 17 00:00:00 2001 From: MaloJaffre Date: Tue, 28 Aug 2018 15:38:56 +0200 Subject: [PATCH 3/5] Add docs and debug asserts --- src/liballoc/collections/vec_deque.rs | 34 ++++++++++++++++++--------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/src/liballoc/collections/vec_deque.rs b/src/liballoc/collections/vec_deque.rs index 7c16258e84ef0..1bd1861db0b27 100644 --- a/src/liballoc/collections/vec_deque.rs +++ b/src/liballoc/collections/vec_deque.rs @@ -202,10 +202,17 @@ impl VecDeque { len); } - /// Copies all values from `src` to `self`, wrapping around if needed. - /// Assumes capacity is sufficient. + /// Copies all values from `src` to the back of `self`, wrapping around if needed. + /// + /// # Safety + /// + /// The capacity must be sufficient to hold self.len() + src.len() elements. + /// If so, this function never panics. #[inline] unsafe fn copy_slice(&mut self, src: &[T]) { + let expected_new_len = self.len() + src.len(); + debug_assert!(self.capacity() >= expected_new_len); + let dst_high_ptr = self.ptr().add(self.head); let dst_high_len = self.cap() - self.head; @@ -216,6 +223,7 @@ impl VecDeque { ptr::copy_nonoverlapping(src_low.as_ptr(), self.ptr(), src_low.len()); self.head = self.wrap_add(self.head, src.len()); + debug_assert!(self.len() == expected_new_len); } /// Copies a potentially wrapping block of memory len long from src to dest. @@ -1850,17 +1858,21 @@ impl VecDeque { #[inline] #[stable(feature = "append", since = "1.4.0")] pub fn append(&mut self, other: &mut Self) { - // Guarantees there is space in `self` for `other - self.reserve(other.len()); - unsafe { - let (src_high, src_low) = other.as_slices(); - self.copy_slice(src_low); - self.copy_slice(src_high); - } + // Guarantees there is space in `self` for `other`. + self.reserve(other.len()); - // Some values now exist in both `other` and `self` but are made inaccessible in `other`. - other.tail = other.head; + { + let (src_high, src_low) = other.as_slices(); + + // This is only safe because copy_slice never panics when capacity is sufficient. + self.copy_slice(src_low); + self.copy_slice(src_high); + } + + // Some values now exist in both `other` and `self` but are made inaccessible in `other`. + other.tail = other.head; + } } /// Retains only the elements specified by the predicate. From 1908892d3f60008f265dfc25ac46db387c0ad6a0 Mon Sep 17 00:00:00 2001 From: MaloJaffre Date: Tue, 28 Aug 2018 15:59:21 +0200 Subject: [PATCH 4/5] Fix tidy --- src/liballoc/collections/vec_deque.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/liballoc/collections/vec_deque.rs b/src/liballoc/collections/vec_deque.rs index 1bd1861db0b27..7b6693268ae3f 100644 --- a/src/liballoc/collections/vec_deque.rs +++ b/src/liballoc/collections/vec_deque.rs @@ -1870,7 +1870,8 @@ impl VecDeque { self.copy_slice(src_high); } - // Some values now exist in both `other` and `self` but are made inaccessible in `other`. + // Some values now exist in both `other` and `self` but are made inaccessible + // in`other`. other.tail = other.head; } } From 21d2a6c9868541ec9829ced9a5bae936b18741c5 Mon Sep 17 00:00:00 2001 From: MaloJaffre Date: Wed, 29 Aug 2018 13:39:57 +0200 Subject: [PATCH 5/5] Add another assert --- src/liballoc/collections/vec_deque.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/liballoc/collections/vec_deque.rs b/src/liballoc/collections/vec_deque.rs index 7b6693268ae3f..c53549ab85d6d 100644 --- a/src/liballoc/collections/vec_deque.rs +++ b/src/liballoc/collections/vec_deque.rs @@ -19,6 +19,7 @@ use core::cmp::Ordering; use core::fmt; +use core::isize; use core::iter::{repeat, FromIterator, FusedIterator}; use core::mem; use core::ops::Bound::{Excluded, Included, Unbounded}; @@ -210,6 +211,9 @@ impl VecDeque { /// If so, this function never panics. #[inline] unsafe fn copy_slice(&mut self, src: &[T]) { + /// This is guaranteed by `RawVec`. + debug_assert!(self.capacity() <= isize::MAX as usize); + let expected_new_len = self.len() + src.len(); debug_assert!(self.capacity() >= expected_new_len);