From 77901f0f6edff2f913fe453577c622659b27bfe5 Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Sun, 23 Feb 2020 16:20:32 +0100 Subject: [PATCH 1/4] refactor From> for Vec --- src/liballoc/collections/vec_deque.rs | 134 ++++++++++++-------- src/liballoc/collections/vec_deque/tests.rs | 83 +++++++++++- 2 files changed, 163 insertions(+), 54 deletions(-) diff --git a/src/liballoc/collections/vec_deque.rs b/src/liballoc/collections/vec_deque.rs index 2cc450bb68a20..90ba94ad7986e 100644 --- a/src/liballoc/collections/vec_deque.rs +++ b/src/liballoc/collections/vec_deque.rs @@ -409,6 +409,81 @@ impl VecDeque { } } + /// Rearranges this deque so it is one continuous slice. + #[inline] + fn make_continuous(&mut self) { + if self.is_contiguous() { + return; + } + + let buf = self.buf.ptr(); + let cap = self.cap(); + let len = self.len(); + + let free = self.tail - self.head; + let tail_len = cap - self.tail; + + if free >= tail_len { + // from: DEFGH....ABC + // to: ABCDEFGH.... + unsafe { + ptr::copy(buf, buf.add(tail_len), self.head); + // ...DEFGH.ABC + ptr::copy(buf.add(self.tail), buf, tail_len); + // ABCDEFGH.... + + self.tail = 0; + self.head = len; + } + } else if free >= self.head { + // from: FGH....ABCDE + // to: ...ABCDEFGH. + unsafe { + ptr::copy(buf.add(self.tail), buf.add(self.head), tail_len); + // FGHABCDE.... + ptr::copy(buf, buf.add(self.head + tail_len), self.head); + // ...ABCDEFGH. + + self.tail = self.head; + self.head = self.tail + len; + } + } else { + // free is smaller than both head and tail, + // this means we have to slowly "swap" the tail and the head. + // + // from: EFGHI...ABCD or HIJK.ABCDEFG + // to: ABCDEFGHI... or ABCDEFGHIJK. + let mut left_edge: usize = 0; + let mut right_edge: usize = self.tail; + unsafe { + // The general problem looks like this + // GHIJKLM...ABCDEF - before any swaps + // ABCDEFM...GHIJKL - after 1 pass of swaps + // ABCDEFGHIJM...KL - swap until the left edge reaches the temp store + // - then restart the algorithm with a new (smaller) store + // Sometimes the temp store is reached when the right edge is at the end + // of the buffer - this means we've hit the right order with fewer swaps! + // E.g + // EF..ABCD + // ABCDEF.. - after four only swaps we've finished + while left_edge < len && right_edge != cap { + 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 n_ops = right_edge - left_edge; + left_edge += n_ops; + right_edge += right_offset + 1; + } + + self.tail = 0; + self.head = len; + } + } + } + /// Frobs the head and tail sections around to handle the fact that we /// just reallocated. Unsafe because it trusts old_capacity. #[inline] @@ -2889,63 +2964,16 @@ impl From> for Vec { /// assert_eq!(vec, [8, 9, 1, 2, 3, 4]); /// assert_eq!(vec.as_ptr(), ptr); /// ``` - fn from(other: VecDeque) -> Self { + fn from(mut other: VecDeque) -> Self { + other.make_continuous(); + unsafe { let buf = other.buf.ptr(); let len = other.len(); - let tail = other.tail; - let head = other.head; let cap = other.cap(); - - // Need to move the ring to the front of the buffer, as vec will expect this. - if other.is_contiguous() { - ptr::copy(buf.add(tail), buf, len); - } else { - if (tail - head) >= cmp::min(cap - tail, head) { - // There is enough free space in the centre for the shortest block so we can - // do this in at most three copy moves. - if (cap - tail) > head { - // right hand block is the long one; move that enough for the left - ptr::copy(buf.add(tail), buf.add(tail - head), cap - tail); - // copy left in the end - ptr::copy(buf, buf.add(cap - head), head); - // shift the new thing to the start - ptr::copy(buf.add(tail - head), buf, len); - } else { - // left hand block is the long one, we can do it in two! - ptr::copy(buf, buf.add(cap - tail), head); - ptr::copy(buf.add(tail), buf, cap - tail); - } - } else { - // Need to use N swaps to move the ring - // We can use the space at the end of the ring as a temp store - - let mut left_edge: usize = 0; - let mut right_edge: usize = tail; - - // The general problem looks like this - // GHIJKLM...ABCDEF - before any swaps - // ABCDEFM...GHIJKL - after 1 pass of swaps - // ABCDEFGHIJM...KL - swap until the left edge reaches the temp store - // - then restart the algorithm with a new (smaller) store - // Sometimes the temp store is reached when the right edge is at the end - // of the buffer - this means we've hit the right order with fewer swaps! - // E.g - // EF..ABCD - // ABCDEF.. - after four only swaps we've finished - - while left_edge < len && right_edge != cap { - 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 n_ops = right_edge - left_edge; - left_edge += n_ops; - right_edge += right_offset + 1; - } - } + + if other.head != 0 { + ptr::copy(buf.add(other.tail), buf, len); } let out = Vec::from_raw_parts(buf, len, cap); mem::forget(other); diff --git a/src/liballoc/collections/vec_deque/tests.rs b/src/liballoc/collections/vec_deque/tests.rs index f2ce5b1d15dde..bd41a46658c18 100644 --- a/src/liballoc/collections/vec_deque/tests.rs +++ b/src/liballoc/collections/vec_deque/tests.rs @@ -1,6 +1,6 @@ use super::*; -use ::test; +use test; #[bench] #[cfg_attr(miri, ignore)] // Miri does not support benchmarks @@ -130,6 +130,87 @@ fn test_insert() { } } +#[test] +fn make_continuous_big_tail() { + let mut tester = VecDeque::with_capacity(15); + + for i in 0..3 { + tester.push_back(i); + } + + for i in 3..10 { + tester.push_front(i); + } + + // 012......9876543 + assert_eq!(tester.capacity(), 15); + assert_eq!((&[9, 8, 7, 6, 5, 4, 3] as &[_], &[0, 1, 2] as &[_]), tester.as_slices()); + + let expected_start = tester.head; + tester.make_continuous(); + assert_eq!(tester.tail, expected_start); + assert_eq!((&[9, 8, 7, 6, 5, 4, 3, 0, 1, 2] as &[_], &[] as &[_]), tester.as_slices()); +} + +#[test] +fn make_continuous_big_head() { + let mut tester = VecDeque::with_capacity(15); + + for i in 0..8 { + tester.push_back(i); + } + + for i in 8..10 { + tester.push_front(i); + } + + // 01234567......98 + let expected_start = 0; + tester.make_continuous(); + assert_eq!(tester.tail, expected_start); + assert_eq!((&[9, 8, 0, 1, 2, 3, 4, 5, 6, 7] as &[_], &[] as &[_]), tester.as_slices()); +} + +#[test] +fn make_continuous_small_free() { + let mut tester = VecDeque::with_capacity(15); + + for i in 'A' as u8..'I' as u8 { + tester.push_back(i as char); + } + + for i in 'I' as u8..'N' as u8 { + tester.push_front(i as char); + } + + // ABCDEFGH...MLKJI + let expected_start = 0; + tester.make_continuous(); + assert_eq!(tester.tail, expected_start); + assert_eq!( + (&['M', 'L', 'K', 'J', 'I', 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H'] as &[_], &[] as &[_]), + tester.as_slices() + ); + + tester.clear(); + for i in 'I' as u8..'N' as u8 { + tester.push_back(i as char); + } + + for i in 'A' as u8..'I' as u8 { + tester.push_front(i as char); + } + + // IJKLM...HGFEDCBA + let expected_start = 0; + tester.make_continuous(); + assert_eq!(tester.tail, expected_start); + assert_eq!( + (&['H', 'G', 'F', 'E', 'D', 'C', 'B', 'A', 'I', 'J', 'K', 'L', 'M'] as &[_], &[] as &[_]), + tester.as_slices() + ); +} + #[test] fn test_remove() { // This test checks that every single combination of tail position, length, and From 4aa96520443ed838febce99623be1abd5d7d4e6c Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Sun, 23 Feb 2020 18:56:13 +0100 Subject: [PATCH 2/4] impl `sort_by` for VecDeque --- src/liballoc/collections/vec_deque.rs | 51 ++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/src/liballoc/collections/vec_deque.rs b/src/liballoc/collections/vec_deque.rs index 90ba94ad7986e..93238675cc980 100644 --- a/src/liballoc/collections/vec_deque.rs +++ b/src/liballoc/collections/vec_deque.rs @@ -2141,6 +2141,55 @@ impl VecDeque { } } + /// Sorts the deque with a comparator function. + /// + /// This sort is stable (i.e., does not reorder equal elements) and `O(n log n)` worst-case. + /// + /// The comparator function must define a total ordering for the elements in the deque. If + /// the ordering is not total, the order of the elements is unspecified. An order is a + /// total order if it is (for all `a`, `b` and `c`): + /// + /// * total and antisymmetric: exactly one of `a < b`, `a == b` or `a > b` is true, and + /// * transitive, `a < b` and `b < c` implies `a < c`. The same must hold for both `==` and `>`. + /// + /// When applicable, unstable sorting is preferred because it is generally faster than stable + /// sorting and it doesn't allocate auxiliary memory. + /// See [`sort_unstable_by`](#method.sort_unstable_by). + /// + /// # Current implementation + /// + /// The current implementation moves all elements of the internal buffer into one continous slice without + /// allocating, which is then sorted using [`slice::sort_by`] (which may allocate). + /// + /// # Examples + /// + /// ``` + /// #![feature(vecdeque_sort)] + /// use std::collections::VecDeque; + /// + /// let mut v: VecDeque<_> = [5, 4, 1, 3, 2].iter().collect(); + /// + /// v.sort_by(|a, b| a.cmp(b)); + /// let expected: VecDeque<_> = [1, 2, 3, 4, 5].iter().collect(); + /// assert_eq!(v, expected); + /// + /// // reverse sorting + /// v.sort_by(|a, b| b.cmp(a)); + /// let expected: VecDeque<_> = [5, 4, 3, 2, 1].iter().collect(); + /// assert_eq!(v, expected); + /// ``` + /// + /// [`slice::sort_by`]: https://doc.rust-lang.org/std/primitive.slice.html#method.sort_by + #[unstable(feature = "vecdeque_sort", issue = "none")] + pub fn sort_by(&mut self, compare: F) + where + F: FnMut(&T, &T) -> Ordering, + { + self.make_continuous(); + + self.as_mut_slices().0.sort_by(compare); + } + /// Rotates the double-ended queue `k` places to the right. /// /// Equivalently, @@ -2971,7 +3020,7 @@ impl From> for Vec { let buf = other.buf.ptr(); let len = other.len(); let cap = other.cap(); - + if other.head != 0 { ptr::copy(buf.add(other.tail), buf, len); } From 1c81ef5b0729ed2c13fac92a3bffc8d1528e95f0 Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Sun, 23 Feb 2020 18:56:47 +0100 Subject: [PATCH 3/4] add ignore-tidy-filelength --- src/liballoc/collections/vec_deque.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/liballoc/collections/vec_deque.rs b/src/liballoc/collections/vec_deque.rs index 93238675cc980..d207cbdf3740c 100644 --- a/src/liballoc/collections/vec_deque.rs +++ b/src/liballoc/collections/vec_deque.rs @@ -1,3 +1,4 @@ +// ignore-tidy-filelength //! A double-ended queue implemented with a growable ring buffer. //! //! This queue has `O(1)` amortized inserts and removals from both ends of the From afa87342769f910e56b2dceff986c9d9ce5e86fa Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Sun, 23 Feb 2020 23:04:49 +0100 Subject: [PATCH 4/4] temporarily remove link to `sort_unstable_by` --- 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 d207cbdf3740c..1206c7b693e6c 100644 --- a/src/liballoc/collections/vec_deque.rs +++ b/src/liballoc/collections/vec_deque.rs @@ -2155,7 +2155,8 @@ impl VecDeque { /// /// When applicable, unstable sorting is preferred because it is generally faster than stable /// sorting and it doesn't allocate auxiliary memory. - /// See [`sort_unstable_by`](#method.sort_unstable_by). + /// + /// FIXME: add link to `sort_unstable_by`. /// /// # Current implementation ///