From 076417e978d86676bc403bb7ed963c107a8f72db Mon Sep 17 00:00:00 2001 From: The8472 Date: Fri, 11 Oct 2019 20:42:32 +0200 Subject: [PATCH 01/63] unrelated typo fix --- library/alloc/src/vec.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index 27eefb0584216..719ee8a3b0fc3 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -3042,7 +3042,7 @@ where old_len: usize, /// The filter test predicate. pred: F, - /// A flag that indicates a panic has occurred in the filter test prodicate. + /// A flag that indicates a panic has occurred in the filter test predicate. /// This is used as a hint in the drop implementation to prevent consumption /// of the remainder of the `DrainFilter`. Any unprocessed items will be /// backshifted in the `vec`, but no further items will be dropped or From 038394a330f24e5ec63d78657a32f3279b8b276b Mon Sep 17 00:00:00 2001 From: The8472 Date: Sat, 19 Oct 2019 00:00:45 +0200 Subject: [PATCH 02/63] bench --- library/alloc/benches/vec.rs | 41 +++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/library/alloc/benches/vec.rs b/library/alloc/benches/vec.rs index 4e71eec03e5bf..2eee988dccc80 100644 --- a/library/alloc/benches/vec.rs +++ b/library/alloc/benches/vec.rs @@ -1,5 +1,5 @@ use std::iter::{repeat, FromIterator}; -use test::Bencher; +use test::{black_box, Bencher}; #[bench] fn bench_new(b: &mut Bencher) { @@ -431,3 +431,42 @@ fn bench_clone_from_10_0100_0010(b: &mut Bencher) { fn bench_clone_from_10_1000_0100(b: &mut Bencher) { do_bench_clone_from(b, 10, 1000, 100) } + +macro_rules! bench_in_place { + ( + $($fname:ident, $type:ty , $count:expr, $init: expr);* + ) => { + $( + #[bench] + fn $fname(b: &mut Bencher) { + b.iter(|| { + let src: Vec<$type> = vec![$init; $count]; + black_box(src.into_iter() + .enumerate() + .map(|(idx, e)| { (idx as $type) ^ e }).collect::>()) + }); + } + )+ + }; +} + +bench_in_place![ + bench_in_place_xxu8_i0_0010, u8, 10, 0; + bench_in_place_xxu8_i0_0100, u8, 100, 0; + bench_in_place_xxu8_i0_1000, u8, 1000, 0; + bench_in_place_xxu8_i1_0010, u8, 10, 1; + bench_in_place_xxu8_i1_0100, u8, 100, 1; + bench_in_place_xxu8_i1_1000, u8, 1000, 1; + bench_in_place_xu32_i0_0010, u32, 10, 0; + bench_in_place_xu32_i0_0100, u32, 100, 0; + bench_in_place_xu32_i0_1000, u32, 1000, 0; + bench_in_place_xu32_i1_0010, u32, 10, 1; + bench_in_place_xu32_i1_0100, u32, 100, 1; + bench_in_place_xu32_i1_1000, u32, 1000, 1; + bench_in_place_u128_i0_0010, u128, 10, 0; + bench_in_place_u128_i0_0100, u128, 100, 0; + bench_in_place_u128_i0_1000, u128, 1000, 0; + bench_in_place_u128_i1_0010, u128, 10, 1; + bench_in_place_u128_i1_0100, u128, 100, 1; + bench_in_place_u128_i1_1000, u128, 1000, 1 +]; From bb2d533bb9b5c247f48f7932f7e533475b59e402 Mon Sep 17 00:00:00 2001 From: The8472 Date: Fri, 11 Oct 2019 20:43:25 +0200 Subject: [PATCH 03/63] in-place collect for Vec. Box<[]> and BinaryHeap IntoIter and some adapters --- library/alloc/src/collections/binary_heap.rs | 15 +- library/alloc/src/lib.rs | 1 + library/alloc/src/vec.rs | 138 +++++++++++++------ library/alloc/tests/binary_heap.rs | 12 ++ library/alloc/tests/lib.rs | 1 + library/alloc/tests/slice.rs | 9 ++ library/alloc/tests/vec.rs | 22 +++ library/core/src/iter/adapters/mod.rs | 106 +++++++++++++- library/core/src/iter/mod.rs | 5 + library/core/src/iter/traits/marker.rs | 12 ++ library/core/src/iter/traits/mod.rs | 2 + 11 files changed, 280 insertions(+), 43 deletions(-) diff --git a/library/alloc/src/collections/binary_heap.rs b/library/alloc/src/collections/binary_heap.rs index 477a598ff5b00..f7012a0342528 100644 --- a/library/alloc/src/collections/binary_heap.rs +++ b/library/alloc/src/collections/binary_heap.rs @@ -145,7 +145,7 @@ #![stable(feature = "rust1", since = "1.0.0")] use core::fmt; -use core::iter::{FromIterator, FusedIterator, TrustedLen}; +use core::iter::{FromIterator, FusedIterator, InPlaceIterable, SourceIter, TrustedLen}; use core::mem::{self, size_of, swap, ManuallyDrop}; use core::ops::{Deref, DerefMut}; use core::ptr; @@ -1173,6 +1173,19 @@ impl ExactSizeIterator for IntoIter { #[stable(feature = "fused", since = "1.26.0")] impl FusedIterator for IntoIter {} +#[unstable(issue = "0", feature = "inplace_iteration")] +impl SourceIter for IntoIter { + type Source = crate::vec::IntoIter; + + #[inline] + fn as_inner(&mut self) -> &mut Self::Source { + &mut self.iter + } +} + +#[unstable(issue = "0", feature = "inplace_iteration")] +unsafe impl InPlaceIterable for IntoIter {} + #[unstable(feature = "binary_heap_into_iter_sorted", issue = "59278")] #[derive(Clone, Debug)] pub struct IntoIterSorted { diff --git a/library/alloc/src/lib.rs b/library/alloc/src/lib.rs index 755a21934f0c5..72aa7fea4cfc3 100644 --- a/library/alloc/src/lib.rs +++ b/library/alloc/src/lib.rs @@ -99,6 +99,7 @@ #![feature(fmt_internals)] #![feature(fn_traits)] #![feature(fundamental)] +#![feature(inplace_iteration)] #![feature(internal_uninit_const)] #![feature(lang_items)] #![feature(layout_for_ptr)] diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index 719ee8a3b0fc3..ad639ca320ae1 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -58,7 +58,7 @@ use core::cmp::{self, Ordering}; use core::fmt; use core::hash::{Hash, Hasher}; use core::intrinsics::{arith_offset, assume}; -use core::iter::{FromIterator, FusedIterator, TrustedLen}; +use core::iter::{FromIterator, FusedIterator, InPlaceIterable, SourceIter, TrustedLen}; use core::marker::PhantomData; use core::mem::{self, ManuallyDrop, MaybeUninit}; use core::ops::Bound::{Excluded, Included, Unbounded}; @@ -2012,7 +2012,7 @@ impl> IndexMut for Vec { impl FromIterator for Vec { #[inline] fn from_iter>(iter: I) -> Vec { - >::from_iter(iter.into_iter()) + >::from_iter(iter.into_iter()) } } @@ -2094,13 +2094,12 @@ impl Extend for Vec { } } -// Specialization trait used for Vec::from_iter and Vec::extend -trait SpecExtend { +// Specialization trait used for Vec::from_iter +trait SpecFrom { fn from_iter(iter: I) -> Self; - fn spec_extend(&mut self, iter: I); } -impl SpecExtend for Vec +impl SpecFrom for Vec where I: Iterator, { @@ -2125,7 +2124,86 @@ where as SpecExtend>::spec_extend(&mut vector, iterator); vector } +} + +fn from_into_iter_source(mut iterator: I) -> Vec +where + I: Iterator + InPlaceIterable + SourceIter>, +{ + let mut insert_pos = 0; + + // FIXME: how to drop values written into source when iteration panics? + // tail already gets cleaned by IntoIter::drop + while let Some(item) = iterator.next() { + let source_iter = iterator.as_inner(); + let src_buf = source_iter.buf.as_ptr(); + let src_idx = source_iter.ptr; + unsafe { + let dst = src_buf.offset(insert_pos as isize); + debug_assert!( + dst as *const _ < src_idx, + "InPlaceIterable implementation produced more\ + items than it consumed from the source" + ); + ptr::write(dst, item) + } + insert_pos += 1; + } + + let src = iterator.as_inner(); + let vec = unsafe { Vec::from_raw_parts(src.buf.as_ptr(), insert_pos, src.cap) }; + mem::forget(iterator); + vec +} + +impl SpecFrom> for Vec { + fn from_iter(iterator: IntoIter) -> Self { + // A common case is passing a vector into a function which immediately + // re-collects into a vector. We can short circuit this if the IntoIter + // has not been advanced at all. + if iterator.buf.as_ptr() as *const _ == iterator.ptr { + unsafe { + let it = ManuallyDrop::new(iterator); + return Vec::from_raw_parts(it.buf.as_ptr(), it.len(), it.cap); + } + } + + from_into_iter_source(iterator) + } +} + +// Further specialization potential once lattice specialization exists +// and https://github.com/rust-lang/rust/issues/62645 has been solved: +// This can be broadened to only require size and alignment equality between +// input and output Item types. +impl SpecFrom for Vec +where + I: Iterator + InPlaceIterable + SourceIter>, +{ + default fn from_iter(iterator: I) -> Self { + from_into_iter_source(iterator) + } +} + +impl<'a, T: 'a, I> SpecFrom<&'a T, I> for Vec +where + I: Iterator, + T: Clone, +{ + default fn from_iter(iterator: I) -> Self { + SpecFrom::from_iter(iterator.cloned()) + } +} +// Specialization trait used for Vec::extend +trait SpecExtend { + fn spec_extend(&mut self, iter: I); +} + +impl SpecExtend for Vec +where + I: Iterator, +{ default fn spec_extend(&mut self, iter: I) { self.extend_desugared(iter) } @@ -2135,12 +2213,6 @@ impl SpecExtend for Vec where I: TrustedLen, { - default fn from_iter(iterator: I) -> Self { - let mut vector = Vec::new(); - vector.spec_extend(iterator); - vector - } - default fn spec_extend(&mut self, iterator: I) { // This is the case for a TrustedLen iterator. let (low, high) = iterator.size_hint(); @@ -2170,40 +2242,11 @@ where } } -impl SpecExtend> for Vec { - fn from_iter(iterator: IntoIter) -> Self { - // A common case is passing a vector into a function which immediately - // re-collects into a vector. We can short circuit this if the IntoIter - // has not been advanced at all. - if iterator.buf.as_ptr() as *const _ == iterator.ptr { - unsafe { - let it = ManuallyDrop::new(iterator); - Vec::from_raw_parts(it.buf.as_ptr(), it.len(), it.cap) - } - } else { - let mut vector = Vec::new(); - vector.spec_extend(iterator); - vector - } - } - - fn spec_extend(&mut self, mut iterator: IntoIter) { - unsafe { - self.append_elements(iterator.as_slice() as _); - } - iterator.ptr = iterator.end; - } -} - impl<'a, T: 'a, I> SpecExtend<&'a T, I> for Vec where I: Iterator, T: Clone, { - default fn from_iter(iterator: I) -> Self { - SpecExtend::from_iter(iterator.cloned()) - } - default fn spec_extend(&mut self, iterator: I) { self.spec_extend(iterator.cloned()) } @@ -2779,6 +2822,19 @@ unsafe impl<#[may_dangle] T> Drop for IntoIter { } } +#[unstable(issue = "0", feature = "inplace_iteration")] +unsafe impl InPlaceIterable for IntoIter {} + +#[unstable(issue = "0", feature = "inplace_iteration")] +impl SourceIter for IntoIter { + type Source = IntoIter; + + #[inline] + fn as_inner(&mut self) -> &mut Self::Source { + self + } +} + /// A draining iterator for `Vec`. /// /// This `struct` is created by [`Vec::drain`]. diff --git a/library/alloc/tests/binary_heap.rs b/library/alloc/tests/binary_heap.rs index 62084ccf53c59..ce794a9a4afa2 100644 --- a/library/alloc/tests/binary_heap.rs +++ b/library/alloc/tests/binary_heap.rs @@ -230,6 +230,18 @@ fn test_to_vec() { check_to_vec(vec![5, 4, 3, 2, 1, 5, 4, 3, 2, 1, 5, 4, 3, 2, 1]); } +#[test] +fn test_in_place_iterator_specialization() { + let src: Vec = vec![1, 2, 3]; + let src_ptr = src.as_ptr(); + let heap: BinaryHeap<_> = src.into_iter().map(std::convert::identity).collect(); + let heap_ptr = heap.iter().next().unwrap() as *const usize; + assert_eq!(src_ptr, heap_ptr); + let sink: Vec<_> = heap.into_iter().map(std::convert::identity).collect(); + let sink_ptr = sink.as_ptr(); + assert_eq!(heap_ptr, sink_ptr); +} + #[test] fn test_empty_pop() { let mut heap = BinaryHeap::::new(); diff --git a/library/alloc/tests/lib.rs b/library/alloc/tests/lib.rs index f2ba1ab64810b..e0e146dc42754 100644 --- a/library/alloc/tests/lib.rs +++ b/library/alloc/tests/lib.rs @@ -14,6 +14,7 @@ #![feature(slice_ptr_get)] #![feature(split_inclusive)] #![feature(binary_heap_retain)] +#![feature(inplace_iteration)] use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher}; diff --git a/library/alloc/tests/slice.rs b/library/alloc/tests/slice.rs index 3c7d57f8b32a8..1f561bebd908b 100644 --- a/library/alloc/tests/slice.rs +++ b/library/alloc/tests/slice.rs @@ -1459,6 +1459,15 @@ fn test_to_vec() { assert_eq!(ys, [1, 2, 3]); } +#[test] +fn test_in_place_iterator_specialization() { + let src: Box<[usize]> = box [1, 2, 3]; + let src_ptr = src.as_ptr(); + let sink: Box<_> = src.into_vec().into_iter().map(std::convert::identity).collect(); + let sink_ptr = sink.as_ptr(); + assert_eq!(src_ptr, sink_ptr); +} + #[test] fn test_box_slice_clone() { let data = vec![vec![0, 1], vec![0], vec![1]]; diff --git a/library/alloc/tests/vec.rs b/library/alloc/tests/vec.rs index ffff543b07fe5..271a705cf0631 100644 --- a/library/alloc/tests/vec.rs +++ b/library/alloc/tests/vec.rs @@ -1,6 +1,7 @@ use std::borrow::Cow; use std::collections::TryReserveError::*; use std::fmt::Debug; +use std::iter::InPlaceIterable; use std::mem::size_of; use std::panic::{catch_unwind, AssertUnwindSafe}; use std::vec::{Drain, IntoIter}; @@ -775,6 +776,27 @@ fn test_into_iter_leak() { assert_eq!(unsafe { DROPS }, 3); } +#[test] +fn test_from_iter_specialization() { + let src: Vec = vec![0usize; 1]; + let srcptr = src.as_ptr(); + let sink = src.into_iter().collect::>(); + let sinkptr = sink.as_ptr(); + assert_eq!(srcptr, sinkptr); +} + +#[test] +fn test_from_iter_specialization_with_iterator_adapters() { + fn assert_in_place_trait(_: &T) {}; + let src: Vec = vec![0usize; 65535]; + let srcptr = src.as_ptr(); + let iter = src.into_iter().enumerate().map(|i| i.0 + i.1).peekable().skip(1); + assert_in_place_trait(&iter); + let sink = iter.collect::>(); + let sinkptr = sink.as_ptr(); + assert_eq!(srcptr, sinkptr); +} + #[test] fn test_cow_from() { let borrowed: &[_] = &["borrowed", "(slice)"]; diff --git a/library/core/src/iter/adapters/mod.rs b/library/core/src/iter/adapters/mod.rs index f32c3963abea2..c7488ef272064 100644 --- a/library/core/src/iter/adapters/mod.rs +++ b/library/core/src/iter/adapters/mod.rs @@ -4,7 +4,9 @@ use crate::intrinsics; use crate::ops::{Add, AddAssign, ControlFlow, Try}; use super::from_fn; -use super::{DoubleEndedIterator, ExactSizeIterator, FusedIterator, Iterator, TrustedLen}; +use super::{ + DoubleEndedIterator, ExactSizeIterator, FusedIterator, InPlaceIterable, Iterator, TrustedLen, +}; mod chain; mod flatten; @@ -19,6 +21,40 @@ use self::zip::try_get_unchecked; pub(crate) use self::zip::TrustedRandomAccess; pub use self::zip::Zip; +/// This trait provides access to to the backing source of an interator-adapter pipeline +/// under the conditions that +/// * the iterator source `S` itself implements `SourceIter` +/// * there is a delegating implementation of this trait for each adapter in the pipeline +/// +/// This is useful for specializing [`FromIterator`] implementations or to retrieve +/// the remaining values from a source of a partially consumed iterator. +/// +/// # Examples +/// +/// Retrieving a partially consumed source and wrapping it into a different pipeline: +/// +/// ``` +/// # #![feature(inplace_iteration)] +/// # use std::iter::SourceIter; +/// +/// let mut iter = vec![9, 9, 9].into_iter().map(|i| i * i); +/// let first = iter.next().unwrap(); +/// let mut remainder = std::mem::replace(iter.as_inner(), Vec::new().into_iter()); +/// let second = remainder.map(|i| i + 1).next().unwrap(); +/// assert_eq!(first, 81); +/// assert_eq!(second, 10); +/// ``` +/// +/// [`FromIterator`]: trait.FromIterator.html +#[unstable(issue = "0", feature = "inplace_iteration")] +pub trait SourceIter { + /// The source iterator of the adapter. + type Source: Iterator; + + /// Recursively extract the source of an iterator pipeline. + fn as_inner(&mut self) -> &mut Self::Source; +} + /// A double-ended iterator with the direction inverted. /// /// This `struct` is created by the [`rev`] method on [`Iterator`]. See its @@ -939,6 +975,23 @@ where } } +#[unstable(issue = "0", feature = "inplace_iteration")] +impl SourceIter for Map +where + F: FnMut(I::Item) -> B, + I: SourceIter, +{ + type Source = S; + + #[inline] + fn as_inner(&mut self) -> &mut S { + SourceIter::as_inner(&mut self.iter) + } +} + +#[unstable(issue = "0", feature = "inplace_iteration")] +unsafe impl InPlaceIterable for Map where F: FnMut(I::Item) -> B {} + /// An iterator that filters the elements of `iter` with `predicate`. /// /// This `struct` is created by the [`filter`] method on [`Iterator`]. See its @@ -1414,6 +1467,22 @@ impl FusedIterator for Enumerate where I: FusedIterator {} #[unstable(feature = "trusted_len", issue = "37572")] unsafe impl TrustedLen for Enumerate where I: TrustedLen {} +#[unstable(issue = "0", feature = "inplace_iteration")] +impl SourceIter for Enumerate +where + I: SourceIter, +{ + type Source = S; + + #[inline] + fn as_inner(&mut self) -> &mut S { + SourceIter::as_inner(&mut self.iter) + } +} + +#[unstable(issue = "0", feature = "inplace_iteration")] +unsafe impl InPlaceIterable for Enumerate {} + /// An iterator with a `peek()` that returns an optional reference to the next /// element. /// @@ -1692,6 +1761,25 @@ impl Peekable { } } +#[unstable(feature = "trusted_len", issue = "37572")] +unsafe impl TrustedLen for Peekable where I: TrustedLen {} + +#[unstable(issue = "0", feature = "inplace_iteration")] +impl SourceIter for Peekable +where + I: SourceIter, +{ + type Source = S; + + #[inline] + fn as_inner(&mut self) -> &mut S { + SourceIter::as_inner(&mut self.iter) + } +} + +#[unstable(issue = "0", feature = "inplace_iteration")] +unsafe impl InPlaceIterable for Peekable {} + /// An iterator that rejects elements while `predicate` returns `true`. /// /// This `struct` is created by the [`skip_while`] method on [`Iterator`]. See its @@ -2167,6 +2255,22 @@ where #[stable(feature = "fused", since = "1.26.0")] impl FusedIterator for Skip where I: FusedIterator {} +#[unstable(issue = "0", feature = "inplace_iteration")] +impl SourceIter for Skip +where + I: SourceIter, +{ + type Source = S; + + #[inline] + fn as_inner(&mut self) -> &mut S { + SourceIter::as_inner(&mut self.iter) + } +} + +#[unstable(issue = "0", feature = "inplace_iteration")] +unsafe impl InPlaceIterable for Skip {} + /// An iterator that only iterates over the first `n` iterations of `iter`. /// /// This `struct` is created by the [`take`] method on [`Iterator`]. See its diff --git a/library/core/src/iter/mod.rs b/library/core/src/iter/mod.rs index bab8dda2915d9..f35994560c56a 100644 --- a/library/core/src/iter/mod.rs +++ b/library/core/src/iter/mod.rs @@ -342,6 +342,9 @@ pub use self::traits::{DoubleEndedIterator, Extend, FromIterator, IntoIterator}; #[stable(feature = "rust1", since = "1.0.0")] pub use self::traits::{ExactSizeIterator, Product, Sum}; +#[unstable(issue = "0", feature = "inplace_iteration")] +pub use self::traits::InPlaceIterable; + #[stable(feature = "iter_cloned", since = "1.1.0")] pub use self::adapters::Cloned; #[stable(feature = "iter_copied", since = "1.36.0")] @@ -350,6 +353,8 @@ pub use self::adapters::Copied; pub use self::adapters::Flatten; #[unstable(feature = "iter_map_while", reason = "recently added", issue = "68537")] pub use self::adapters::MapWhile; +#[unstable(issue = "0", feature = "inplace_iteration")] +pub use self::adapters::SourceIter; #[stable(feature = "iterator_step_by", since = "1.28.0")] pub use self::adapters::StepBy; #[stable(feature = "rust1", since = "1.0.0")] diff --git a/library/core/src/iter/traits/marker.rs b/library/core/src/iter/traits/marker.rs index 3c893c039923e..549f7972689e2 100644 --- a/library/core/src/iter/traits/marker.rs +++ b/library/core/src/iter/traits/marker.rs @@ -42,3 +42,15 @@ pub unsafe trait TrustedLen: Iterator {} #[unstable(feature = "trusted_len", issue = "37572")] unsafe impl TrustedLen for &mut I {} + +/// An iterator that when yielding an item will have taken at least one element +/// from its underlying [`SourceIter`]. +/// +/// Calling next() guarantees that at least one value of the iterator's underlying source +/// has been moved out and the result of the iterator chain could be inserted in its place, +/// assuming structural constraints of the source allow such an insertion. +/// In other words this trait indicates that an iterator pipeline can be collected in place. +/// +/// [`SourceIter`]: ../../std/iter/trait.SourceIter.html +#[unstable(issue = "0", feature = "inplace_iteration")] +pub unsafe trait InPlaceIterable: Iterator {} diff --git a/library/core/src/iter/traits/mod.rs b/library/core/src/iter/traits/mod.rs index efd1580a54807..9ed2de7313df1 100644 --- a/library/core/src/iter/traits/mod.rs +++ b/library/core/src/iter/traits/mod.rs @@ -11,5 +11,7 @@ pub use self::double_ended::DoubleEndedIterator; pub use self::exact_size::ExactSizeIterator; #[stable(feature = "rust1", since = "1.0.0")] pub use self::iterator::Iterator; +#[unstable(issue = "0", feature = "inplace_iteration")] +pub use self::marker::InPlaceIterable; #[stable(feature = "rust1", since = "1.0.0")] pub use self::marker::{FusedIterator, TrustedLen}; From 2a327394e4ae84600f506079fddd0e2bb1959b06 Mon Sep 17 00:00:00 2001 From: The8472 Date: Fri, 15 Nov 2019 01:18:30 +0100 Subject: [PATCH 04/63] mark SourceIter as unsafe, document invariants --- library/alloc/src/collections/binary_heap.rs | 2 +- library/alloc/src/vec.rs | 2 +- library/core/src/iter/adapters/mod.rs | 66 +++++++++++++++----- 3 files changed, 52 insertions(+), 18 deletions(-) diff --git a/library/alloc/src/collections/binary_heap.rs b/library/alloc/src/collections/binary_heap.rs index f7012a0342528..6154d03226fd9 100644 --- a/library/alloc/src/collections/binary_heap.rs +++ b/library/alloc/src/collections/binary_heap.rs @@ -1174,7 +1174,7 @@ impl ExactSizeIterator for IntoIter { impl FusedIterator for IntoIter {} #[unstable(issue = "0", feature = "inplace_iteration")] -impl SourceIter for IntoIter { +unsafe impl SourceIter for IntoIter { type Source = crate::vec::IntoIter; #[inline] diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index ad639ca320ae1..e7d61dcde45a2 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2826,7 +2826,7 @@ unsafe impl<#[may_dangle] T> Drop for IntoIter { unsafe impl InPlaceIterable for IntoIter {} #[unstable(issue = "0", feature = "inplace_iteration")] -impl SourceIter for IntoIter { +unsafe impl SourceIter for IntoIter { type Source = IntoIter; #[inline] diff --git a/library/core/src/iter/adapters/mod.rs b/library/core/src/iter/adapters/mod.rs index c7488ef272064..7a48026a847fe 100644 --- a/library/core/src/iter/adapters/mod.rs +++ b/library/core/src/iter/adapters/mod.rs @@ -21,37 +21,71 @@ use self::zip::try_get_unchecked; pub(crate) use self::zip::TrustedRandomAccess; pub use self::zip::Zip; -/// This trait provides access to to the backing source of an interator-adapter pipeline +/// This trait provides transitive access to source-stages in an interator-adapter pipeline /// under the conditions that /// * the iterator source `S` itself implements `SourceIter` -/// * there is a delegating implementation of this trait for each adapter in the pipeline +/// * there is a delegating implementation of this trait for each adapter in the pipeline between +/// the source and the pipeline consumer. /// -/// This is useful for specializing [`FromIterator`] implementations or to retrieve -/// the remaining values from a source of a partially consumed iterator. +/// When the source is an owning iterator struct (commonly called `IntoIter`) then +/// this can be useful for specializing [`FromIterator`] implementations or recovering the +/// remaining elements after an iterator has been partially exhausted. +/// +/// Note that implementations do not necessarily have to provide access to the inner-most +/// source of a pipeline. A stateful intermediate adapter might eagerly evaluate a part +/// of the pipeline and expose its internal storage as source. +/// +/// The trait is unsafe because implementers must uphold additional safety properties. +/// See [`as_inner`] for details. /// /// # Examples /// -/// Retrieving a partially consumed source and wrapping it into a different pipeline: +/// Retrieving a partially consumed source: /// /// ``` /// # #![feature(inplace_iteration)] /// # use std::iter::SourceIter; /// /// let mut iter = vec![9, 9, 9].into_iter().map(|i| i * i); -/// let first = iter.next().unwrap(); +/// let _ = iter.next(); /// let mut remainder = std::mem::replace(iter.as_inner(), Vec::new().into_iter()); -/// let second = remainder.map(|i| i + 1).next().unwrap(); -/// assert_eq!(first, 81); -/// assert_eq!(second, 10); +/// println!("n = {} elements remaining", remainder.len()); /// ``` /// /// [`FromIterator`]: trait.FromIterator.html +/// [`as_inner`]: #method.as_inner #[unstable(issue = "0", feature = "inplace_iteration")] -pub trait SourceIter { - /// The source iterator of the adapter. +pub unsafe trait SourceIter { + /// A source stage in an iterator pipeline. type Source: Iterator; - /// Recursively extract the source of an iterator pipeline. + /// Extract the source of an iterator pipeline. + /// + /// Callers may assume that calls to [`next()`] or any method taking `&self` + /// does no replace the referenced value. + /// But callers may replace the referenced values as long they in turn do not + /// expose it through a delegating implementation of this trait. + /// Which means that while adapters may not modify the reference they cannot + /// rely on it not being modified. + /// + /// Adapters must not rely on exclusive ownership or immutability of the source. + /// For example a peeking adapter could either exploit [`TrustedRandomAccess`] to look ahead + /// or implement this trait, but it cannot do both because a caller could call `next()` or any + /// other mutating method on the source between iteration steps and thus invalidate the peeked + /// values. + /// The lack of exclusive ownership also requires that adapters must uphold the source's + /// public API even when they have crate- or module-internal access. + /// + /// Callers in turn must expect the source to be in any state that is consistent with + /// its public API since adapters sitting between it and the source have the same + /// access. In particular an adapter may have consumed more elements than strictly necessary. + /// + /// The overall goal of these requirements is to grant the consumer of a pipeline + /// access to the underlying storage of an iterator while restricting any statefulness + /// and side-effects of the pipeline stages from affecting or relying on that storage. + /// + /// [`TrustedRandomAccess`]: trait.TrustedRandomAccess.html + /// [`next`]: trait.Iterator.html#method.next fn as_inner(&mut self) -> &mut Self::Source; } @@ -976,7 +1010,7 @@ where } #[unstable(issue = "0", feature = "inplace_iteration")] -impl SourceIter for Map +unsafe impl SourceIter for Map where F: FnMut(I::Item) -> B, I: SourceIter, @@ -1468,7 +1502,7 @@ impl FusedIterator for Enumerate where I: FusedIterator {} unsafe impl TrustedLen for Enumerate where I: TrustedLen {} #[unstable(issue = "0", feature = "inplace_iteration")] -impl SourceIter for Enumerate +unsafe impl SourceIter for Enumerate where I: SourceIter, { @@ -1765,7 +1799,7 @@ impl Peekable { unsafe impl TrustedLen for Peekable where I: TrustedLen {} #[unstable(issue = "0", feature = "inplace_iteration")] -impl SourceIter for Peekable +unsafe impl SourceIter for Peekable where I: SourceIter, { @@ -2256,7 +2290,7 @@ where impl FusedIterator for Skip where I: FusedIterator {} #[unstable(issue = "0", feature = "inplace_iteration")] -impl SourceIter for Skip +unsafe impl SourceIter for Skip where I: SourceIter, { From 73a982e9ecc81ad40b3c7f35ef6a4387140fa12f Mon Sep 17 00:00:00 2001 From: The8472 Date: Fri, 15 Nov 2019 01:27:41 +0100 Subject: [PATCH 05/63] assert that SourceIter requirements have not been violated by the pipeline --- library/alloc/src/vec.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index e7d61dcde45a2..b21f2a908440d 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2131,12 +2131,14 @@ where I: Iterator + InPlaceIterable + SourceIter>, { let mut insert_pos = 0; + let original_ptr = iterator.as_inner().buf.as_ptr(); // FIXME: how to drop values written into source when iteration panics? // tail already gets cleaned by IntoIter::drop while let Some(item) = iterator.next() { let source_iter = iterator.as_inner(); let src_buf = source_iter.buf.as_ptr(); + debug_assert_eq!(original_ptr, src_buf); let src_idx = source_iter.ptr; unsafe { let dst = src_buf.offset(insert_pos as isize); From 88b7ae642cbf1968452f63dca32a3c9567d4d8ff Mon Sep 17 00:00:00 2001 From: The8472 Date: Fri, 15 Nov 2019 02:37:55 +0100 Subject: [PATCH 06/63] implement drop handling --- library/alloc/src/vec.rs | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index b21f2a908440d..a056f2abcd60e 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2126,22 +2126,37 @@ where } } +struct InPlaceIterFront { + inner: *mut T, + count: usize, + did_panic: bool, +} + +impl Drop for InPlaceIterFront { + #[inline] + fn drop(&mut self) { + unsafe { + if mem::needs_drop::() && self.did_panic { + ptr::drop_in_place(slice::from_raw_parts_mut(self.inner, self.count) as *mut _); + } + } + } +} + fn from_into_iter_source(mut iterator: I) -> Vec where I: Iterator + InPlaceIterable + SourceIter>, { - let mut insert_pos = 0; let original_ptr = iterator.as_inner().buf.as_ptr(); + let mut front_buffer = InPlaceIterFront { inner: original_ptr, count: 0, did_panic: true }; - // FIXME: how to drop values written into source when iteration panics? - // tail already gets cleaned by IntoIter::drop while let Some(item) = iterator.next() { let source_iter = iterator.as_inner(); let src_buf = source_iter.buf.as_ptr(); debug_assert_eq!(original_ptr, src_buf); let src_idx = source_iter.ptr; unsafe { - let dst = src_buf.offset(insert_pos as isize); + let dst = src_buf.offset(front_buffer.count as isize); debug_assert!( dst as *const _ < src_idx, "InPlaceIterable implementation produced more\ @@ -2149,12 +2164,16 @@ where ); ptr::write(dst, item) } - insert_pos += 1; + front_buffer.count += 1; } let src = iterator.as_inner(); - let vec = unsafe { Vec::from_raw_parts(src.buf.as_ptr(), insert_pos, src.cap) }; - mem::forget(iterator); + front_buffer.did_panic = false; + let vec = unsafe { Vec::from_raw_parts(src.buf.as_ptr(), front_buffer.count, src.cap) }; + src.cap = 0; + src.buf = unsafe { NonNull::new_unchecked(RawVec::NEW.ptr()) }; + src.ptr = src.buf.as_ptr(); + src.end = src.buf.as_ptr(); vec } From 328a75f766ed51aea6787c19c5df8f3e36183849 Mon Sep 17 00:00:00 2001 From: The8472 Date: Fri, 15 Nov 2019 03:54:44 +0100 Subject: [PATCH 07/63] use add instead of offset --- library/alloc/src/vec.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index a056f2abcd60e..68d8bba62118d 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2156,7 +2156,7 @@ where debug_assert_eq!(original_ptr, src_buf); let src_idx = source_iter.ptr; unsafe { - let dst = src_buf.offset(front_buffer.count as isize); + let dst = src_buf.add(front_buffer.count); debug_assert!( dst as *const _ < src_idx, "InPlaceIterable implementation produced more\ From f904d0339a1e3f83fd99fe3d780cd07dac6dd11d Mon Sep 17 00:00:00 2001 From: The8472 Date: Fri, 15 Nov 2019 21:01:13 +0100 Subject: [PATCH 08/63] fix doc link --- library/core/src/iter/adapters/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/iter/adapters/mod.rs b/library/core/src/iter/adapters/mod.rs index 7a48026a847fe..7149bb1db052e 100644 --- a/library/core/src/iter/adapters/mod.rs +++ b/library/core/src/iter/adapters/mod.rs @@ -85,7 +85,7 @@ pub unsafe trait SourceIter { /// and side-effects of the pipeline stages from affecting or relying on that storage. /// /// [`TrustedRandomAccess`]: trait.TrustedRandomAccess.html - /// [`next`]: trait.Iterator.html#method.next + /// [`next()`]: trait.Iterator.html#method.next fn as_inner(&mut self) -> &mut Self::Source; } From bead910b211d3a3dcb6aa39e93b5d6bad2a92ff4 Mon Sep 17 00:00:00 2001 From: The8472 Date: Fri, 15 Nov 2019 21:02:54 +0100 Subject: [PATCH 09/63] simplify pointer arithmetic --- library/alloc/src/vec.rs | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index 68d8bba62118d..a81514e96486d 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2128,16 +2128,22 @@ where struct InPlaceIterFront { inner: *mut T, - count: usize, + dst: *mut T, did_panic: bool, } +impl InPlaceIterFront { + unsafe fn len(&self) -> usize { + self.dst.offset_from(self.inner) as usize + } +} + impl Drop for InPlaceIterFront { #[inline] fn drop(&mut self) { unsafe { if mem::needs_drop::() && self.did_panic { - ptr::drop_in_place(slice::from_raw_parts_mut(self.inner, self.count) as *mut _); + ptr::drop_in_place(slice::from_raw_parts_mut(self.inner, self.len()) as *mut _); } } } @@ -2148,28 +2154,26 @@ where I: Iterator + InPlaceIterable + SourceIter>, { let original_ptr = iterator.as_inner().buf.as_ptr(); - let mut front_buffer = InPlaceIterFront { inner: original_ptr, count: 0, did_panic: true }; + let mut front_buffer = + InPlaceIterFront { inner: original_ptr, dst: original_ptr, did_panic: true }; while let Some(item) = iterator.next() { let source_iter = iterator.as_inner(); - let src_buf = source_iter.buf.as_ptr(); - debug_assert_eq!(original_ptr, src_buf); - let src_idx = source_iter.ptr; + debug_assert_eq!(original_ptr, source_iter.buf.as_ptr()); unsafe { - let dst = src_buf.add(front_buffer.count); debug_assert!( - dst as *const _ < src_idx, + front_buffer.dst as *const _ < source_iter.ptr, "InPlaceIterable implementation produced more\ items than it consumed from the source" ); - ptr::write(dst, item) + ptr::write(front_buffer.dst, item); + front_buffer.dst = front_buffer.dst.add(1); } - front_buffer.count += 1; } let src = iterator.as_inner(); front_buffer.did_panic = false; - let vec = unsafe { Vec::from_raw_parts(src.buf.as_ptr(), front_buffer.count, src.cap) }; + let vec = unsafe { Vec::from_raw_parts(src.buf.as_ptr(), front_buffer.len(), src.cap) }; src.cap = 0; src.buf = unsafe { NonNull::new_unchecked(RawVec::NEW.ptr()) }; src.ptr = src.buf.as_ptr(); From 6c5c47b82b982d385eaff1d959bf716925f096f6 Mon Sep 17 00:00:00 2001 From: The8472 Date: Sat, 16 Nov 2019 17:27:39 +0100 Subject: [PATCH 10/63] update benches --- library/alloc/benches/vec.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/library/alloc/benches/vec.rs b/library/alloc/benches/vec.rs index 2eee988dccc80..6a3d704612644 100644 --- a/library/alloc/benches/vec.rs +++ b/library/alloc/benches/vec.rs @@ -440,10 +440,11 @@ macro_rules! bench_in_place { #[bench] fn $fname(b: &mut Bencher) { b.iter(|| { - let src: Vec<$type> = vec![$init; $count]; - black_box(src.into_iter() + let src: Vec<$type> = black_box(vec![$init; $count]); + let mut sink = src.into_iter() .enumerate() - .map(|(idx, e)| { (idx as $type) ^ e }).collect::>()) + .map(|(idx, e)| { (idx as $type) ^ e }).collect::>(); + black_box(sink.as_mut_ptr()) }); } )+ From 232065074d5cbc15065860abe4d7fbc8e765ad1d Mon Sep 17 00:00:00 2001 From: The8472 Date: Sat, 16 Nov 2019 22:31:23 +0100 Subject: [PATCH 11/63] recover vectorization switch to try_fold and segregate the drop handling to keep collect::>() and similar optimizer-friendly It comes at the cost of less accurate debug_asserts and code complexity --- library/alloc/src/lib.rs | 1 + library/alloc/src/vec.rs | 76 ++++++++++++++++++++++++++++------------ 2 files changed, 54 insertions(+), 23 deletions(-) diff --git a/library/alloc/src/lib.rs b/library/alloc/src/lib.rs index 72aa7fea4cfc3..e755afe9606f1 100644 --- a/library/alloc/src/lib.rs +++ b/library/alloc/src/lib.rs @@ -107,6 +107,7 @@ #![feature(map_first_last)] #![feature(map_into_keys_values)] #![feature(negative_impls)] +#![feature(never_type)] #![feature(new_uninit)] #![feature(nll)] #![feature(nonnull_slice_from_raw_parts)] diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index a81514e96486d..a5caf2522c8f4 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2126,23 +2126,23 @@ where } } -struct InPlaceIterFront { +struct InPlaceDrop { inner: *mut T, dst: *mut T, did_panic: bool, } -impl InPlaceIterFront { +impl InPlaceDrop { unsafe fn len(&self) -> usize { self.dst.offset_from(self.inner) as usize } } -impl Drop for InPlaceIterFront { +impl Drop for InPlaceDrop { #[inline] fn drop(&mut self) { unsafe { - if mem::needs_drop::() && self.did_panic { + if self.did_panic { ptr::drop_in_place(slice::from_raw_parts_mut(self.inner, self.len()) as *mut _); } } @@ -2153,31 +2153,61 @@ fn from_into_iter_source(mut iterator: I) -> Vec where I: Iterator + InPlaceIterable + SourceIter>, { - let original_ptr = iterator.as_inner().buf.as_ptr(); - let mut front_buffer = - InPlaceIterFront { inner: original_ptr, dst: original_ptr, did_panic: true }; - - while let Some(item) = iterator.next() { - let source_iter = iterator.as_inner(); - debug_assert_eq!(original_ptr, source_iter.buf.as_ptr()); - unsafe { - debug_assert!( - front_buffer.dst as *const _ < source_iter.ptr, - "InPlaceIterable implementation produced more\ - items than it consumed from the source" - ); - ptr::write(front_buffer.dst, item); - front_buffer.dst = front_buffer.dst.add(1); - } - } + let src_buf = iterator.as_inner().buf.as_ptr(); + let src_end = iterator.as_inner().end; + let dst = src_buf; + + let dst = if mem::needs_drop::() { + // special-case drop handling since it prevents vectorization + let mut sink = InPlaceDrop { inner: src_buf, dst, did_panic: true }; + let _ = iterator.try_for_each::<_, Result<_, !>>(|item| { + unsafe { + debug_assert!( + sink.dst as *const _ <= src_end, + "InPlaceIterable contract violation" + ); + ptr::write(sink.dst, item); + sink.dst = sink.dst.add(1); + } + Ok(()) + }); + sink.did_panic = false; + sink.dst + } else { + // use try-fold since it vectorizes better, does not take ownership and lets us thread the + // write pointer through its innards + iterator + .try_fold::<_, _, Result<_, !>>(dst, move |mut dst, item| { + unsafe { + // the InPlaceIterable contract cannot be verified precisely here since + // try_fold has an exclusive reference to the source pointer + // all we can do is check if it's still in range + debug_assert!(dst as *const _ <= src_end, "InPlaceIterable contract violation"); + ptr::write(dst, item); + dst = dst.add(1); + } + Ok(dst) + }) + .unwrap() + }; let src = iterator.as_inner(); - front_buffer.did_panic = false; - let vec = unsafe { Vec::from_raw_parts(src.buf.as_ptr(), front_buffer.len(), src.cap) }; + // check if SourceIter and InPlaceIterable contracts were upheld. + // but if they weren't we may not even make it to this point + debug_assert_eq!(src_buf, src.buf.as_ptr()); + debug_assert!(dst as *const _ <= src.ptr, "InPlaceIterable contract violation"); + + let vec = unsafe { + let len = dst.offset_from(src_buf) as usize; + Vec::from_raw_parts(src.buf.as_ptr(), len, src.cap) + }; + // prevent drop of the underlying storage by turning the IntoIter into + // the equivalent of Vec::new().into_iter() src.cap = 0; src.buf = unsafe { NonNull::new_unchecked(RawVec::NEW.ptr()) }; src.ptr = src.buf.as_ptr(); src.end = src.buf.as_ptr(); + vec } From b90816deb7be23d1884f051d20bf9b1b8883d2e4 Mon Sep 17 00:00:00 2001 From: The8472 Date: Sun, 17 Nov 2019 01:32:36 +0100 Subject: [PATCH 12/63] remove example that relied on non-public trait --- library/core/src/iter/adapters/mod.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/library/core/src/iter/adapters/mod.rs b/library/core/src/iter/adapters/mod.rs index 7149bb1db052e..aaf3f69049067 100644 --- a/library/core/src/iter/adapters/mod.rs +++ b/library/core/src/iter/adapters/mod.rs @@ -69,10 +69,6 @@ pub unsafe trait SourceIter { /// rely on it not being modified. /// /// Adapters must not rely on exclusive ownership or immutability of the source. - /// For example a peeking adapter could either exploit [`TrustedRandomAccess`] to look ahead - /// or implement this trait, but it cannot do both because a caller could call `next()` or any - /// other mutating method on the source between iteration steps and thus invalidate the peeked - /// values. /// The lack of exclusive ownership also requires that adapters must uphold the source's /// public API even when they have crate- or module-internal access. /// @@ -84,7 +80,6 @@ pub unsafe trait SourceIter { /// access to the underlying storage of an iterator while restricting any statefulness /// and side-effects of the pipeline stages from affecting or relying on that storage. /// - /// [`TrustedRandomAccess`]: trait.TrustedRandomAccess.html /// [`next()`]: trait.Iterator.html#method.next fn as_inner(&mut self) -> &mut Self::Source; } From 07a8c1b95ad23a0d08bffce2613eb36cd0be8400 Mon Sep 17 00:00:00 2001 From: The8472 Date: Sun, 17 Nov 2019 12:04:12 +0100 Subject: [PATCH 13/63] hide binary_heap::IntoIter internals behind impl Trait --- library/alloc/src/collections/binary_heap.rs | 2 +- library/alloc/src/lib.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/library/alloc/src/collections/binary_heap.rs b/library/alloc/src/collections/binary_heap.rs index 6154d03226fd9..73f119c680f20 100644 --- a/library/alloc/src/collections/binary_heap.rs +++ b/library/alloc/src/collections/binary_heap.rs @@ -1175,7 +1175,7 @@ impl FusedIterator for IntoIter {} #[unstable(issue = "0", feature = "inplace_iteration")] unsafe impl SourceIter for IntoIter { - type Source = crate::vec::IntoIter; + type Source = impl Iterator; #[inline] fn as_inner(&mut self) -> &mut Self::Source { diff --git a/library/alloc/src/lib.rs b/library/alloc/src/lib.rs index e755afe9606f1..5cbd9d380b0d9 100644 --- a/library/alloc/src/lib.rs +++ b/library/alloc/src/lib.rs @@ -136,6 +136,7 @@ #![feature(maybe_uninit_extra, maybe_uninit_slice)] #![feature(alloc_layout_extra)] #![feature(try_trait)] +#![feature(type_alias_impl_trait)] #![feature(associated_type_bounds)] // Allow testing this library From 631543dcb4e79d3c134a7dfc8e87b62287e96796 Mon Sep 17 00:00:00 2001 From: The8472 Date: Sun, 17 Nov 2019 12:49:18 +0100 Subject: [PATCH 14/63] restore Vec::extend specialization for vec::IntoIter sources that was lost during refactoring --- library/alloc/src/vec.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index a5caf2522c8f4..29878851da5ce 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2297,6 +2297,15 @@ where } } +impl SpecExtend> for Vec { + fn spec_extend(&mut self, mut iterator: IntoIter) { + unsafe { + self.append_elements(iterator.as_slice() as _); + } + iterator.ptr = iterator.end; + } +} + impl<'a, T: 'a, I> SpecExtend<&'a T, I> for Vec where I: Iterator, From a4e385a0d0c1966870a18db5e138a55b8ebc7b04 Mon Sep 17 00:00:00 2001 From: The8472 Date: Sun, 17 Nov 2019 14:50:48 +0100 Subject: [PATCH 15/63] use memmove instead of generic in-place iteration for IntoIter source this is the original SpecExtend<_, IntoIter> logic except generalizing the fast-path to include a memmove --- library/alloc/src/vec.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index 29878851da5ce..ce7ea2058b5b3 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2216,14 +2216,22 @@ impl SpecFrom> for Vec { // A common case is passing a vector into a function which immediately // re-collects into a vector. We can short circuit this if the IntoIter // has not been advanced at all. - if iterator.buf.as_ptr() as *const _ == iterator.ptr { + // We can also reuse the memory and move the data to the front if + // allocating a new vector and moving to it would result in the same capacity + let non_zero_offset = iterator.buf.as_ptr() as *const _ != iterator.ptr; + if !non_zero_offset || iterator.len() >= iterator.cap / 2 { unsafe { let it = ManuallyDrop::new(iterator); + if non_zero_offset { + ptr::copy(it.ptr, it.buf.as_ptr(), it.len()); + } return Vec::from_raw_parts(it.buf.as_ptr(), it.len(), it.cap); } } - from_into_iter_source(iterator) + let mut vec = Vec::new(); + vec.extend(iterator); + vec } } From 2f700d085a988b1f2a51d80879e9e55bba031008 Mon Sep 17 00:00:00 2001 From: The8472 Date: Tue, 19 Nov 2019 23:10:43 +0100 Subject: [PATCH 16/63] add benches from bluss' gists --- library/alloc/benches/vec.rs | 104 +++++++++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/library/alloc/benches/vec.rs b/library/alloc/benches/vec.rs index 6a3d704612644..df11aa9355f5b 100644 --- a/library/alloc/benches/vec.rs +++ b/library/alloc/benches/vec.rs @@ -471,3 +471,107 @@ bench_in_place![ bench_in_place_u128_i1_0100, u128, 100, 1; bench_in_place_u128_i1_1000, u128, 1000, 1 ]; + +#[bench] +fn bench_chain_collect(b: &mut test::Bencher) { + let data = black_box([0; LEN]); + b.iter(|| data.iter().cloned().chain([1].iter().cloned()).collect::>()); +} + +#[bench] +fn bench_chain_chain_collect(b: &mut test::Bencher) { + let data = black_box([0; LEN]); + b.iter(|| { + data.iter() + .cloned() + .chain([1].iter().cloned()) + .chain([2].iter().cloned()) + .collect::>() + }); +} + +#[bench] +fn bench_nest_chain_chain_collect(b: &mut test::Bencher) { + let data = black_box([0; LEN]); + b.iter(|| { + data.iter().cloned().chain([1].iter().chain([2].iter()).cloned()).collect::>() + }); +} + +pub fn example_plain_slow(l: &[u32]) -> Vec { + let mut result = Vec::with_capacity(l.len()); + result.extend(l.iter().rev()); + result +} + +pub fn map_fast(l: &[(u32, u32)]) -> Vec { + let mut result = Vec::with_capacity(l.len()); + for i in 0..l.len() { + unsafe { + *result.get_unchecked_mut(i) = l[i].0; + result.set_len(i); + } + } + result +} + +const LEN: usize = 16384; + +#[bench] +fn bench_range_map_collect(b: &mut test::Bencher) { + b.iter(|| (0..LEN).map(|_| u32::default()).collect::>()); +} + +#[bench] +fn bench_chain_extend_ref(b: &mut test::Bencher) { + let data = black_box([0; LEN]); + b.iter(|| { + let mut v = Vec::::with_capacity(data.len() + 1); + v.extend(data.iter().chain([1].iter())); + v + }); +} + +#[bench] +fn bench_chain_extend_value(b: &mut test::Bencher) { + let data = black_box([0; LEN]); + b.iter(|| { + let mut v = Vec::::with_capacity(data.len() + 1); + v.extend(data.iter().cloned().chain(Some(1))); + v + }); +} + +#[bench] +fn bench_rev_1(b: &mut test::Bencher) { + let data = black_box([0; LEN]); + b.iter(|| { + let mut v = Vec::::new(); + v.extend(data.iter().rev()); + v + }); +} + +#[bench] +fn bench_rev_2(b: &mut test::Bencher) { + let data = black_box([0; LEN]); + b.iter(|| { + example_plain_slow(&data); + }); +} + +#[bench] +fn bench_map_regular(b: &mut test::Bencher) { + let data = black_box([(0, 0); LEN]); + b.iter(|| { + let mut v = Vec::::new(); + v.extend(data.iter().map(|t| t.1)); + v + }); +} + +#[bench] +fn bench_map_fast(b: &mut test::Bencher) { + let data = black_box([(0, 0); LEN]); + b.iter(|| map_fast(&data)); +} From bb4f888a590b1fe24a386f3f40bad8537c3232a9 Mon Sep 17 00:00:00 2001 From: The8472 Date: Wed, 20 Nov 2019 03:09:30 +0100 Subject: [PATCH 17/63] return the things under test so they get black_box()'ed --- library/alloc/benches/vec.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/library/alloc/benches/vec.rs b/library/alloc/benches/vec.rs index df11aa9355f5b..870bb3d14026c 100644 --- a/library/alloc/benches/vec.rs +++ b/library/alloc/benches/vec.rs @@ -7,6 +7,7 @@ fn bench_new(b: &mut Bencher) { let v: Vec = Vec::new(); assert_eq!(v.len(), 0); assert_eq!(v.capacity(), 0); + v }) } @@ -17,6 +18,7 @@ fn do_bench_with_capacity(b: &mut Bencher, src_len: usize) { let v: Vec = Vec::with_capacity(src_len); assert_eq!(v.len(), 0); assert_eq!(v.capacity(), src_len); + v }) } @@ -47,6 +49,7 @@ fn do_bench_from_fn(b: &mut Bencher, src_len: usize) { let dst = (0..src_len).collect::>(); assert_eq!(dst.len(), src_len); assert!(dst.iter().enumerate().all(|(i, x)| i == *x)); + dst }) } @@ -77,6 +80,7 @@ fn do_bench_from_elem(b: &mut Bencher, src_len: usize) { let dst: Vec = repeat(5).take(src_len).collect(); assert_eq!(dst.len(), src_len); assert!(dst.iter().all(|x| *x == 5)); + dst }) } @@ -109,6 +113,7 @@ fn do_bench_from_slice(b: &mut Bencher, src_len: usize) { let dst = src.clone()[..].to_vec(); assert_eq!(dst.len(), src_len); assert!(dst.iter().enumerate().all(|(i, x)| i == *x)); + dst }); } @@ -141,6 +146,7 @@ fn do_bench_from_iter(b: &mut Bencher, src_len: usize) { let dst: Vec<_> = FromIterator::from_iter(src.clone()); assert_eq!(dst.len(), src_len); assert!(dst.iter().enumerate().all(|(i, x)| i == *x)); + dst }); } @@ -175,6 +181,7 @@ fn do_bench_extend(b: &mut Bencher, dst_len: usize, src_len: usize) { dst.extend(src.clone()); assert_eq!(dst.len(), dst_len + src_len); assert!(dst.iter().enumerate().all(|(i, x)| i == *x)); + dst }); } @@ -224,6 +231,7 @@ fn do_bench_extend_from_slice(b: &mut Bencher, dst_len: usize, src_len: usize) { dst.extend_from_slice(&src); assert_eq!(dst.len(), dst_len + src_len); assert!(dst.iter().enumerate().all(|(i, x)| i == *x)); + dst }); } @@ -271,6 +279,7 @@ fn do_bench_clone(b: &mut Bencher, src_len: usize) { let dst = src.clone(); assert_eq!(dst.len(), src_len); assert!(dst.iter().enumerate().all(|(i, x)| i == *x)); + dst }); } @@ -305,10 +314,10 @@ fn do_bench_clone_from(b: &mut Bencher, times: usize, dst_len: usize, src_len: u for _ in 0..times { dst.clone_from(&src); - assert_eq!(dst.len(), src_len); assert!(dst.iter().enumerate().all(|(i, x)| dst_len + i == *x)); } + dst }); } From 8ac96e6a9832d70f9b5d43967cd2680711fa92df Mon Sep 17 00:00:00 2001 From: The8472 Date: Wed, 20 Nov 2019 23:37:50 +0100 Subject: [PATCH 18/63] cyclic in-place reuse bench --- library/alloc/benches/vec.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/library/alloc/benches/vec.rs b/library/alloc/benches/vec.rs index 870bb3d14026c..409bc49b03146 100644 --- a/library/alloc/benches/vec.rs +++ b/library/alloc/benches/vec.rs @@ -481,6 +481,26 @@ bench_in_place![ bench_in_place_u128_i1_1000, u128, 1000, 1 ]; +#[bench] +fn bench_in_place_recycle(b: &mut test::Bencher) { + let mut data = vec![0; 1000]; + + b.iter(|| { + let tmp = std::mem::replace(&mut data, Vec::new()); + std::mem::replace( + &mut data, + black_box( + tmp.into_iter() + .enumerate() + .map(|(idx, e)| idx.wrapping_add(e)) + .fuse() + .peekable() + .collect::>(), + ), + ); + }); +} + #[bench] fn bench_chain_collect(b: &mut test::Bencher) { let data = black_box([0; LEN]); From a9c78e371e3b46f0d1cf0b38368d503b6aa1ce6e Mon Sep 17 00:00:00 2001 From: The8472 Date: Thu, 21 Nov 2019 00:06:31 +0100 Subject: [PATCH 19/63] bench in-place collect of droppables --- library/alloc/benches/vec.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/library/alloc/benches/vec.rs b/library/alloc/benches/vec.rs index 409bc49b03146..a11b946c44b94 100644 --- a/library/alloc/benches/vec.rs +++ b/library/alloc/benches/vec.rs @@ -501,6 +501,21 @@ fn bench_in_place_recycle(b: &mut test::Bencher) { }); } +#[derive(Clone)] +struct Droppable(usize); + +impl Drop for Droppable { + fn drop(&mut self) { + black_box(self); + } +} + +#[bench] +fn bench_in_place_collect_droppable(b: &mut test::Bencher) { + let v: Vec = std::iter::repeat_with(|| Droppable(0)).take(1000).collect(); + b.iter(|| v.clone().into_iter().skip(100).collect::>()) +} + #[bench] fn bench_chain_collect(b: &mut test::Bencher) { let data = black_box([0; LEN]); From a596ff36b55a37e7a74abd0504ff895a3d2fba6f Mon Sep 17 00:00:00 2001 From: The8472 Date: Thu, 21 Nov 2019 01:01:35 +0100 Subject: [PATCH 20/63] exercise more of the in-place pipeline in the bench --- library/alloc/benches/vec.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/library/alloc/benches/vec.rs b/library/alloc/benches/vec.rs index a11b946c44b94..ea8cd6b5155b6 100644 --- a/library/alloc/benches/vec.rs +++ b/library/alloc/benches/vec.rs @@ -513,7 +513,14 @@ impl Drop for Droppable { #[bench] fn bench_in_place_collect_droppable(b: &mut test::Bencher) { let v: Vec = std::iter::repeat_with(|| Droppable(0)).take(1000).collect(); - b.iter(|| v.clone().into_iter().skip(100).collect::>()) + b.iter(|| { + v.clone() + .into_iter() + .skip(100) + .enumerate() + .map(|(i, e)| Droppable(i ^ e.0)) + .collect::>() + }) } #[bench] From 582fbb1d62420d8d85f364d06669038f12b7e423 Mon Sep 17 00:00:00 2001 From: The8472 Date: Thu, 21 Nov 2019 13:24:48 +0100 Subject: [PATCH 21/63] use From specializations on extend if extended Vec is empty this enables in-place iteration and allocation reuse in additional cases --- library/alloc/src/vec.rs | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index ce7ea2058b5b3..ffd6f5b31d438 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2080,7 +2080,16 @@ impl<'a, T> IntoIterator for &'a mut Vec { impl Extend for Vec { #[inline] fn extend>(&mut self, iter: I) { - >::spec_extend(self, iter.into_iter()) + if self.capacity() > 0 { + >::spec_extend(self, iter.into_iter()) + } else { + // if self has no allocation then use the more powerful from_iter specializations + let other = SpecFrom::from_iter(iter.into_iter()); + // replace self, don't run drop since self was empty + unsafe { + ptr::write(self, other); + } + } } #[inline] @@ -2121,6 +2130,8 @@ where vector } }; + // must delegate to spec_extend() since extend() itself delegates + // to spec_from for empty Vecs as SpecExtend>::spec_extend(&mut vector, iterator); vector } @@ -2230,7 +2241,9 @@ impl SpecFrom> for Vec { } let mut vec = Vec::new(); - vec.extend(iterator); + // must delegate to spec_extend() since extend() itself delegates + // to spec_from for empty Vecs + vec.spec_extend(iterator); vec } } @@ -2475,7 +2488,16 @@ impl Vec { #[stable(feature = "extend_ref", since = "1.2.0")] impl<'a, T: 'a + Copy> Extend<&'a T> for Vec { fn extend>(&mut self, iter: I) { - self.spec_extend(iter.into_iter()) + if self.capacity() > 0 { + self.spec_extend(iter.into_iter()) + } else { + // if self has no allocation then use the more powerful from_iter specializations + let other = SpecFrom::from_iter(iter.into_iter()); + // replace self, don't run drop since self was empty + unsafe { + ptr::write(self, other); + } + } } #[inline] From dac0edfaaaa5a8c668f70a1cd58468c700a04627 Mon Sep 17 00:00:00 2001 From: The8472 Date: Thu, 21 Nov 2019 13:40:49 +0100 Subject: [PATCH 22/63] restore SpecFrom> specialization by nesting specializations --- library/alloc/src/vec.rs | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index ffd6f5b31d438..272ad748bc756 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2108,7 +2108,13 @@ trait SpecFrom { fn from_iter(iter: I) -> Self; } -impl SpecFrom for Vec +// Another specialization trait for Vec::from_iter +// necessary to manually prioritize overlapping specializations +trait SpecFromNested { + fn from_iter(iter: I) -> Self; +} + +impl SpecFromNested for Vec where I: Iterator, { @@ -2137,6 +2143,28 @@ where } } +impl SpecFromNested for Vec +where + I: TrustedLen, +{ + fn from_iter(iterator: I) -> Self { + let mut vector = Vec::new(); + // must delegate to spec_extend() since extend() itself delegates + // to spec_from for empty Vecs + vector.spec_extend(iterator); + vector + } +} + +impl SpecFrom for Vec +where + I: Iterator, +{ + default fn from_iter(iterator: I) -> Self { + SpecFromNested::from_iter(iterator) + } +} + struct InPlaceDrop { inner: *mut T, dst: *mut T, From 290fe895ba6a507479a745cb9ce720b47570a52c Mon Sep 17 00:00:00 2001 From: The8472 Date: Thu, 21 Nov 2019 13:43:53 +0100 Subject: [PATCH 23/63] specialize creating a Vec from a slice iterator where T: Copy this was already implemented for Extend but not for FromIterator --- library/alloc/src/vec.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index 272ad748bc756..9e5b609615209 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2299,6 +2299,20 @@ where } } +impl<'a, T: 'a> SpecFrom<&'a T, slice::Iter<'a, T>> for Vec +where + T: Copy, +{ + // reuses the extend specialization for T: Copy + fn from_iter(iterator: slice::Iter<'a, T>) -> Self { + let mut vec = Vec::new(); + // must delegate to spec_extend() since extend() itself delegates + // to spec_from for empty Vecs + vec.spec_extend(iterator); + vec + } +} + // Specialization trait used for Vec::extend trait SpecExtend { fn spec_extend(&mut self, iter: I); From cc67c8eb911a2ce607614434dabc41df26ca5d37 Mon Sep 17 00:00:00 2001 From: The8472 Date: Thu, 21 Nov 2019 13:44:28 +0100 Subject: [PATCH 24/63] improve comments --- library/alloc/src/vec.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index 9e5b609615209..fc4ccf043fd9f 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2213,8 +2213,10 @@ where sink.did_panic = false; sink.dst } else { - // use try-fold since it vectorizes better, does not take ownership and lets us thread the - // write pointer through its innards + // use try-fold + // - it vectorizes better + // - unlike most internal iteration methods methods it only takes a &mut self + // - lets us thread the write pointer through its innards and get it back in the end iterator .try_fold::<_, _, Result<_, !>>(dst, move |mut dst, item| { unsafe { @@ -2232,7 +2234,7 @@ where let src = iterator.as_inner(); // check if SourceIter and InPlaceIterable contracts were upheld. - // but if they weren't we may not even make it to this point + // caveat: if they weren't we may not even make it to this point debug_assert_eq!(src_buf, src.buf.as_ptr()); debug_assert!(dst as *const _ <= src.ptr, "InPlaceIterable contract violation"); @@ -2276,10 +2278,9 @@ impl SpecFrom> for Vec { } } -// Further specialization potential once lattice specialization exists -// and https://github.com/rust-lang/rust/issues/62645 has been solved: -// This can be broadened to only require size and alignment equality between -// input and output Item types. +// Further specialization potential once +// https://github.com/rust-lang/rust/issues/62645 has been solved: +// T can be split into IN and OUT which only need to have the same size and alignment impl SpecFrom for Vec where I: Iterator + InPlaceIterable + SourceIter>, @@ -2396,6 +2397,8 @@ where } impl Vec { + // leaf method to which various SpecFrom/SpecExtend implementations delegate when + // they have no further optimizations to apply fn extend_desugared>(&mut self, mut iterator: I) { // This is the case for a general iterator. // From 8c816b96dd549d24146f6c4be410fcf7526221d1 Mon Sep 17 00:00:00 2001 From: The8472 Date: Sat, 23 Nov 2019 14:30:10 +0100 Subject: [PATCH 25/63] remove redundant code --- library/alloc/src/vec.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index fc4ccf043fd9f..fdc7738e733e8 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2386,13 +2386,7 @@ where { fn spec_extend(&mut self, iterator: slice::Iter<'a, T>) { let slice = iterator.as_slice(); - self.reserve(slice.len()); - unsafe { - let len = self.len(); - let dst_slice = slice::from_raw_parts_mut(self.as_mut_ptr().add(len), slice.len()); - dst_slice.copy_from_slice(slice); - self.set_len(len + slice.len()); - } + unsafe { self.append_elements(slice) }; } } From 00a32eb54f65c11cd2f4d10c2414dd633fab3c5b Mon Sep 17 00:00:00 2001 From: The8472 Date: Sat, 23 Nov 2019 14:32:20 +0100 Subject: [PATCH 26/63] fix some in-place-collect edge-cases - it's an allocation optimization, so don't attempt to do it on ZSTs - drop the tail of partially exhausted iters --- library/alloc/src/vec.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index fdc7738e733e8..1e7f95a25cc25 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2192,6 +2192,12 @@ fn from_into_iter_source(mut iterator: I) -> Vec where I: Iterator + InPlaceIterable + SourceIter>, { + // This specialization only makes sense if we're juggling real allocations. + // Additionally some of the pointer arithmetic would panic on ZSTs. + if mem::size_of::() == 0 { + return SpecFromNested::from_iter(iterator); + } + let src_buf = iterator.as_inner().buf.as_ptr(); let src_end = iterator.as_inner().end; let dst = src_buf; @@ -2238,6 +2244,13 @@ where debug_assert_eq!(src_buf, src.buf.as_ptr()); debug_assert!(dst as *const _ <= src.ptr, "InPlaceIterable contract violation"); + if mem::needs_drop::() { + // drop tail if iterator was only partially exhaused + unsafe { + ptr::drop_in_place(src.as_mut_slice()); + } + } + let vec = unsafe { let len = dst.offset_from(src_buf) as usize; Vec::from_raw_parts(src.buf.as_ptr(), len, src.cap) From 2b0b2ae9f61c26338dc00c70eee936230b4b75c0 Mon Sep 17 00:00:00 2001 From: The8472 Date: Sat, 23 Nov 2019 14:34:58 +0100 Subject: [PATCH 27/63] additional specializations tests --- library/alloc/tests/vec.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/library/alloc/tests/vec.rs b/library/alloc/tests/vec.rs index 271a705cf0631..000797d3600fc 100644 --- a/library/alloc/tests/vec.rs +++ b/library/alloc/tests/vec.rs @@ -785,6 +785,28 @@ fn test_from_iter_specialization() { assert_eq!(srcptr, sinkptr); } +#[test] +fn test_from_iter_partially_drained_in_place_specialization() { + let src: Vec = vec![0usize; 10]; + let srcptr = src.as_ptr(); + let mut iter = src.into_iter(); + iter.next(); + iter.next(); + let sink = iter.collect::>(); + let sinkptr = sink.as_ptr(); + assert_eq!(srcptr, sinkptr); +} + +#[test] +fn test_extend_in_place_specialization() { + let src: Vec = vec![0usize; 1]; + let srcptr = src.as_ptr(); + let mut dst = Vec::new(); + dst.extend(src.into_iter()); + let dstptr = dst.as_ptr(); + assert_eq!(srcptr, dstptr); +} + #[test] fn test_from_iter_specialization_with_iterator_adapters() { fn assert_in_place_trait(_: &T) {}; From 3d5e9f1904e2c08369bfa7ec3963dab12a76544b Mon Sep 17 00:00:00 2001 From: The8472 Date: Sat, 23 Nov 2019 18:26:52 +0100 Subject: [PATCH 28/63] bench in-place zip --- library/alloc/benches/vec.rs | 37 ++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/library/alloc/benches/vec.rs b/library/alloc/benches/vec.rs index ea8cd6b5155b6..a6af316e6a09b 100644 --- a/library/alloc/benches/vec.rs +++ b/library/alloc/benches/vec.rs @@ -1,3 +1,4 @@ +use rand::prelude::*; use std::iter::{repeat, FromIterator}; use test::{black_box, Bencher}; @@ -501,6 +502,42 @@ fn bench_in_place_recycle(b: &mut test::Bencher) { }); } +#[bench] +fn bench_in_place_zip_recycle(b: &mut test::Bencher) { + let mut data = vec![0u8; 256]; + let mut rng = rand::thread_rng(); + let mut subst = (0..=255u8).collect::>(); + subst.shuffle(&mut rng); + + b.iter(|| { + let tmp = std::mem::replace(&mut data, Vec::new()); + let mangled = tmp + .into_iter() + .zip(subst.iter().copied()) + .enumerate() + .map(|(i, (d, s))| d.wrapping_add(i as u8) ^ s) + .collect::>(); + assert_eq!(mangled.len(), 256); + std::mem::replace(&mut data, black_box(mangled)); + }); +} + +#[bench] +fn bench_in_place_zip_iter_mut(b: &mut test::Bencher) { + let mut data = vec![0u8; 256]; + let mut rng = rand::thread_rng(); + let mut subst = (0..=255u8).collect::>(); + subst.shuffle(&mut rng); + + b.iter(|| { + data.iter_mut().enumerate().for_each(|(i, d)| { + *d = d.wrapping_add(i as u8) ^ subst[i]; + }); + }); + + black_box(data); +} + #[derive(Clone)] struct Droppable(usize); From 0f122e11196875c75071f4e3ac4ce1a652e320bf Mon Sep 17 00:00:00 2001 From: The8472 Date: Sat, 23 Nov 2019 18:29:47 +0100 Subject: [PATCH 29/63] add in-place iteration for Zip this picks the left hand side as source since it might be more natural to consume that as IntoIter source --- library/core/src/iter/adapters/zip.rs | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/library/core/src/iter/adapters/zip.rs b/library/core/src/iter/adapters/zip.rs index 6cb618964830e..6624b5223e327 100644 --- a/library/core/src/iter/adapters/zip.rs +++ b/library/core/src/iter/adapters/zip.rs @@ -1,7 +1,10 @@ use crate::cmp; use crate::fmt::{self, Debug}; -use super::super::{DoubleEndedIterator, ExactSizeIterator, FusedIterator, Iterator, TrustedLen}; +use super::super::{ + DoubleEndedIterator, ExactSizeIterator, FusedIterator, InPlaceIterable, Iterator, SourceIter, + TrustedLen, +}; /// An iterator that iterates two other iterators simultaneously. /// @@ -327,6 +330,26 @@ where { } +// Arbitrarily selects the left side of the zip iteration as extractable "source" +// it would require negative trait bounds to be able to try both +#[unstable(issue = "0", feature = "inplace_iteration")] +unsafe impl SourceIter for Zip + where + A: SourceIter, + B: Iterator, + S: Iterator, +{ + type Source = S; + + #[inline] + fn as_inner(&mut self) -> &mut S { + SourceIter::as_inner(&mut self.a) + } +} + +#[unstable(issue = "0", feature = "inplace_iteration")] +unsafe impl InPlaceIterable for Zip {} + #[stable(feature = "rust1", since = "1.0.0")] impl Debug for Zip { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { From 085eb20a61164067f5c71ec64dc23100006f91c9 Mon Sep 17 00:00:00 2001 From: The8472 Date: Sat, 23 Nov 2019 18:30:32 +0100 Subject: [PATCH 30/63] move free-standing method into trait impl --- library/alloc/src/vec.rs | 154 +++++++++++++++++++-------------------- 1 file changed, 75 insertions(+), 79 deletions(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index 1e7f95a25cc25..9327cf16c1593 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2188,83 +2188,6 @@ impl Drop for InPlaceDrop { } } -fn from_into_iter_source(mut iterator: I) -> Vec -where - I: Iterator + InPlaceIterable + SourceIter>, -{ - // This specialization only makes sense if we're juggling real allocations. - // Additionally some of the pointer arithmetic would panic on ZSTs. - if mem::size_of::() == 0 { - return SpecFromNested::from_iter(iterator); - } - - let src_buf = iterator.as_inner().buf.as_ptr(); - let src_end = iterator.as_inner().end; - let dst = src_buf; - - let dst = if mem::needs_drop::() { - // special-case drop handling since it prevents vectorization - let mut sink = InPlaceDrop { inner: src_buf, dst, did_panic: true }; - let _ = iterator.try_for_each::<_, Result<_, !>>(|item| { - unsafe { - debug_assert!( - sink.dst as *const _ <= src_end, - "InPlaceIterable contract violation" - ); - ptr::write(sink.dst, item); - sink.dst = sink.dst.add(1); - } - Ok(()) - }); - sink.did_panic = false; - sink.dst - } else { - // use try-fold - // - it vectorizes better - // - unlike most internal iteration methods methods it only takes a &mut self - // - lets us thread the write pointer through its innards and get it back in the end - iterator - .try_fold::<_, _, Result<_, !>>(dst, move |mut dst, item| { - unsafe { - // the InPlaceIterable contract cannot be verified precisely here since - // try_fold has an exclusive reference to the source pointer - // all we can do is check if it's still in range - debug_assert!(dst as *const _ <= src_end, "InPlaceIterable contract violation"); - ptr::write(dst, item); - dst = dst.add(1); - } - Ok(dst) - }) - .unwrap() - }; - - let src = iterator.as_inner(); - // check if SourceIter and InPlaceIterable contracts were upheld. - // caveat: if they weren't we may not even make it to this point - debug_assert_eq!(src_buf, src.buf.as_ptr()); - debug_assert!(dst as *const _ <= src.ptr, "InPlaceIterable contract violation"); - - if mem::needs_drop::() { - // drop tail if iterator was only partially exhaused - unsafe { - ptr::drop_in_place(src.as_mut_slice()); - } - } - - let vec = unsafe { - let len = dst.offset_from(src_buf) as usize; - Vec::from_raw_parts(src.buf.as_ptr(), len, src.cap) - }; - // prevent drop of the underlying storage by turning the IntoIter into - // the equivalent of Vec::new().into_iter() - src.cap = 0; - src.buf = unsafe { NonNull::new_unchecked(RawVec::NEW.ptr()) }; - src.ptr = src.buf.as_ptr(); - src.end = src.buf.as_ptr(); - - vec -} - impl SpecFrom> for Vec { fn from_iter(iterator: IntoIter) -> Self { // A common case is passing a vector into a function which immediately @@ -2298,8 +2221,81 @@ impl SpecFrom for Vec where I: Iterator + InPlaceIterable + SourceIter>, { - default fn from_iter(iterator: I) -> Self { - from_into_iter_source(iterator) + default fn from_iter(mut iterator: I) -> Self { + // This specialization only makes sense if we're juggling real allocations. + // Additionally some of the pointer arithmetic would panic on ZSTs. + if mem::size_of::() == 0 { + return SpecFromNested::from_iter(iterator); + } + + let src_buf = iterator.as_inner().buf.as_ptr(); + let src_end = iterator.as_inner().end; + let dst = src_buf; + + let dst = if mem::needs_drop::() { + // special-case drop handling since it prevents vectorization + let mut sink = InPlaceDrop { inner: src_buf, dst, did_panic: true }; + let _ = iterator.try_for_each::<_, Result<_, !>>(|item| { + unsafe { + debug_assert!( + sink.dst as *const _ <= src_end, + "InPlaceIterable contract violation" + ); + ptr::write(sink.dst, item); + sink.dst = sink.dst.add(1); + } + Ok(()) + }); + sink.did_panic = false; + sink.dst + } else { + // use try-fold + // - it vectorizes better + // - unlike most internal iteration methods methods it only takes a &mut self + // - lets us thread the write pointer through its innards and get it back in the end + iterator + .try_fold::<_, _, Result<_, !>>(dst, move |mut dst, item| { + unsafe { + // the InPlaceIterable contract cannot be verified precisely here since + // try_fold has an exclusive reference to the source pointer + // all we can do is check if it's still in range + debug_assert!( + dst as *const _ <= src_end, + "InPlaceIterable contract violation" + ); + ptr::write(dst, item); + dst = dst.add(1); + } + Ok(dst) + }) + .unwrap() + }; + + let src = iterator.as_inner(); + // check if SourceIter and InPlaceIterable contracts were upheld. + // caveat: if they weren't we may not even make it to this point + debug_assert_eq!(src_buf, src.buf.as_ptr()); + debug_assert!(dst as *const _ <= src.ptr, "InPlaceIterable contract violation"); + + if mem::needs_drop::() { + // drop tail if iterator was only partially exhaused + unsafe { + ptr::drop_in_place(src.as_mut_slice()); + } + } + + let vec = unsafe { + let len = dst.offset_from(src_buf) as usize; + Vec::from_raw_parts(src.buf.as_ptr(), len, src.cap) + }; + // prevent drop of the underlying storage by turning the IntoIter into + // the equivalent of Vec::new().into_iter() + src.cap = 0; + src.buf = unsafe { NonNull::new_unchecked(RawVec::NEW.ptr()) }; + src.ptr = src.buf.as_ptr(); + src.end = src.buf.as_ptr(); + + vec } } From 21a17d105c9cd81dfa8bd3a178e4a6b095f69e5d Mon Sep 17 00:00:00 2001 From: The8472 Date: Sat, 23 Nov 2019 18:59:18 +0100 Subject: [PATCH 31/63] support in-place iteration for most adapters `Take` is not included since users probably call it with small constants and it doesn't make sense to hold onto huge allocations in that case --- library/core/src/iter/adapters/fuse.rs | 23 +++++ library/core/src/iter/adapters/mod.rs | 113 +++++++++++++++++++++++++ 2 files changed, 136 insertions(+) diff --git a/library/core/src/iter/adapters/fuse.rs b/library/core/src/iter/adapters/fuse.rs index 94ba6f56476ae..e2613be4a46a1 100644 --- a/library/core/src/iter/adapters/fuse.rs +++ b/library/core/src/iter/adapters/fuse.rs @@ -3,6 +3,8 @@ use crate::iter::adapters::zip::try_get_unchecked; use crate::iter::TrustedRandomAccess; use crate::iter::{DoubleEndedIterator, ExactSizeIterator, FusedIterator, Iterator}; use crate::ops::Try; +use crate::iter::adapters::SourceIter; +use super::InPlaceIterable; /// An iterator that yields `None` forever after the underlying iterator /// yields `None` once. @@ -517,3 +519,24 @@ where unchecked!(self).is_empty() } } + + +#[unstable(issue = "0", feature = "inplace_iteration")] +unsafe impl SourceIter for Fuse + where + I: SourceIter, +{ + type Source = S; + + #[inline] + fn as_inner(&mut self) -> &mut S { + match self.iter { + Some(ref mut iter) => SourceIter::as_inner(iter), + // SAFETY: the specialized iterator never sets `None` + None => unsafe { intrinsics::unreachable() }, + } + } +} + +#[unstable(issue = "0", feature = "inplace_iteration")] +unsafe impl InPlaceIterable for Fuse {} diff --git a/library/core/src/iter/adapters/mod.rs b/library/core/src/iter/adapters/mod.rs index aaf3f69049067..a8414bf913764 100644 --- a/library/core/src/iter/adapters/mod.rs +++ b/library/core/src/iter/adapters/mod.rs @@ -1152,6 +1152,22 @@ where #[stable(feature = "fused", since = "1.26.0")] impl FusedIterator for Filter where P: FnMut(&I::Item) -> bool {} +#[unstable(issue = "0", feature = "inplace_iteration")] +unsafe impl SourceIter for Filter where + P: FnMut(&I::Item) -> bool, + I: SourceIter +{ + type Source = S; + + #[inline] + fn as_inner(&mut self) -> &mut S { + SourceIter::as_inner(&mut self.iter) + } +} + +#[unstable(issue = "0", feature = "inplace_iteration")] +unsafe impl InPlaceIterable for Filter where P: FnMut(&I::Item) -> bool {} + /// An iterator that uses `f` to both filter and map elements from `iter`. /// /// This `struct` is created by the [`filter_map`] method on [`Iterator`]. See its @@ -1278,6 +1294,23 @@ where #[stable(feature = "fused", since = "1.26.0")] impl FusedIterator for FilterMap where F: FnMut(I::Item) -> Option {} +#[unstable(issue = "0", feature = "inplace_iteration")] +unsafe impl SourceIter for FilterMap where + F: FnMut(I::Item) -> Option, + I: SourceIter +{ + type Source = S; + + #[inline] + fn as_inner(&mut self) -> &mut S { + SourceIter::as_inner(&mut self.iter) + } +} + +#[unstable(issue = "0", feature = "inplace_iteration")] +unsafe impl InPlaceIterable for FilterMap where F: FnMut(I::Item) -> Option {} + + /// An iterator that yields the current count and the element during iteration. /// /// This `struct` is created by the [`enumerate`] method on [`Iterator`]. See its @@ -1910,6 +1943,22 @@ where { } +#[unstable(issue = "0", feature = "inplace_iteration")] +unsafe impl SourceIter for SkipWhile where + P: FnMut(&I::Item) -> bool, + I: SourceIter +{ + type Source = S; + + #[inline] + fn as_inner(&mut self) -> &mut S { + SourceIter::as_inner(&mut self.iter) + } +} + +#[unstable(issue = "0", feature = "inplace_iteration")] +unsafe impl InPlaceIterable for SkipWhile where F: FnMut(&I::Item) -> bool {} + /// An iterator that only accepts elements while `predicate` returns `true`. /// /// This `struct` is created by the [`take_while`] method on [`Iterator`]. See its @@ -2101,6 +2150,23 @@ where } } +#[unstable(issue = "0", feature = "inplace_iteration")] +unsafe impl SourceIter for TakeWhile where + P: FnMut(&I::Item) -> bool, + I: SourceIter +{ + type Source = S; + + #[inline] + fn as_inner(&mut self) -> &mut S { + SourceIter::as_inner(&mut self.iter) + } +} + +#[unstable(issue = "0", feature = "inplace_iteration")] +unsafe impl InPlaceIterable for TakeWhile where F: FnMut(&I::Item) -> bool {} + + /// An iterator that skips over `n` elements of `iter`. /// /// This `struct` is created by the [`skip`] method on [`Iterator`]. See its @@ -2410,6 +2476,19 @@ where } } +#[unstable(issue = "0", feature = "inplace_iteration")] +unsafe impl SourceIter for Take where I: SourceIter { + type Source = S; + + #[inline] + fn as_inner(&mut self) -> &mut S { + SourceIter::as_inner(&mut self.iter) + } +} + +#[unstable(issue = "0", feature = "inplace_iteration")] +unsafe impl InPlaceIterable for Take {} + #[stable(feature = "double_ended_take_iterator", since = "1.38.0")] impl DoubleEndedIterator for Take where @@ -2574,6 +2653,24 @@ where } } +#[unstable(issue = "0", feature = "inplace_iteration")] +unsafe impl SourceIter for Scan + where I: SourceIter, + F: FnMut(&mut St, I::Item) -> Option, +{ + type Source = S; + + #[inline] + fn as_inner(&mut self) -> &mut S { + SourceIter::as_inner(&mut self.iter) + } +} + +#[unstable(issue = "0", feature = "inplace_iteration")] +unsafe impl InPlaceIterable for Scan + where F: FnMut(&mut St, I::Item) -> Option, +{} + /// An iterator that calls a function with a reference to each element before /// yielding it. /// @@ -2720,6 +2817,22 @@ where #[stable(feature = "fused", since = "1.26.0")] impl FusedIterator for Inspect where F: FnMut(&I::Item) {} +#[unstable(issue = "0", feature = "inplace_iteration")] +unsafe impl SourceIter for Inspect where + F: FnMut(&I::Item), + I: SourceIter +{ + type Source = S; + + #[inline] + fn as_inner(&mut self) -> &mut S { + SourceIter::as_inner(&mut self.iter) + } +} + +#[unstable(issue = "0", feature = "inplace_iteration")] +unsafe impl InPlaceIterable for Inspect where F: FnMut(&I::Item) {} + /// An iterator adapter that produces output as long as the underlying /// iterator produces `Result::Ok` values. /// From 70293c658f326ff5efb9a2fcf049f8c9e7ee9916 Mon Sep 17 00:00:00 2001 From: The8472 Date: Sat, 23 Nov 2019 19:34:26 +0100 Subject: [PATCH 32/63] make tidy happy --- library/core/src/iter/adapters/mod.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/library/core/src/iter/adapters/mod.rs b/library/core/src/iter/adapters/mod.rs index a8414bf913764..0ad279c55337a 100644 --- a/library/core/src/iter/adapters/mod.rs +++ b/library/core/src/iter/adapters/mod.rs @@ -1166,7 +1166,8 @@ unsafe impl SourceIter for Filter where } #[unstable(issue = "0", feature = "inplace_iteration")] -unsafe impl InPlaceIterable for Filter where P: FnMut(&I::Item) -> bool {} +unsafe impl InPlaceIterable for Filter + where P: FnMut(&I::Item) -> bool {} /// An iterator that uses `f` to both filter and map elements from `iter`. /// @@ -1308,7 +1309,8 @@ unsafe impl SourceIter for FilterMap where } #[unstable(issue = "0", feature = "inplace_iteration")] -unsafe impl InPlaceIterable for FilterMap where F: FnMut(I::Item) -> Option {} +unsafe impl InPlaceIterable for FilterMap + where F: FnMut(I::Item) -> Option {} /// An iterator that yields the current count and the element during iteration. @@ -1957,7 +1959,8 @@ unsafe impl SourceIter for SkipWhile where } #[unstable(issue = "0", feature = "inplace_iteration")] -unsafe impl InPlaceIterable for SkipWhile where F: FnMut(&I::Item) -> bool {} +unsafe impl InPlaceIterable for SkipWhile + where F: FnMut(&I::Item) -> bool {} /// An iterator that only accepts elements while `predicate` returns `true`. /// @@ -2164,7 +2167,8 @@ unsafe impl SourceIter for TakeWhile where } #[unstable(issue = "0", feature = "inplace_iteration")] -unsafe impl InPlaceIterable for TakeWhile where F: FnMut(&I::Item) -> bool {} +unsafe impl InPlaceIterable for TakeWhile + where F: FnMut(&I::Item) -> bool {} /// An iterator that skips over `n` elements of `iter`. From fbb3371e5b513bb238cc794c7a2694451c998cf5 Mon Sep 17 00:00:00 2001 From: The8472 Date: Mon, 24 Aug 2020 22:09:44 +0200 Subject: [PATCH 33/63] remove unecessary feature flag # Conflicts: # library/alloc/src/lib.rs --- library/alloc/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/library/alloc/src/lib.rs b/library/alloc/src/lib.rs index 5cbd9d380b0d9..81169c1dd75e7 100644 --- a/library/alloc/src/lib.rs +++ b/library/alloc/src/lib.rs @@ -107,7 +107,6 @@ #![feature(map_first_last)] #![feature(map_into_keys_values)] #![feature(negative_impls)] -#![feature(never_type)] #![feature(new_uninit)] #![feature(nll)] #![feature(nonnull_slice_from_raw_parts)] From fd16202e36f1d4568550eb9022a43842b93fff6e Mon Sep 17 00:00:00 2001 From: The8472 Date: Wed, 27 Nov 2019 22:19:06 +0100 Subject: [PATCH 34/63] include in-place .zip() in test --- library/alloc/tests/vec.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/library/alloc/tests/vec.rs b/library/alloc/tests/vec.rs index 000797d3600fc..cbff7d37cdf7b 100644 --- a/library/alloc/tests/vec.rs +++ b/library/alloc/tests/vec.rs @@ -812,7 +812,14 @@ fn test_from_iter_specialization_with_iterator_adapters() { fn assert_in_place_trait(_: &T) {}; let src: Vec = vec![0usize; 65535]; let srcptr = src.as_ptr(); - let iter = src.into_iter().enumerate().map(|i| i.0 + i.1).peekable().skip(1); + let iter = src + .into_iter() + .enumerate() + .map(|i| i.0 + i.1) + .zip(std::iter::repeat(1usize)) + .map(|(a, b)| a + b) + .peekable() + .skip(1); assert_in_place_trait(&iter); let sink = iter.collect::>(); let sinkptr = sink.as_ptr(); From e1151844fae40ce45bb46808e31e58cc7795500d Mon Sep 17 00:00:00 2001 From: The8472 Date: Wed, 27 Nov 2019 22:19:15 +0100 Subject: [PATCH 35/63] bench larger allocations --- library/alloc/benches/vec.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/library/alloc/benches/vec.rs b/library/alloc/benches/vec.rs index a6af316e6a09b..1ae72a24709bf 100644 --- a/library/alloc/benches/vec.rs +++ b/library/alloc/benches/vec.rs @@ -504,10 +504,10 @@ fn bench_in_place_recycle(b: &mut test::Bencher) { #[bench] fn bench_in_place_zip_recycle(b: &mut test::Bencher) { - let mut data = vec![0u8; 256]; + let mut data = vec![0u8; 1000]; let mut rng = rand::thread_rng(); - let mut subst = (0..=255u8).collect::>(); - subst.shuffle(&mut rng); + let mut subst = vec![0u8; 1000]; + rng.fill_bytes(&mut subst[..]); b.iter(|| { let tmp = std::mem::replace(&mut data, Vec::new()); @@ -517,7 +517,7 @@ fn bench_in_place_zip_recycle(b: &mut test::Bencher) { .enumerate() .map(|(i, (d, s))| d.wrapping_add(i as u8) ^ s) .collect::>(); - assert_eq!(mangled.len(), 256); + assert_eq!(mangled.len(), 1000); std::mem::replace(&mut data, black_box(mangled)); }); } @@ -526,8 +526,8 @@ fn bench_in_place_zip_recycle(b: &mut test::Bencher) { fn bench_in_place_zip_iter_mut(b: &mut test::Bencher) { let mut data = vec![0u8; 256]; let mut rng = rand::thread_rng(); - let mut subst = (0..=255u8).collect::>(); - subst.shuffle(&mut rng); + let mut subst = vec![0u8; 1000]; + rng.fill_bytes(&mut subst[..]); b.iter(|| { data.iter_mut().enumerate().for_each(|(i, d)| { From e85cfa4f2296b6f335d4428c20ce5bec8e59e935 Mon Sep 17 00:00:00 2001 From: The8472 Date: Fri, 29 Nov 2019 01:11:14 +0100 Subject: [PATCH 36/63] impl TrustedRandomAccess for vec::IntoIter --- library/alloc/src/vec.rs | 23 ++++++++++++++++++++++- library/core/src/iter/adapters/mod.rs | 4 +++- library/core/src/iter/mod.rs | 4 +++- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index 9327cf16c1593..27f2440ddc03b 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -58,7 +58,9 @@ use core::cmp::{self, Ordering}; use core::fmt; use core::hash::{Hash, Hasher}; use core::intrinsics::{arith_offset, assume}; -use core::iter::{FromIterator, FusedIterator, InPlaceIterable, SourceIter, TrustedLen}; +use core::iter::{ + FromIterator, FusedIterator, InPlaceIterable, SourceIter, TrustedLen, TrustedRandomAccess, +}; use core::marker::PhantomData; use core::mem::{self, ManuallyDrop, MaybeUninit}; use core::ops::Bound::{Excluded, Included, Unbounded}; @@ -2936,6 +2938,25 @@ impl FusedIterator for IntoIter {} #[unstable(feature = "trusted_len", issue = "37572")] unsafe impl TrustedLen for IntoIter {} +#[doc(hidden)] +#[unstable(issue = "0", feature = "std_internals")] +// T: Copy as approximation for !Drop since get_unchecked does not advance self.ptr +// and thus we can't implement drop-handling +unsafe impl TrustedRandomAccess for IntoIter +where + T: Copy, +{ + unsafe fn get_unchecked(&mut self, i: usize) -> Self::Item { + unsafe { + if mem::size_of::() == 0 { mem::zeroed() } else { ptr::read(self.ptr.add(i)) } + } + } + + fn may_have_side_effect() -> bool { + false + } +} + #[stable(feature = "vec_into_iter_clone", since = "1.8.0")] impl Clone for IntoIter { fn clone(&self) -> IntoIter { diff --git a/library/core/src/iter/adapters/mod.rs b/library/core/src/iter/adapters/mod.rs index 0ad279c55337a..12fdd3f49c331 100644 --- a/library/core/src/iter/adapters/mod.rs +++ b/library/core/src/iter/adapters/mod.rs @@ -18,7 +18,8 @@ pub use self::chain::Chain; pub use self::flatten::{FlatMap, Flatten}; pub use self::fuse::Fuse; use self::zip::try_get_unchecked; -pub(crate) use self::zip::TrustedRandomAccess; +#[unstable(feature = "trusted_random_access", issue = "none")] +pub use self::zip::TrustedRandomAccess; pub use self::zip::Zip; /// This trait provides transitive access to source-stages in an interator-adapter pipeline @@ -480,6 +481,7 @@ where unsafe impl TrustedRandomAccess for Cloned where I: TrustedRandomAccess, + { #[inline] fn may_have_side_effect() -> bool { diff --git a/library/core/src/iter/mod.rs b/library/core/src/iter/mod.rs index f35994560c56a..922ffd1e0b588 100644 --- a/library/core/src/iter/mod.rs +++ b/library/core/src/iter/mod.rs @@ -357,6 +357,8 @@ pub use self::adapters::MapWhile; pub use self::adapters::SourceIter; #[stable(feature = "iterator_step_by", since = "1.28.0")] pub use self::adapters::StepBy; +#[unstable(feature = "trusted_random_access", issue = "none")] +pub use self::adapters::TrustedRandomAccess; #[stable(feature = "rust1", since = "1.0.0")] pub use self::adapters::{Chain, Cycle, Enumerate, Filter, FilterMap, Map, Rev, Zip}; #[stable(feature = "rust1", since = "1.0.0")] @@ -364,7 +366,7 @@ pub use self::adapters::{FlatMap, Peekable, Scan, Skip, SkipWhile, Take, TakeWhi #[stable(feature = "rust1", since = "1.0.0")] pub use self::adapters::{Fuse, Inspect}; -pub(crate) use self::adapters::{process_results, TrustedRandomAccess}; +pub(crate) use self::adapters::process_results; mod adapters; mod range; From 08567712483a4ad3846deafce73c6baea4989ac1 Mon Sep 17 00:00:00 2001 From: The8472 Date: Sat, 30 Nov 2019 15:35:18 +0100 Subject: [PATCH 37/63] fix build issue due to stabilized feature --- library/alloc/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/library/alloc/src/lib.rs b/library/alloc/src/lib.rs index 81169c1dd75e7..390535358f33e 100644 --- a/library/alloc/src/lib.rs +++ b/library/alloc/src/lib.rs @@ -107,6 +107,7 @@ #![feature(map_first_last)] #![feature(map_into_keys_values)] #![feature(negative_impls)] +#![cfg_attr(bootstrap, feature(never_type))] #![feature(new_uninit)] #![feature(nll)] #![feature(nonnull_slice_from_raw_parts)] From c731648e77a0986b435b82177c8b3ffcd0714c9a Mon Sep 17 00:00:00 2001 From: The8472 Date: Sat, 30 Nov 2019 19:40:28 +0100 Subject: [PATCH 38/63] fix: bench didn't black_box its results --- library/alloc/benches/vec.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/library/alloc/benches/vec.rs b/library/alloc/benches/vec.rs index 1ae72a24709bf..39f67c664d475 100644 --- a/library/alloc/benches/vec.rs +++ b/library/alloc/benches/vec.rs @@ -643,9 +643,7 @@ fn bench_rev_1(b: &mut test::Bencher) { #[bench] fn bench_rev_2(b: &mut test::Bencher) { let data = black_box([0; LEN]); - b.iter(|| { - example_plain_slow(&data); - }); + b.iter(|| example_plain_slow(&data)); } #[bench] From 2a51e579f5ce78c490f0ed584e83cb376a634e9e Mon Sep 17 00:00:00 2001 From: The8472 Date: Fri, 20 Dec 2019 20:28:10 +0100 Subject: [PATCH 39/63] avoid exposing that binary heap's IntoIter is backed by vec::IntoIter, use a private trait instead --- library/alloc/src/collections/binary_heap.rs | 12 ++++++++--- library/alloc/src/lib.rs | 2 +- library/alloc/src/vec.rs | 21 +++++++++++++++----- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/library/alloc/src/collections/binary_heap.rs b/library/alloc/src/collections/binary_heap.rs index 73f119c680f20..d133204d66c5c 100644 --- a/library/alloc/src/collections/binary_heap.rs +++ b/library/alloc/src/collections/binary_heap.rs @@ -151,7 +151,7 @@ use core::ops::{Deref, DerefMut}; use core::ptr; use crate::slice; -use crate::vec::{self, Vec}; +use crate::vec::{self, Vec, AsIntoIter}; use super::SpecExtend; @@ -1175,17 +1175,23 @@ impl FusedIterator for IntoIter {} #[unstable(issue = "0", feature = "inplace_iteration")] unsafe impl SourceIter for IntoIter { - type Source = impl Iterator; + type Source = IntoIter; #[inline] fn as_inner(&mut self) -> &mut Self::Source { - &mut self.iter + self } } #[unstable(issue = "0", feature = "inplace_iteration")] unsafe impl InPlaceIterable for IntoIter {} +impl AsIntoIter for IntoIter { + fn as_into_iter(&mut self) -> &mut vec::IntoIter { + &mut self.iter + } +} + #[unstable(feature = "binary_heap_into_iter_sorted", issue = "59278")] #[derive(Clone, Debug)] pub struct IntoIterSorted { diff --git a/library/alloc/src/lib.rs b/library/alloc/src/lib.rs index 390535358f33e..5cbd9d380b0d9 100644 --- a/library/alloc/src/lib.rs +++ b/library/alloc/src/lib.rs @@ -107,7 +107,7 @@ #![feature(map_first_last)] #![feature(map_into_keys_values)] #![feature(negative_impls)] -#![cfg_attr(bootstrap, feature(never_type))] +#![feature(never_type)] #![feature(new_uninit)] #![feature(nll)] #![feature(nonnull_slice_from_raw_parts)] diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index 27f2440ddc03b..8cca3d904ac61 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2221,7 +2221,7 @@ impl SpecFrom> for Vec { // T can be split into IN and OUT which only need to have the same size and alignment impl SpecFrom for Vec where - I: Iterator + InPlaceIterable + SourceIter>, + I: Iterator + InPlaceIterable + SourceIter>, { default fn from_iter(mut iterator: I) -> Self { // This specialization only makes sense if we're juggling real allocations. @@ -2230,8 +2230,8 @@ where return SpecFromNested::from_iter(iterator); } - let src_buf = iterator.as_inner().buf.as_ptr(); - let src_end = iterator.as_inner().end; + let src_buf = iterator.as_inner().as_into_iter().buf.as_ptr(); + let src_end = iterator.as_inner().as_into_iter().end; let dst = src_buf; let dst = if mem::needs_drop::() { @@ -2273,14 +2273,14 @@ where .unwrap() }; - let src = iterator.as_inner(); + let src = iterator.as_inner().as_into_iter(); // check if SourceIter and InPlaceIterable contracts were upheld. // caveat: if they weren't we may not even make it to this point debug_assert_eq!(src_buf, src.buf.as_ptr()); debug_assert!(dst as *const _ <= src.ptr, "InPlaceIterable contract violation"); if mem::needs_drop::() { - // drop tail if iterator was only partially exhaused + // drop tail if iterator was only partially exhausted unsafe { ptr::drop_in_place(src.as_mut_slice()); } @@ -2998,6 +2998,17 @@ unsafe impl SourceIter for IntoIter { } } +// internal helper trait for in-place iteration specialization. +pub(crate) trait AsIntoIter { + fn as_into_iter(&mut self) -> &mut IntoIter; +} + +impl AsIntoIter for IntoIter { + fn as_into_iter(&mut self) -> &mut IntoIter { + self + } +} + /// A draining iterator for `Vec`. /// /// This `struct` is created by [`Vec::drain`]. From ab382b76616e20bdb29973d44aa220341133d407 Mon Sep 17 00:00:00 2001 From: The8472 Date: Sat, 18 Jan 2020 16:03:58 +0100 Subject: [PATCH 40/63] mark as_inner as unsafe and update comments --- library/alloc/src/collections/binary_heap.rs | 2 +- library/alloc/src/vec.rs | 12 ++- library/core/src/iter/adapters/fuse.rs | 5 +- library/core/src/iter/adapters/mod.rs | 95 +++++++++++--------- library/core/src/iter/adapters/zip.rs | 12 ++- 5 files changed, 76 insertions(+), 50 deletions(-) diff --git a/library/alloc/src/collections/binary_heap.rs b/library/alloc/src/collections/binary_heap.rs index d133204d66c5c..49d3213217c5d 100644 --- a/library/alloc/src/collections/binary_heap.rs +++ b/library/alloc/src/collections/binary_heap.rs @@ -1178,7 +1178,7 @@ unsafe impl SourceIter for IntoIter { type Source = IntoIter; #[inline] - fn as_inner(&mut self) -> &mut Self::Source { + unsafe fn as_inner(&mut self) -> &mut Self::Source { self } } diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index 8cca3d904ac61..9d0ab47f8f250 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2167,6 +2167,8 @@ where } } +// A helper struct for in-place iteration that drops the destination slice of iteration. +// The source slice is dropped by IntoIter struct InPlaceDrop { inner: *mut T, dst: *mut T, @@ -2230,8 +2232,10 @@ where return SpecFromNested::from_iter(iterator); } - let src_buf = iterator.as_inner().as_into_iter().buf.as_ptr(); - let src_end = iterator.as_inner().as_into_iter().end; + let (src_buf, src_end) = { + let inner = unsafe { iterator.as_inner().as_into_iter() }; + (inner.buf.as_ptr(), inner.end) + }; let dst = src_buf; let dst = if mem::needs_drop::() { @@ -2273,7 +2277,7 @@ where .unwrap() }; - let src = iterator.as_inner().as_into_iter(); + let src = unsafe { iterator.as_inner().as_into_iter() }; // check if SourceIter and InPlaceIterable contracts were upheld. // caveat: if they weren't we may not even make it to this point debug_assert_eq!(src_buf, src.buf.as_ptr()); @@ -2993,7 +2997,7 @@ unsafe impl SourceIter for IntoIter { type Source = IntoIter; #[inline] - fn as_inner(&mut self) -> &mut Self::Source { + unsafe fn as_inner(&mut self) -> &mut Self::Source { self } } diff --git a/library/core/src/iter/adapters/fuse.rs b/library/core/src/iter/adapters/fuse.rs index e2613be4a46a1..1a4b3d379e435 100644 --- a/library/core/src/iter/adapters/fuse.rs +++ b/library/core/src/iter/adapters/fuse.rs @@ -529,9 +529,10 @@ unsafe impl SourceIter for Fuse type Source = S; #[inline] - fn as_inner(&mut self) -> &mut S { + unsafe fn as_inner(&mut self) -> &mut S { match self.iter { - Some(ref mut iter) => SourceIter::as_inner(iter), + // Safety: unsafe function forwarding to unsafe function with the same requirements + Some(ref mut iter) => unsafe { SourceIter::as_inner(iter) }, // SAFETY: the specialized iterator never sets `None` None => unsafe { intrinsics::unreachable() }, } diff --git a/library/core/src/iter/adapters/mod.rs b/library/core/src/iter/adapters/mod.rs index 12fdd3f49c331..2b9d54f7eab5e 100644 --- a/library/core/src/iter/adapters/mod.rs +++ b/library/core/src/iter/adapters/mod.rs @@ -22,7 +22,7 @@ use self::zip::try_get_unchecked; pub use self::zip::TrustedRandomAccess; pub use self::zip::Zip; -/// This trait provides transitive access to source-stages in an interator-adapter pipeline +/// This trait provides transitive access to source-stage in an interator-adapter pipeline /// under the conditions that /// * the iterator source `S` itself implements `SourceIter` /// * there is a delegating implementation of this trait for each adapter in the pipeline between @@ -49,40 +49,44 @@ pub use self::zip::Zip; /// /// let mut iter = vec![9, 9, 9].into_iter().map(|i| i * i); /// let _ = iter.next(); -/// let mut remainder = std::mem::replace(iter.as_inner(), Vec::new().into_iter()); +/// let mut remainder = std::mem::replace(unsafe { iter.as_inner() }, Vec::new().into_iter()); /// println!("n = {} elements remaining", remainder.len()); /// ``` /// -/// [`FromIterator`]: trait.FromIterator.html -/// [`as_inner`]: #method.as_inner +/// [`FromIterator`]: crate::iter::FromIterator +/// [`as_inner`]: SourceIter::as_inner #[unstable(issue = "0", feature = "inplace_iteration")] pub unsafe trait SourceIter { /// A source stage in an iterator pipeline. type Source: Iterator; - /// Extract the source of an iterator pipeline. + /// Retrieve the source of an iterator pipeline. /// - /// Callers may assume that calls to [`next()`] or any method taking `&self` - /// does no replace the referenced value. - /// But callers may replace the referenced values as long they in turn do not - /// expose it through a delegating implementation of this trait. - /// Which means that while adapters may not modify the reference they cannot - /// rely on it not being modified. + /// # Safety /// - /// Adapters must not rely on exclusive ownership or immutability of the source. - /// The lack of exclusive ownership also requires that adapters must uphold the source's - /// public API even when they have crate- or module-internal access. + /// Implementations of must return the same mutable reference for their lifetime, unless + /// replaced by a caller. + /// Callers may only replace the reference when they stopped iteration and drop the + /// iterator pipeline after extracting the source. + /// + /// This means iterator adapters can rely on the source not changing during + /// iteration but they cannot rely on it in their Drop implementations. + /// + /// Implementing this method means adapters relinquish private-only access to their + /// source and can only rely on guarantees made based on method receiver types. + /// The lack of restricted access also requires that adapters must uphold the source's + /// public API even when they have access to its internals. /// /// Callers in turn must expect the source to be in any state that is consistent with /// its public API since adapters sitting between it and the source have the same /// access. In particular an adapter may have consumed more elements than strictly necessary. /// - /// The overall goal of these requirements is to grant the consumer of a pipeline - /// access to the underlying storage of an iterator while restricting any statefulness - /// and side-effects of the pipeline stages from affecting or relying on that storage. + /// The overall goal of these requirements is to let the consumer of a pipeline use + /// * whatever remains in the source after iteration has stopped + /// * the memory that has become unused by advancing a consuming iterator /// /// [`next()`]: trait.Iterator.html#method.next - fn as_inner(&mut self) -> &mut Self::Source; + unsafe fn as_inner(&mut self) -> &mut Self::Source; } /// A double-ended iterator with the direction inverted. @@ -1015,8 +1019,9 @@ where type Source = S; #[inline] - fn as_inner(&mut self) -> &mut S { - SourceIter::as_inner(&mut self.iter) + unsafe fn as_inner(&mut self) -> &mut S { + // Safety: unsafe function forwarding to unsafe function with the same requirements + unsafe { SourceIter::as_inner(&mut self.iter) } } } @@ -1162,8 +1167,9 @@ unsafe impl SourceIter for Filter where type Source = S; #[inline] - fn as_inner(&mut self) -> &mut S { - SourceIter::as_inner(&mut self.iter) + unsafe fn as_inner(&mut self) -> &mut S { + // Safety: unsafe function forwarding to unsafe function with the same requirements + unsafe { SourceIter::as_inner(&mut self.iter) } } } @@ -1305,8 +1311,9 @@ unsafe impl SourceIter for FilterMap where type Source = S; #[inline] - fn as_inner(&mut self) -> &mut S { - SourceIter::as_inner(&mut self.iter) + unsafe fn as_inner(&mut self) -> &mut S { + // Safety: unsafe function forwarding to unsafe function with the same requirements + unsafe { SourceIter::as_inner(&mut self.iter) } } } @@ -1541,8 +1548,9 @@ where type Source = S; #[inline] - fn as_inner(&mut self) -> &mut S { - SourceIter::as_inner(&mut self.iter) + unsafe fn as_inner(&mut self) -> &mut S { + // Safety: unsafe function forwarding to unsafe function with the same requirements + unsafe { SourceIter::as_inner(&mut self.iter) } } } @@ -1838,8 +1846,9 @@ where type Source = S; #[inline] - fn as_inner(&mut self) -> &mut S { - SourceIter::as_inner(&mut self.iter) + unsafe fn as_inner(&mut self) -> &mut S { + // Safety: unsafe function forwarding to unsafe function with the same requirements + unsafe { SourceIter::as_inner(&mut self.iter) } } } @@ -1955,8 +1964,9 @@ unsafe impl SourceIter for SkipWhile where type Source = S; #[inline] - fn as_inner(&mut self) -> &mut S { - SourceIter::as_inner(&mut self.iter) + unsafe fn as_inner(&mut self) -> &mut S { + // Safety: unsafe function forwarding to unsafe function with the same requirements + unsafe { SourceIter::as_inner(&mut self.iter) } } } @@ -2163,8 +2173,9 @@ unsafe impl SourceIter for TakeWhile where type Source = S; #[inline] - fn as_inner(&mut self) -> &mut S { - SourceIter::as_inner(&mut self.iter) + unsafe fn as_inner(&mut self) -> &mut S { + // Safety: unsafe function forwarding to unsafe function with the same requirements + unsafe { SourceIter::as_inner(&mut self.iter) } } } @@ -2364,8 +2375,9 @@ where type Source = S; #[inline] - fn as_inner(&mut self) -> &mut S { - SourceIter::as_inner(&mut self.iter) + unsafe fn as_inner(&mut self) -> &mut S { + // Safety: unsafe function forwarding to unsafe function with the same requirements + unsafe { SourceIter::as_inner(&mut self.iter) } } } @@ -2487,8 +2499,9 @@ unsafe impl SourceIter for Take where I: SourceIter type Source = S; #[inline] - fn as_inner(&mut self) -> &mut S { - SourceIter::as_inner(&mut self.iter) + unsafe fn as_inner(&mut self) -> &mut S { + // Safety: unsafe function forwarding to unsafe function with the same requirements + unsafe { SourceIter::as_inner(&mut self.iter) } } } @@ -2667,8 +2680,9 @@ unsafe impl SourceIter for Scan type Source = S; #[inline] - fn as_inner(&mut self) -> &mut S { - SourceIter::as_inner(&mut self.iter) + unsafe fn as_inner(&mut self) -> &mut S { + // Safety: unsafe function forwarding to unsafe function with the same requirements + unsafe { SourceIter::as_inner(&mut self.iter) } } } @@ -2831,8 +2845,9 @@ unsafe impl SourceIter for Inspect where type Source = S; #[inline] - fn as_inner(&mut self) -> &mut S { - SourceIter::as_inner(&mut self.iter) + unsafe fn as_inner(&mut self) -> &mut S { + // Safety: unsafe function forwarding to unsafe function with the same requirements + unsafe { SourceIter::as_inner(&mut self.iter) } } } diff --git a/library/core/src/iter/adapters/zip.rs b/library/core/src/iter/adapters/zip.rs index 6624b5223e327..d52ae1b05b44b 100644 --- a/library/core/src/iter/adapters/zip.rs +++ b/library/core/src/iter/adapters/zip.rs @@ -342,13 +342,19 @@ unsafe impl SourceIter for Zip type Source = S; #[inline] - fn as_inner(&mut self) -> &mut S { - SourceIter::as_inner(&mut self.a) + unsafe fn as_inner(&mut self) -> &mut S { + // Safety: unsafe function forwarding to unsafe function with the same requirements + unsafe { SourceIter::as_inner(&mut self.a) } } } #[unstable(issue = "0", feature = "inplace_iteration")] -unsafe impl InPlaceIterable for Zip {} +// Limited to Item: Copy since interaction between Zip's use of TrustedRandomAccess +// and Drop implementation of the source is unclear. +// +// An additional method returning the number of times the source has been logically advanced +// (without calling next()) would be needed to properly drop the remainder of the source. +unsafe impl InPlaceIterable for Zip where A::Item: Copy {} #[stable(feature = "rust1", since = "1.0.0")] impl Debug for Zip { From 6ed05fd99573c481e2484edc28f18588e9135d1f Mon Sep 17 00:00:00 2001 From: The8472 Date: Sat, 18 Jan 2020 16:13:28 +0100 Subject: [PATCH 41/63] replace drop flag with ManuallyDrop --- library/alloc/src/vec.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index 9d0ab47f8f250..27618f4a38827 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2172,7 +2172,6 @@ where struct InPlaceDrop { inner: *mut T, dst: *mut T, - did_panic: bool, } impl InPlaceDrop { @@ -2185,9 +2184,7 @@ impl Drop for InPlaceDrop { #[inline] fn drop(&mut self) { unsafe { - if self.did_panic { - ptr::drop_in_place(slice::from_raw_parts_mut(self.inner, self.len()) as *mut _); - } + ptr::drop_in_place(slice::from_raw_parts_mut(self.inner, self.len()) as *mut _); } } } @@ -2240,7 +2237,7 @@ where let dst = if mem::needs_drop::() { // special-case drop handling since it prevents vectorization - let mut sink = InPlaceDrop { inner: src_buf, dst, did_panic: true }; + let mut sink = InPlaceDrop { inner: src_buf, dst }; let _ = iterator.try_for_each::<_, Result<_, !>>(|item| { unsafe { debug_assert!( @@ -2252,7 +2249,8 @@ where } Ok(()) }); - sink.did_panic = false; + // iteration succeeded, don't drop head + let sink = mem::ManuallyDrop::new(sink); sink.dst } else { // use try-fold From 9596e5a2f23771bdbf68b2872b26cce715c8011e Mon Sep 17 00:00:00 2001 From: The8472 Date: Wed, 15 Jul 2020 20:53:03 +0200 Subject: [PATCH 42/63] pacify tidy --- library/alloc/src/collections/binary_heap.rs | 6 +- library/alloc/src/vec.rs | 6 +- library/core/src/iter/adapters/fuse.rs | 5 +- library/core/src/iter/adapters/mod.rs | 137 ++++++++++--------- library/core/src/iter/adapters/zip.rs | 4 +- library/core/src/iter/mod.rs | 5 +- library/core/src/iter/traits/marker.rs | 2 +- library/core/src/iter/traits/mod.rs | 2 +- 8 files changed, 90 insertions(+), 77 deletions(-) diff --git a/library/alloc/src/collections/binary_heap.rs b/library/alloc/src/collections/binary_heap.rs index 49d3213217c5d..226448b4379a0 100644 --- a/library/alloc/src/collections/binary_heap.rs +++ b/library/alloc/src/collections/binary_heap.rs @@ -151,7 +151,7 @@ use core::ops::{Deref, DerefMut}; use core::ptr; use crate::slice; -use crate::vec::{self, Vec, AsIntoIter}; +use crate::vec::{self, AsIntoIter, Vec}; use super::SpecExtend; @@ -1173,7 +1173,7 @@ impl ExactSizeIterator for IntoIter { #[stable(feature = "fused", since = "1.26.0")] impl FusedIterator for IntoIter {} -#[unstable(issue = "0", feature = "inplace_iteration")] +#[unstable(issue = "none", feature = "inplace_iteration")] unsafe impl SourceIter for IntoIter { type Source = IntoIter; @@ -1183,7 +1183,7 @@ unsafe impl SourceIter for IntoIter { } } -#[unstable(issue = "0", feature = "inplace_iteration")] +#[unstable(issue = "none", feature = "inplace_iteration")] unsafe impl InPlaceIterable for IntoIter {} impl AsIntoIter for IntoIter { diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index 27618f4a38827..9b7f2af3ba9e7 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2941,7 +2941,7 @@ impl FusedIterator for IntoIter {} unsafe impl TrustedLen for IntoIter {} #[doc(hidden)] -#[unstable(issue = "0", feature = "std_internals")] +#[unstable(issue = "none", feature = "std_internals")] // T: Copy as approximation for !Drop since get_unchecked does not advance self.ptr // and thus we can't implement drop-handling unsafe impl TrustedRandomAccess for IntoIter @@ -2987,10 +2987,10 @@ unsafe impl<#[may_dangle] T> Drop for IntoIter { } } -#[unstable(issue = "0", feature = "inplace_iteration")] +#[unstable(issue = "none", feature = "inplace_iteration")] unsafe impl InPlaceIterable for IntoIter {} -#[unstable(issue = "0", feature = "inplace_iteration")] +#[unstable(issue = "none", feature = "inplace_iteration")] unsafe impl SourceIter for IntoIter { type Source = IntoIter; diff --git a/library/core/src/iter/adapters/fuse.rs b/library/core/src/iter/adapters/fuse.rs index 1a4b3d379e435..280f1075be69a 100644 --- a/library/core/src/iter/adapters/fuse.rs +++ b/library/core/src/iter/adapters/fuse.rs @@ -520,8 +520,7 @@ where } } - -#[unstable(issue = "0", feature = "inplace_iteration")] +#[unstable(issue = "none", feature = "inplace_iteration")] unsafe impl SourceIter for Fuse where I: SourceIter, @@ -539,5 +538,5 @@ unsafe impl SourceIter for Fuse } } -#[unstable(issue = "0", feature = "inplace_iteration")] +#[unstable(issue = "none", feature = "inplace_iteration")] unsafe impl InPlaceIterable for Fuse {} diff --git a/library/core/src/iter/adapters/mod.rs b/library/core/src/iter/adapters/mod.rs index 2b9d54f7eab5e..ed5600f1824a5 100644 --- a/library/core/src/iter/adapters/mod.rs +++ b/library/core/src/iter/adapters/mod.rs @@ -55,7 +55,7 @@ pub use self::zip::Zip; /// /// [`FromIterator`]: crate::iter::FromIterator /// [`as_inner`]: SourceIter::as_inner -#[unstable(issue = "0", feature = "inplace_iteration")] +#[unstable(issue = "none", feature = "inplace_iteration")] pub unsafe trait SourceIter { /// A source stage in an iterator pipeline. type Source: Iterator; @@ -1010,7 +1010,7 @@ where } } -#[unstable(issue = "0", feature = "inplace_iteration")] +#[unstable(issue = "none", feature = "inplace_iteration")] unsafe impl SourceIter for Map where F: FnMut(I::Item) -> B, @@ -1025,7 +1025,7 @@ where } } -#[unstable(issue = "0", feature = "inplace_iteration")] +#[unstable(issue = "none", feature = "inplace_iteration")] unsafe impl InPlaceIterable for Map where F: FnMut(I::Item) -> B {} /// An iterator that filters the elements of `iter` with `predicate`. @@ -1159,10 +1159,11 @@ where #[stable(feature = "fused", since = "1.26.0")] impl FusedIterator for Filter where P: FnMut(&I::Item) -> bool {} -#[unstable(issue = "0", feature = "inplace_iteration")] -unsafe impl SourceIter for Filter where +#[unstable(issue = "none", feature = "inplace_iteration")] +unsafe impl SourceIter for Filter +where P: FnMut(&I::Item) -> bool, - I: SourceIter + I: SourceIter, { type Source = S; @@ -1173,9 +1174,8 @@ unsafe impl SourceIter for Filter where } } -#[unstable(issue = "0", feature = "inplace_iteration")] -unsafe impl InPlaceIterable for Filter - where P: FnMut(&I::Item) -> bool {} +#[unstable(issue = "none", feature = "inplace_iteration")] +unsafe impl InPlaceIterable for Filter where P: FnMut(&I::Item) -> bool {} /// An iterator that uses `f` to both filter and map elements from `iter`. /// @@ -1303,10 +1303,11 @@ where #[stable(feature = "fused", since = "1.26.0")] impl FusedIterator for FilterMap where F: FnMut(I::Item) -> Option {} -#[unstable(issue = "0", feature = "inplace_iteration")] -unsafe impl SourceIter for FilterMap where +#[unstable(issue = "none", feature = "inplace_iteration")] +unsafe impl SourceIter for FilterMap +where F: FnMut(I::Item) -> Option, - I: SourceIter + I: SourceIter, { type Source = S; @@ -1317,10 +1318,11 @@ unsafe impl SourceIter for FilterMap where } } -#[unstable(issue = "0", feature = "inplace_iteration")] -unsafe impl InPlaceIterable for FilterMap - where F: FnMut(I::Item) -> Option {} - +#[unstable(issue = "none", feature = "inplace_iteration")] +unsafe impl InPlaceIterable for FilterMap where + F: FnMut(I::Item) -> Option +{ +} /// An iterator that yields the current count and the element during iteration. /// @@ -1540,7 +1542,7 @@ impl FusedIterator for Enumerate where I: FusedIterator {} #[unstable(feature = "trusted_len", issue = "37572")] unsafe impl TrustedLen for Enumerate where I: TrustedLen {} -#[unstable(issue = "0", feature = "inplace_iteration")] +#[unstable(issue = "none", feature = "inplace_iteration")] unsafe impl SourceIter for Enumerate where I: SourceIter, @@ -1554,7 +1556,7 @@ where } } -#[unstable(issue = "0", feature = "inplace_iteration")] +#[unstable(issue = "none", feature = "inplace_iteration")] unsafe impl InPlaceIterable for Enumerate {} /// An iterator with a `peek()` that returns an optional reference to the next @@ -1838,7 +1840,7 @@ impl Peekable { #[unstable(feature = "trusted_len", issue = "37572")] unsafe impl TrustedLen for Peekable where I: TrustedLen {} -#[unstable(issue = "0", feature = "inplace_iteration")] +#[unstable(issue = "none", feature = "inplace_iteration")] unsafe impl SourceIter for Peekable where I: SourceIter, @@ -1852,7 +1854,7 @@ where } } -#[unstable(issue = "0", feature = "inplace_iteration")] +#[unstable(issue = "none", feature = "inplace_iteration")] unsafe impl InPlaceIterable for Peekable {} /// An iterator that rejects elements while `predicate` returns `true`. @@ -1956,10 +1958,11 @@ where { } -#[unstable(issue = "0", feature = "inplace_iteration")] -unsafe impl SourceIter for SkipWhile where +#[unstable(issue = "none", feature = "inplace_iteration")] +unsafe impl SourceIter for SkipWhile +where P: FnMut(&I::Item) -> bool, - I: SourceIter + I: SourceIter, { type Source = S; @@ -1970,9 +1973,11 @@ unsafe impl SourceIter for SkipWhile where } } -#[unstable(issue = "0", feature = "inplace_iteration")] -unsafe impl InPlaceIterable for SkipWhile - where F: FnMut(&I::Item) -> bool {} +#[unstable(issue = "none", feature = "inplace_iteration")] +unsafe impl InPlaceIterable for SkipWhile where + F: FnMut(&I::Item) -> bool +{ +} /// An iterator that only accepts elements while `predicate` returns `true`. /// @@ -2088,6 +2093,27 @@ where { } +#[unstable(issue = "none", feature = "inplace_iteration")] +unsafe impl SourceIter for TakeWhile + where + P: FnMut(&I::Item) -> bool, + I: SourceIter, +{ + type Source = S; + + #[inline] + unsafe fn as_inner(&mut self) -> &mut S { + // Safety: unsafe function forwarding to unsafe function with the same requirements + unsafe { SourceIter::as_inner(&mut self.iter) } + } +} + +#[unstable(issue = "none", feature = "inplace_iteration")] +unsafe impl InPlaceIterable for TakeWhile where + F: FnMut(&I::Item) -> bool +{ +} + /// An iterator that only accepts elements while `predicate` returns `Some(_)`. /// /// This `struct` is created by the [`map_while`] method on [`Iterator`]. See its @@ -2165,25 +2191,6 @@ where } } -#[unstable(issue = "0", feature = "inplace_iteration")] -unsafe impl SourceIter for TakeWhile where - P: FnMut(&I::Item) -> bool, - I: SourceIter -{ - type Source = S; - - #[inline] - unsafe fn as_inner(&mut self) -> &mut S { - // Safety: unsafe function forwarding to unsafe function with the same requirements - unsafe { SourceIter::as_inner(&mut self.iter) } - } -} - -#[unstable(issue = "0", feature = "inplace_iteration")] -unsafe impl InPlaceIterable for TakeWhile - where F: FnMut(&I::Item) -> bool {} - - /// An iterator that skips over `n` elements of `iter`. /// /// This `struct` is created by the [`skip`] method on [`Iterator`]. See its @@ -2367,7 +2374,7 @@ where #[stable(feature = "fused", since = "1.26.0")] impl FusedIterator for Skip where I: FusedIterator {} -#[unstable(issue = "0", feature = "inplace_iteration")] +#[unstable(issue = "none", feature = "inplace_iteration")] unsafe impl SourceIter for Skip where I: SourceIter, @@ -2381,7 +2388,7 @@ where } } -#[unstable(issue = "0", feature = "inplace_iteration")] +#[unstable(issue = "none", feature = "inplace_iteration")] unsafe impl InPlaceIterable for Skip {} /// An iterator that only iterates over the first `n` iterations of `iter`. @@ -2494,8 +2501,11 @@ where } } -#[unstable(issue = "0", feature = "inplace_iteration")] -unsafe impl SourceIter for Take where I: SourceIter { +#[unstable(issue = "none", feature = "inplace_iteration")] +unsafe impl SourceIter for Take +where + I: SourceIter, +{ type Source = S; #[inline] @@ -2505,7 +2515,7 @@ unsafe impl SourceIter for Take where I: SourceIter } } -#[unstable(issue = "0", feature = "inplace_iteration")] +#[unstable(issue = "none", feature = "inplace_iteration")] unsafe impl InPlaceIterable for Take {} #[stable(feature = "double_ended_take_iterator", since = "1.38.0")] @@ -2672,10 +2682,11 @@ where } } -#[unstable(issue = "0", feature = "inplace_iteration")] +#[unstable(issue = "none", feature = "inplace_iteration")] unsafe impl SourceIter for Scan - where I: SourceIter, - F: FnMut(&mut St, I::Item) -> Option, +where + I: SourceIter, + F: FnMut(&mut St, I::Item) -> Option, { type Source = S; @@ -2686,10 +2697,11 @@ unsafe impl SourceIter for Scan } } -#[unstable(issue = "0", feature = "inplace_iteration")] -unsafe impl InPlaceIterable for Scan - where F: FnMut(&mut St, I::Item) -> Option, -{} +#[unstable(issue = "none", feature = "inplace_iteration")] +unsafe impl InPlaceIterable for Scan where + F: FnMut(&mut St, I::Item) -> Option +{ +} /// An iterator that calls a function with a reference to each element before /// yielding it. @@ -2837,10 +2849,11 @@ where #[stable(feature = "fused", since = "1.26.0")] impl FusedIterator for Inspect where F: FnMut(&I::Item) {} -#[unstable(issue = "0", feature = "inplace_iteration")] -unsafe impl SourceIter for Inspect where +#[unstable(issue = "none", feature = "inplace_iteration")] +unsafe impl SourceIter for Inspect +where F: FnMut(&I::Item), - I: SourceIter + I: SourceIter, { type Source = S; @@ -2851,8 +2864,8 @@ unsafe impl SourceIter for Inspect where } } -#[unstable(issue = "0", feature = "inplace_iteration")] -unsafe impl InPlaceIterable for Inspect where F: FnMut(&I::Item) {} +#[unstable(issue = "none", feature = "inplace_iteration")] +unsafe impl InPlaceIterable for Inspect where F: FnMut(&I::Item) {} /// An iterator adapter that produces output as long as the underlying /// iterator produces `Result::Ok` values. diff --git a/library/core/src/iter/adapters/zip.rs b/library/core/src/iter/adapters/zip.rs index d52ae1b05b44b..0597d95018505 100644 --- a/library/core/src/iter/adapters/zip.rs +++ b/library/core/src/iter/adapters/zip.rs @@ -332,7 +332,7 @@ where // Arbitrarily selects the left side of the zip iteration as extractable "source" // it would require negative trait bounds to be able to try both -#[unstable(issue = "0", feature = "inplace_iteration")] +#[unstable(issue = "none", feature = "inplace_iteration")] unsafe impl SourceIter for Zip where A: SourceIter, @@ -348,7 +348,7 @@ unsafe impl SourceIter for Zip } } -#[unstable(issue = "0", feature = "inplace_iteration")] +#[unstable(issue = "none", feature = "inplace_iteration")] // Limited to Item: Copy since interaction between Zip's use of TrustedRandomAccess // and Drop implementation of the source is unclear. // diff --git a/library/core/src/iter/mod.rs b/library/core/src/iter/mod.rs index 922ffd1e0b588..59f333e888b88 100644 --- a/library/core/src/iter/mod.rs +++ b/library/core/src/iter/mod.rs @@ -342,7 +342,7 @@ pub use self::traits::{DoubleEndedIterator, Extend, FromIterator, IntoIterator}; #[stable(feature = "rust1", since = "1.0.0")] pub use self::traits::{ExactSizeIterator, Product, Sum}; -#[unstable(issue = "0", feature = "inplace_iteration")] +#[unstable(issue = "none", feature = "inplace_iteration")] pub use self::traits::InPlaceIterable; #[stable(feature = "iter_cloned", since = "1.1.0")] @@ -351,9 +351,10 @@ pub use self::adapters::Cloned; pub use self::adapters::Copied; #[stable(feature = "iterator_flatten", since = "1.29.0")] pub use self::adapters::Flatten; + #[unstable(feature = "iter_map_while", reason = "recently added", issue = "68537")] pub use self::adapters::MapWhile; -#[unstable(issue = "0", feature = "inplace_iteration")] +#[unstable(issue = "none", feature = "inplace_iteration")] pub use self::adapters::SourceIter; #[stable(feature = "iterator_step_by", since = "1.28.0")] pub use self::adapters::StepBy; diff --git a/library/core/src/iter/traits/marker.rs b/library/core/src/iter/traits/marker.rs index 549f7972689e2..f287196da03ef 100644 --- a/library/core/src/iter/traits/marker.rs +++ b/library/core/src/iter/traits/marker.rs @@ -52,5 +52,5 @@ unsafe impl TrustedLen for &mut I {} /// In other words this trait indicates that an iterator pipeline can be collected in place. /// /// [`SourceIter`]: ../../std/iter/trait.SourceIter.html -#[unstable(issue = "0", feature = "inplace_iteration")] +#[unstable(issue = "none", feature = "inplace_iteration")] pub unsafe trait InPlaceIterable: Iterator {} diff --git a/library/core/src/iter/traits/mod.rs b/library/core/src/iter/traits/mod.rs index 9ed2de7313df1..880f8d831fd92 100644 --- a/library/core/src/iter/traits/mod.rs +++ b/library/core/src/iter/traits/mod.rs @@ -11,7 +11,7 @@ pub use self::double_ended::DoubleEndedIterator; pub use self::exact_size::ExactSizeIterator; #[stable(feature = "rust1", since = "1.0.0")] pub use self::iterator::Iterator; -#[unstable(issue = "0", feature = "inplace_iteration")] +#[unstable(issue = "none", feature = "inplace_iteration")] pub use self::marker::InPlaceIterable; #[stable(feature = "rust1", since = "1.0.0")] pub use self::marker::{FusedIterator, TrustedLen}; From 0d2d0334152f0025b00a826482f130f499e7d627 Mon Sep 17 00:00:00 2001 From: The8472 Date: Sat, 25 Jan 2020 15:01:39 +0100 Subject: [PATCH 43/63] replace unsafe ptr::write with deref-write, benchmarks show no difference --- library/alloc/src/vec.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index 9b7f2af3ba9e7..d2f92b7c0082c 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2086,11 +2086,8 @@ impl Extend for Vec { >::spec_extend(self, iter.into_iter()) } else { // if self has no allocation then use the more powerful from_iter specializations - let other = SpecFrom::from_iter(iter.into_iter()); - // replace self, don't run drop since self was empty - unsafe { - ptr::write(self, other); - } + // and overwrite self + *self = SpecFrom::from_iter(iter.into_iter()); } } @@ -2544,11 +2541,8 @@ impl<'a, T: 'a + Copy> Extend<&'a T> for Vec { self.spec_extend(iter.into_iter()) } else { // if self has no allocation then use the more powerful from_iter specializations - let other = SpecFrom::from_iter(iter.into_iter()); - // replace self, don't run drop since self was empty - unsafe { - ptr::write(self, other); - } + // and overwrite self + *self = SpecFrom::from_iter(iter.into_iter()); } } From fe350dd82d49bc58e1f03cc370d3f6162ccc1d72 Mon Sep 17 00:00:00 2001 From: The8472 Date: Sat, 25 Jan 2020 15:02:13 +0100 Subject: [PATCH 44/63] move unsafety into method, not relevant to caller --- library/alloc/src/vec.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index d2f92b7c0082c..43f5f32ee3222 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2172,8 +2172,8 @@ struct InPlaceDrop { } impl InPlaceDrop { - unsafe fn len(&self) -> usize { - self.dst.offset_from(self.inner) as usize + fn len(&self) -> usize { + unsafe { self.dst.offset_from(self.inner) as usize } } } From 470bf54f94280c3084d08ce7068652a8cd0d1ea5 Mon Sep 17 00:00:00 2001 From: The8472 Date: Sat, 25 Jan 2020 17:55:47 +0100 Subject: [PATCH 45/63] test drops during in-place iteration --- library/alloc/tests/vec.rs | 40 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/library/alloc/tests/vec.rs b/library/alloc/tests/vec.rs index cbff7d37cdf7b..761b76a887615 100644 --- a/library/alloc/tests/vec.rs +++ b/library/alloc/tests/vec.rs @@ -4,6 +4,7 @@ use std::fmt::Debug; use std::iter::InPlaceIterable; use std::mem::size_of; use std::panic::{catch_unwind, AssertUnwindSafe}; +use std::rc::Rc; use std::vec::{Drain, IntoIter}; struct DropCounter<'a> { @@ -826,6 +827,45 @@ fn test_from_iter_specialization_with_iterator_adapters() { assert_eq!(srcptr, sinkptr); } +#[test] +fn test_from_iter_specialization_head_tail_drop() { + let drop_count: Vec<_> = (0..=2).map(|_| Rc::new(())).collect(); + let src: Vec<_> = drop_count.iter().cloned().collect(); + let srcptr = src.as_ptr(); + let iter = src.into_iter(); + let sink: Vec<_> = iter.skip(1).take(1).collect(); + let sinkptr = sink.as_ptr(); + assert_eq!(srcptr, sinkptr, "specialization was applied"); + assert_eq!(Rc::strong_count(&drop_count[0]), 1, "front was dropped"); + assert_eq!(Rc::strong_count(&drop_count[1]), 2, "one element was collected"); + assert_eq!(Rc::strong_count(&drop_count[2]), 1, "tail was dropped"); + assert_eq!(sink.len(), 1); +} + +#[test] +fn test_from_iter_specialization_panic_drop() { + let drop_count: Vec<_> = (0..=2).map(|_| Rc::new(())).collect(); + let src: Vec<_> = drop_count.iter().cloned().collect(); + let iter = src.into_iter(); + + let _ = std::panic::catch_unwind(AssertUnwindSafe(|| { + let _ = iter + .enumerate() + .filter_map(|(i, e)| { + if i == 1 { + std::panic!("aborting iteration"); + } + Some(e) + }) + .collect::>(); + })); + + assert!( + drop_count.iter().map(Rc::strong_count).all(|count| count == 1), + "all items were dropped once" + ); +} + #[test] fn test_cow_from() { let borrowed: &[_] = &["borrowed", "(slice)"]; From a7a8b52e918500a36b48f0b123b20fd53a159403 Mon Sep 17 00:00:00 2001 From: The8472 Date: Sat, 25 Jan 2020 17:58:34 +0100 Subject: [PATCH 46/63] remove redundant cast --- library/alloc/src/vec.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index 43f5f32ee3222..e799a4ce852f0 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2181,7 +2181,7 @@ impl Drop for InPlaceDrop { #[inline] fn drop(&mut self) { unsafe { - ptr::drop_in_place(slice::from_raw_parts_mut(self.inner, self.len()) as *mut _); + ptr::drop_in_place(slice::from_raw_parts_mut(self.inner, self.len())); } } } From 6ad133443a7975b2e86a3bbba16da33c769eeabc Mon Sep 17 00:00:00 2001 From: The8472 Date: Sat, 25 Jan 2020 20:08:46 +0100 Subject: [PATCH 47/63] add benchmark to cover in-place extend --- library/alloc/benches/vec.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/library/alloc/benches/vec.rs b/library/alloc/benches/vec.rs index 39f67c664d475..1019503a8417a 100644 --- a/library/alloc/benches/vec.rs +++ b/library/alloc/benches/vec.rs @@ -236,6 +236,20 @@ fn do_bench_extend_from_slice(b: &mut Bencher, dst_len: usize, src_len: usize) { }); } +#[bench] +fn bench_extend_recycle(b: &mut Bencher) { + let mut data = vec![0; 1000]; + + b.iter(|| { + let tmp = std::mem::replace(&mut data, Vec::new()); + let mut to_extend = black_box(Vec::new()); + to_extend.extend(tmp.into_iter()); + std::mem::replace(&mut data, black_box(to_extend)); + }); + + black_box(data); +} + #[bench] fn bench_extend_from_slice_0000_0000(b: &mut Bencher) { do_bench_extend_from_slice(b, 0, 0) From 771b8ecc83ed55de617cdb29e05d509cfc745a8a Mon Sep 17 00:00:00 2001 From: The8472 Date: Sun, 26 Jan 2020 20:50:05 +0100 Subject: [PATCH 48/63] extract IntoIter drop/forget used by specialization into separate methods --- library/alloc/src/vec.rs | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index e799a4ce852f0..97dd04fbe7050 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2226,9 +2226,9 @@ where return SpecFromNested::from_iter(iterator); } - let (src_buf, src_end) = { + let (src_buf, src_end, cap) = { let inner = unsafe { iterator.as_inner().as_into_iter() }; - (inner.buf.as_ptr(), inner.end) + (inner.buf.as_ptr(), inner.end, inner.cap) }; let dst = src_buf; @@ -2278,23 +2278,15 @@ where debug_assert_eq!(src_buf, src.buf.as_ptr()); debug_assert!(dst as *const _ <= src.ptr, "InPlaceIterable contract violation"); - if mem::needs_drop::() { - // drop tail if iterator was only partially exhausted - unsafe { - ptr::drop_in_place(src.as_mut_slice()); - } - } + // drop any remaining values at the tail of the source + src.drop_in_place(); + // but prevent drop of the allocation itself once IntoIter goes out of scope + src.forget_in_place(); let vec = unsafe { let len = dst.offset_from(src_buf) as usize; - Vec::from_raw_parts(src.buf.as_ptr(), len, src.cap) + Vec::from_raw_parts(src_buf, len, cap) }; - // prevent drop of the underlying storage by turning the IntoIter into - // the equivalent of Vec::new().into_iter() - src.cap = 0; - src.buf = unsafe { NonNull::new_unchecked(RawVec::NEW.ptr()) }; - src.ptr = src.buf.as_ptr(); - src.end = src.buf.as_ptr(); vec } @@ -2839,6 +2831,24 @@ impl IntoIter { fn as_raw_mut_slice(&mut self) -> *mut [T] { ptr::slice_from_raw_parts_mut(self.ptr as *mut T, self.len()) } + + fn drop_in_place(&mut self) { + if mem::needs_drop::() { + unsafe { + ptr::drop_in_place(self.as_mut_slice()); + } + } + self.ptr = self.end; + } + + /// Relinquishes the backing allocation, equivalent to + /// `ptr::write(&mut self, Vec::new().into_iter())` + fn forget_in_place(&mut self) { + self.cap = 0; + self.buf = unsafe { NonNull::new_unchecked(RawVec::NEW.ptr()) }; + self.ptr = self.buf.as_ptr(); + self.end = self.buf.as_ptr(); + } } #[stable(feature = "vec_intoiter_as_ref", since = "1.46.0")] From 872ab780c000026441532ced4873bab0c29971df Mon Sep 17 00:00:00 2001 From: The8472 Date: Sun, 26 Jan 2020 20:51:57 +0100 Subject: [PATCH 49/63] work around compiler overhead around lambdas in generics by extracting them into free functions --- library/alloc/src/vec.rs | 73 +++++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 34 deletions(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index 97dd04fbe7050..fa68fc1e2b444 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2212,6 +2212,34 @@ impl SpecFrom> for Vec { } } +fn write_in_place(src_end: *const T) -> impl FnMut(*mut T, T) -> Result<*mut T, !> { + move |mut dst, item| { + unsafe { + // the InPlaceIterable contract cannot be verified precisely here since + // try_fold has an exclusive reference to the source pointer + // all we can do is check if it's still in range + debug_assert!(dst as *const _ <= src_end, "InPlaceIterable contract violation"); + ptr::write(dst, item); + dst = dst.add(1); + } + Ok(dst) + } +} + +fn write_in_place_with_drop( + src_end: *const T, +) -> impl FnMut(InPlaceDrop, T) -> Result, !> { + move |mut sink, item| { + unsafe { + // same caveat as above + debug_assert!(sink.dst as *const _ <= src_end, "InPlaceIterable contract violation"); + ptr::write(sink.dst, item); + sink.dst = sink.dst.add(1); + } + Ok(sink) + } +} + // Further specialization potential once // https://github.com/rust-lang/rust/issues/62645 has been solved: // T can be split into IN and OUT which only need to have the same size and alignment @@ -2230,46 +2258,23 @@ where let inner = unsafe { iterator.as_inner().as_into_iter() }; (inner.buf.as_ptr(), inner.end, inner.cap) }; - let dst = src_buf; + // use try-fold + // - it vectorizes better for some iterator adapters + // - unlike most internal iteration methods methods it only takes a &mut self + // - lets us thread the write pointer through its innards and get it back in the end let dst = if mem::needs_drop::() { - // special-case drop handling since it prevents vectorization - let mut sink = InPlaceDrop { inner: src_buf, dst }; - let _ = iterator.try_for_each::<_, Result<_, !>>(|item| { - unsafe { - debug_assert!( - sink.dst as *const _ <= src_end, - "InPlaceIterable contract violation" - ); - ptr::write(sink.dst, item); - sink.dst = sink.dst.add(1); - } - Ok(()) - }); + // special-case drop handling since it forces us to lug that extra field around which + // can inhibit optimizations + let sink = InPlaceDrop { inner: src_buf, dst: src_buf }; + let sink = iterator + .try_fold::<_, _, Result<_, !>>(sink, write_in_place_with_drop(src_end)) + .unwrap(); // iteration succeeded, don't drop head let sink = mem::ManuallyDrop::new(sink); sink.dst } else { - // use try-fold - // - it vectorizes better - // - unlike most internal iteration methods methods it only takes a &mut self - // - lets us thread the write pointer through its innards and get it back in the end - iterator - .try_fold::<_, _, Result<_, !>>(dst, move |mut dst, item| { - unsafe { - // the InPlaceIterable contract cannot be verified precisely here since - // try_fold has an exclusive reference to the source pointer - // all we can do is check if it's still in range - debug_assert!( - dst as *const _ <= src_end, - "InPlaceIterable contract violation" - ); - ptr::write(dst, item); - dst = dst.add(1); - } - Ok(dst) - }) - .unwrap() + iterator.try_fold::<_, _, Result<_, !>>(src_buf, write_in_place(src_end)).unwrap() }; let src = unsafe { iterator.as_inner().as_into_iter() }; From fa34b39cd6c2fc05f29d8c20518ef45b5317b9b3 Mon Sep 17 00:00:00 2001 From: The8472 Date: Sun, 26 Jan 2020 20:52:59 +0100 Subject: [PATCH 50/63] increase comment verbosity --- library/alloc/src/vec.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index fa68fc1e2b444..5159ddc99b15a 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2164,8 +2164,8 @@ where } } -// A helper struct for in-place iteration that drops the destination slice of iteration. -// The source slice is dropped by IntoIter +// A helper struct for in-place iteration that drops the destination slice of iteration, +// i.e. the head. The source slice (the tail) is dropped by IntoIter. struct InPlaceDrop { inner: *mut T, dst: *mut T, From 5530858a081e1890a7078506fa11f063ade3366b Mon Sep 17 00:00:00 2001 From: The8472 Date: Tue, 28 Jan 2020 15:14:55 +0100 Subject: [PATCH 51/63] generalize in-place collect to types of same size and alignment --- library/alloc/src/collections/binary_heap.rs | 6 ++- library/alloc/src/vec.rs | 46 ++++++++++++-------- library/alloc/tests/vec.rs | 5 ++- 3 files changed, 34 insertions(+), 23 deletions(-) diff --git a/library/alloc/src/collections/binary_heap.rs b/library/alloc/src/collections/binary_heap.rs index 226448b4379a0..40aa4d850f59d 100644 --- a/library/alloc/src/collections/binary_heap.rs +++ b/library/alloc/src/collections/binary_heap.rs @@ -1186,8 +1186,10 @@ unsafe impl SourceIter for IntoIter { #[unstable(issue = "none", feature = "inplace_iteration")] unsafe impl InPlaceIterable for IntoIter {} -impl AsIntoIter for IntoIter { - fn as_into_iter(&mut self) -> &mut vec::IntoIter { +impl AsIntoIter for IntoIter { + type Item = I; + + fn as_into_iter(&mut self) -> &mut vec::IntoIter { &mut self.iter } } diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index 5159ddc99b15a..db01276241906 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2240,23 +2240,28 @@ fn write_in_place_with_drop( } } -// Further specialization potential once -// https://github.com/rust-lang/rust/issues/62645 has been solved: -// T can be split into IN and OUT which only need to have the same size and alignment impl SpecFrom for Vec where - I: Iterator + InPlaceIterable + SourceIter>, + I: Iterator + InPlaceIterable + SourceIter, { default fn from_iter(mut iterator: I) -> Self { - // This specialization only makes sense if we're juggling real allocations. - // Additionally some of the pointer arithmetic would panic on ZSTs. - if mem::size_of::() == 0 { + // Additional requirements which cannot expressed via trait bounds. We rely on const eval + // instead: + // a) no ZSTs as there would be no allocation to reuse and pointer arithmetic would panic + // b) size match as required by Alloc contract + // c) alignments match as required by Alloc contract + if mem::size_of::() == 0 + || mem::size_of::() + != mem::size_of::<<::Source as AsIntoIter>::Item>() + || mem::align_of::() + != mem::align_of::<<::Source as AsIntoIter>::Item>() + { return SpecFromNested::from_iter(iterator); } - let (src_buf, src_end, cap) = { - let inner = unsafe { iterator.as_inner().as_into_iter() }; - (inner.buf.as_ptr(), inner.end, inner.cap) + let (src_buf, dst_buf, dst_end, cap) = unsafe { + let inner = iterator.as_inner().as_into_iter(); + (inner.buf.as_ptr(), inner.buf.as_ptr() as *mut T, inner.end as *const T, inner.cap) }; // use try-fold @@ -2266,15 +2271,15 @@ where let dst = if mem::needs_drop::() { // special-case drop handling since it forces us to lug that extra field around which // can inhibit optimizations - let sink = InPlaceDrop { inner: src_buf, dst: src_buf }; + let sink = InPlaceDrop { inner: dst_buf, dst: dst_buf }; let sink = iterator - .try_fold::<_, _, Result<_, !>>(sink, write_in_place_with_drop(src_end)) + .try_fold::<_, _, Result<_, !>>(sink, write_in_place_with_drop(dst_end)) .unwrap(); // iteration succeeded, don't drop head let sink = mem::ManuallyDrop::new(sink); sink.dst } else { - iterator.try_fold::<_, _, Result<_, !>>(src_buf, write_in_place(src_end)).unwrap() + iterator.try_fold::<_, _, Result<_, !>>(dst_buf, write_in_place(dst_end)).unwrap() }; let src = unsafe { iterator.as_inner().as_into_iter() }; @@ -2289,8 +2294,8 @@ where src.forget_in_place(); let vec = unsafe { - let len = dst.offset_from(src_buf) as usize; - Vec::from_raw_parts(src_buf, len, cap) + let len = dst.offset_from(dst_buf) as usize; + Vec::from_raw_parts(dst_buf, len, cap) }; vec @@ -3010,12 +3015,15 @@ unsafe impl SourceIter for IntoIter { } // internal helper trait for in-place iteration specialization. -pub(crate) trait AsIntoIter { - fn as_into_iter(&mut self) -> &mut IntoIter; +pub(crate) trait AsIntoIter { + type Item; + fn as_into_iter(&mut self) -> &mut IntoIter; } -impl AsIntoIter for IntoIter { - fn as_into_iter(&mut self) -> &mut IntoIter { +impl AsIntoIter for IntoIter { + type Item = T; + + fn as_into_iter(&mut self) -> &mut IntoIter { self } } diff --git a/library/alloc/tests/vec.rs b/library/alloc/tests/vec.rs index 761b76a887615..3464b75118192 100644 --- a/library/alloc/tests/vec.rs +++ b/library/alloc/tests/vec.rs @@ -820,11 +820,12 @@ fn test_from_iter_specialization_with_iterator_adapters() { .zip(std::iter::repeat(1usize)) .map(|(a, b)| a + b) .peekable() - .skip(1); + .skip(1) + .map(|e| std::num::NonZeroUsize::new(e)); assert_in_place_trait(&iter); let sink = iter.collect::>(); let sinkptr = sink.as_ptr(); - assert_eq!(srcptr, sinkptr); + assert_eq!(srcptr, sinkptr as *const usize); } #[test] From 55d1296a55f29d2e59060798a874bfd5b240ca0b Mon Sep 17 00:00:00 2001 From: The8472 Date: Sun, 2 Feb 2020 20:24:40 +0100 Subject: [PATCH 52/63] pacify tidy --- library/core/src/iter/adapters/fuse.rs | 8 ++++---- library/core/src/iter/adapters/mod.rs | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/library/core/src/iter/adapters/fuse.rs b/library/core/src/iter/adapters/fuse.rs index 280f1075be69a..68aa522af3d11 100644 --- a/library/core/src/iter/adapters/fuse.rs +++ b/library/core/src/iter/adapters/fuse.rs @@ -1,10 +1,10 @@ +use super::InPlaceIterable; use crate::intrinsics; use crate::iter::adapters::zip::try_get_unchecked; use crate::iter::TrustedRandomAccess; use crate::iter::{DoubleEndedIterator, ExactSizeIterator, FusedIterator, Iterator}; -use crate::ops::Try; use crate::iter::adapters::SourceIter; -use super::InPlaceIterable; +use crate::ops::Try; /// An iterator that yields `None` forever after the underlying iterator /// yields `None` once. @@ -522,8 +522,8 @@ where #[unstable(issue = "none", feature = "inplace_iteration")] unsafe impl SourceIter for Fuse - where - I: SourceIter, +where + I: SourceIter, { type Source = S; diff --git a/library/core/src/iter/adapters/mod.rs b/library/core/src/iter/adapters/mod.rs index ed5600f1824a5..6013a2f111f88 100644 --- a/library/core/src/iter/adapters/mod.rs +++ b/library/core/src/iter/adapters/mod.rs @@ -2095,9 +2095,9 @@ where #[unstable(issue = "none", feature = "inplace_iteration")] unsafe impl SourceIter for TakeWhile - where - P: FnMut(&I::Item) -> bool, - I: SourceIter, +where + P: FnMut(&I::Item) -> bool, + I: SourceIter, { type Source = S; From 80638330f29264591ffea866c807851093abb2fd Mon Sep 17 00:00:00 2001 From: The8472 Date: Sat, 4 Apr 2020 18:34:18 +0200 Subject: [PATCH 53/63] support in-place collect for MapWhile adapters --- library/alloc/tests/lib.rs | 1 + library/alloc/tests/vec.rs | 1 + library/core/src/iter/adapters/mod.rs | 21 +++++++++++++++++++++ 3 files changed, 23 insertions(+) diff --git a/library/alloc/tests/lib.rs b/library/alloc/tests/lib.rs index e0e146dc42754..b1513d1b05655 100644 --- a/library/alloc/tests/lib.rs +++ b/library/alloc/tests/lib.rs @@ -15,6 +15,7 @@ #![feature(split_inclusive)] #![feature(binary_heap_retain)] #![feature(inplace_iteration)] +#![feature(iter_map_while)] use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher}; diff --git a/library/alloc/tests/vec.rs b/library/alloc/tests/vec.rs index 3464b75118192..47819714cc4f6 100644 --- a/library/alloc/tests/vec.rs +++ b/library/alloc/tests/vec.rs @@ -819,6 +819,7 @@ fn test_from_iter_specialization_with_iterator_adapters() { .map(|i| i.0 + i.1) .zip(std::iter::repeat(1usize)) .map(|(a, b)| a + b) + .map_while(Option::Some) .peekable() .skip(1) .map(|e| std::num::NonZeroUsize::new(e)); diff --git a/library/core/src/iter/adapters/mod.rs b/library/core/src/iter/adapters/mod.rs index 6013a2f111f88..93e0ad3c8ead1 100644 --- a/library/core/src/iter/adapters/mod.rs +++ b/library/core/src/iter/adapters/mod.rs @@ -2191,6 +2191,27 @@ where } } +#[unstable(issue = "none", feature = "inplace_iteration")] +unsafe impl SourceIter for MapWhile +where + P: FnMut(I::Item) -> Option, + I: SourceIter, +{ + type Source = S; + + #[inline] + unsafe fn as_inner(&mut self) -> &mut S { + // Safety: unsafe function forwarding to unsafe function with the same requirements + unsafe { SourceIter::as_inner(&mut self.iter) } + } +} + +#[unstable(issue = "none", feature = "inplace_iteration")] +unsafe impl InPlaceIterable for MapWhile where + P: FnMut(I::Item) -> Option +{ +} + /// An iterator that skips over `n` elements of `iter`. /// /// This `struct` is created by the [`skip`] method on [`Iterator`]. See its From bec9f9223cefb50cdb4a9b8a7d056545fa3641c2 Mon Sep 17 00:00:00 2001 From: The8472 Date: Tue, 19 May 2020 00:44:16 +0200 Subject: [PATCH 54/63] apply required min_specialization attributes --- library/alloc/src/vec.rs | 8 +++++++- library/core/src/iter/traits/marker.rs | 1 + 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index db01276241906..c0c63a27be51a 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2240,9 +2240,14 @@ fn write_in_place_with_drop( } } +#[rustc_unsafe_specialization_marker] +trait SourceIterMarker: SourceIter {} + +impl SourceIterMarker for T where T: SourceIter {} + impl SpecFrom for Vec where - I: Iterator + InPlaceIterable + SourceIter, + I: Iterator + InPlaceIterable + SourceIterMarker, { default fn from_iter(mut iterator: I) -> Self { // Additional requirements which cannot expressed via trait bounds. We rely on const eval @@ -3015,6 +3020,7 @@ unsafe impl SourceIter for IntoIter { } // internal helper trait for in-place iteration specialization. +#[rustc_specialization_trait] pub(crate) trait AsIntoIter { type Item; fn as_into_iter(&mut self) -> &mut IntoIter; diff --git a/library/core/src/iter/traits/marker.rs b/library/core/src/iter/traits/marker.rs index f287196da03ef..5d31e78dd859d 100644 --- a/library/core/src/iter/traits/marker.rs +++ b/library/core/src/iter/traits/marker.rs @@ -53,4 +53,5 @@ unsafe impl TrustedLen for &mut I {} /// /// [`SourceIter`]: ../../std/iter/trait.SourceIter.html #[unstable(issue = "none", feature = "inplace_iteration")] +#[rustc_specialization_trait] pub unsafe trait InPlaceIterable: Iterator {} From a62cd1b44cdf34313cd0afc4e66410444bab6669 Mon Sep 17 00:00:00 2001 From: The8472 Date: Thu, 21 May 2020 01:34:05 +0200 Subject: [PATCH 55/63] fix benchmark compile errors --- library/alloc/benches/vec.rs | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/library/alloc/benches/vec.rs b/library/alloc/benches/vec.rs index 1019503a8417a..5ba3e0e00572c 100644 --- a/library/alloc/benches/vec.rs +++ b/library/alloc/benches/vec.rs @@ -244,7 +244,7 @@ fn bench_extend_recycle(b: &mut Bencher) { let tmp = std::mem::replace(&mut data, Vec::new()); let mut to_extend = black_box(Vec::new()); to_extend.extend(tmp.into_iter()); - std::mem::replace(&mut data, black_box(to_extend)); + data = black_box(to_extend); }); black_box(data); @@ -502,16 +502,13 @@ fn bench_in_place_recycle(b: &mut test::Bencher) { b.iter(|| { let tmp = std::mem::replace(&mut data, Vec::new()); - std::mem::replace( - &mut data, - black_box( - tmp.into_iter() - .enumerate() - .map(|(idx, e)| idx.wrapping_add(e)) - .fuse() - .peekable() - .collect::>(), - ), + data = black_box( + tmp.into_iter() + .enumerate() + .map(|(idx, e)| idx.wrapping_add(e)) + .fuse() + .peekable() + .collect::>(), ); }); } @@ -532,7 +529,7 @@ fn bench_in_place_zip_recycle(b: &mut test::Bencher) { .map(|(i, (d, s))| d.wrapping_add(i as u8) ^ s) .collect::>(); assert_eq!(mangled.len(), 1000); - std::mem::replace(&mut data, black_box(mangled)); + data = black_box(mangled); }); } From 7badb7a7f8e66f183106ae2eac2062ea6aa63f2f Mon Sep 17 00:00:00 2001 From: The8472 Date: Sun, 24 May 2020 03:23:15 +0200 Subject: [PATCH 56/63] avoid applying in-place collect specialization in type-length test the test was sized to match external iteration only, since vec's in-place collect now uses internal iteration we collect into a different type now. Note that any other try-fold based operation such as count() would also have exceeded the type length limit for that iterator. --- src/test/ui/iterators/issue-58952-filter-type-length.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/ui/iterators/issue-58952-filter-type-length.rs b/src/test/ui/iterators/issue-58952-filter-type-length.rs index 046e37840849e..ffbe89a14e3bb 100644 --- a/src/test/ui/iterators/issue-58952-filter-type-length.rs +++ b/src/test/ui/iterators/issue-58952-filter-type-length.rs @@ -3,6 +3,7 @@ //! so check that we don't accidentially exceed the type length limit. // FIXME: Once the size of iterator adaptors is further reduced, // increase the complexity of this test. +use std::collections::VecDeque; fn main() { let c = 2; @@ -27,5 +28,5 @@ fn main() { .filter(|a| b.clone().any(|b| *b == *a)) .filter(|a| b.clone().any(|b| *b == *a)) .filter(|a| b.clone().any(|b| *b == *a)) - .collect::>(); + .collect::>(); } From 9aeea0022225ccc11aae031ebf06dc2b125a29f1 Mon Sep 17 00:00:00 2001 From: The8472 Date: Fri, 21 Aug 2020 23:53:13 +0200 Subject: [PATCH 57/63] get things to work under min_specialization by leaning more heavily on #[rustc_unsafe_specialization_marker] --- library/alloc/src/lib.rs | 1 + library/alloc/src/vec.rs | 21 +++++++++++++-------- library/core/src/iter/traits/marker.rs | 1 - 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/library/alloc/src/lib.rs b/library/alloc/src/lib.rs index 5cbd9d380b0d9..43b70a5163690 100644 --- a/library/alloc/src/lib.rs +++ b/library/alloc/src/lib.rs @@ -135,6 +135,7 @@ #![feature(slice_partition_dedup)] #![feature(maybe_uninit_extra, maybe_uninit_slice)] #![feature(alloc_layout_extra)] +#![feature(trusted_random_access)] #![feature(try_trait)] #![feature(type_alias_impl_trait)] #![feature(associated_type_bounds)] diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index c0c63a27be51a..dcfd3ecb3d369 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2243,11 +2243,11 @@ fn write_in_place_with_drop( #[rustc_unsafe_specialization_marker] trait SourceIterMarker: SourceIter {} -impl SourceIterMarker for T where T: SourceIter {} +impl SourceIterMarker for T where T: SourceIter + InPlaceIterable {} impl SpecFrom for Vec where - I: Iterator + InPlaceIterable + SourceIterMarker, + I: Iterator + SourceIterMarker, { default fn from_iter(mut iterator: I) -> Self { // Additional requirements which cannot expressed via trait bounds. We rely on const eval @@ -2920,6 +2920,17 @@ impl Iterator for IntoIter { fn count(self) -> usize { self.len() } + + unsafe fn get_unchecked(&mut self, i: usize) -> Self::Item + where + Self: TrustedRandomAccess, + { + // SAFETY: the caller must uphold the contract for + // `Iterator::get_unchecked`. + unsafe { + if mem::size_of::() == 0 { mem::zeroed() } else { ptr::read(self.ptr.add(i)) } + } + } } #[stable(feature = "rust1", since = "1.0.0")] @@ -2967,12 +2978,6 @@ unsafe impl TrustedRandomAccess for IntoIter where T: Copy, { - unsafe fn get_unchecked(&mut self, i: usize) -> Self::Item { - unsafe { - if mem::size_of::() == 0 { mem::zeroed() } else { ptr::read(self.ptr.add(i)) } - } - } - fn may_have_side_effect() -> bool { false } diff --git a/library/core/src/iter/traits/marker.rs b/library/core/src/iter/traits/marker.rs index 5d31e78dd859d..f287196da03ef 100644 --- a/library/core/src/iter/traits/marker.rs +++ b/library/core/src/iter/traits/marker.rs @@ -53,5 +53,4 @@ unsafe impl TrustedLen for &mut I {} /// /// [`SourceIter`]: ../../std/iter/trait.SourceIter.html #[unstable(issue = "none", feature = "inplace_iteration")] -#[rustc_specialization_trait] pub unsafe trait InPlaceIterable: Iterator {} From 7492f76f777d2815290d3b8590dc3911df6336de Mon Sep 17 00:00:00 2001 From: The8472 Date: Fri, 21 Aug 2020 23:56:23 +0200 Subject: [PATCH 58/63] please tidy --- library/core/src/iter/adapters/fuse.rs | 2 +- library/core/src/iter/adapters/mod.rs | 1 - library/core/src/iter/adapters/zip.rs | 8 ++++---- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/library/core/src/iter/adapters/fuse.rs b/library/core/src/iter/adapters/fuse.rs index 68aa522af3d11..4185453ac5ae3 100644 --- a/library/core/src/iter/adapters/fuse.rs +++ b/library/core/src/iter/adapters/fuse.rs @@ -1,9 +1,9 @@ use super::InPlaceIterable; use crate::intrinsics; use crate::iter::adapters::zip::try_get_unchecked; +use crate::iter::adapters::SourceIter; use crate::iter::TrustedRandomAccess; use crate::iter::{DoubleEndedIterator, ExactSizeIterator, FusedIterator, Iterator}; -use crate::iter::adapters::SourceIter; use crate::ops::Try; /// An iterator that yields `None` forever after the underlying iterator diff --git a/library/core/src/iter/adapters/mod.rs b/library/core/src/iter/adapters/mod.rs index 93e0ad3c8ead1..c3f1269401aa9 100644 --- a/library/core/src/iter/adapters/mod.rs +++ b/library/core/src/iter/adapters/mod.rs @@ -485,7 +485,6 @@ where unsafe impl TrustedRandomAccess for Cloned where I: TrustedRandomAccess, - { #[inline] fn may_have_side_effect() -> bool { diff --git a/library/core/src/iter/adapters/zip.rs b/library/core/src/iter/adapters/zip.rs index 0597d95018505..c1c90ec9a836a 100644 --- a/library/core/src/iter/adapters/zip.rs +++ b/library/core/src/iter/adapters/zip.rs @@ -334,10 +334,10 @@ where // it would require negative trait bounds to be able to try both #[unstable(issue = "none", feature = "inplace_iteration")] unsafe impl SourceIter for Zip - where - A: SourceIter, - B: Iterator, - S: Iterator, +where + A: SourceIter, + B: Iterator, + S: Iterator, { type Source = S; From 435219dd82896e9576ef3f7fbc5c2cf17bae0016 Mon Sep 17 00:00:00 2001 From: The8472 Date: Wed, 26 Aug 2020 23:24:34 +0200 Subject: [PATCH 59/63] remove empty Vec extend optimization The optimization meant that every extend code path had to emit llvm IR for from_iter and extend spec_extend, which likely impacts compile times while only improving a few edge-cases --- library/alloc/src/vec.rs | 16 ++-------------- library/alloc/tests/vec.rs | 10 ---------- 2 files changed, 2 insertions(+), 24 deletions(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index dcfd3ecb3d369..2a778e1470ded 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2082,13 +2082,7 @@ impl<'a, T> IntoIterator for &'a mut Vec { impl Extend for Vec { #[inline] fn extend>(&mut self, iter: I) { - if self.capacity() > 0 { - >::spec_extend(self, iter.into_iter()) - } else { - // if self has no allocation then use the more powerful from_iter specializations - // and overwrite self - *self = SpecFrom::from_iter(iter.into_iter()); - } + >::spec_extend(self, iter.into_iter()) } #[inline] @@ -2544,13 +2538,7 @@ impl Vec { #[stable(feature = "extend_ref", since = "1.2.0")] impl<'a, T: 'a + Copy> Extend<&'a T> for Vec { fn extend>(&mut self, iter: I) { - if self.capacity() > 0 { - self.spec_extend(iter.into_iter()) - } else { - // if self has no allocation then use the more powerful from_iter specializations - // and overwrite self - *self = SpecFrom::from_iter(iter.into_iter()); - } + self.spec_extend(iter.into_iter()) } #[inline] diff --git a/library/alloc/tests/vec.rs b/library/alloc/tests/vec.rs index 47819714cc4f6..8e66c8a22cec5 100644 --- a/library/alloc/tests/vec.rs +++ b/library/alloc/tests/vec.rs @@ -798,16 +798,6 @@ fn test_from_iter_partially_drained_in_place_specialization() { assert_eq!(srcptr, sinkptr); } -#[test] -fn test_extend_in_place_specialization() { - let src: Vec = vec![0usize; 1]; - let srcptr = src.as_ptr(); - let mut dst = Vec::new(); - dst.extend(src.into_iter()); - let dstptr = dst.as_ptr(); - assert_eq!(srcptr, dstptr); -} - #[test] fn test_from_iter_specialization_with_iterator_adapters() { fn assert_in_place_trait(_: &T) {}; From acdd441cc3cc03e7ed2e6e4b26db56b4cb5db7f3 Mon Sep 17 00:00:00 2001 From: The8472 Date: Wed, 26 Aug 2020 23:32:09 +0200 Subject: [PATCH 60/63] remove separate no-drop code path since it resulted in more LLVM IR --- library/alloc/src/vec.rs | 47 +++++++++++++--------------------------- 1 file changed, 15 insertions(+), 32 deletions(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index 2a778e1470ded..ecc103fc98c0f 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2174,8 +2174,10 @@ impl InPlaceDrop { impl Drop for InPlaceDrop { #[inline] fn drop(&mut self) { - unsafe { - ptr::drop_in_place(slice::from_raw_parts_mut(self.inner, self.len())); + if mem::needs_drop::() { + unsafe { + ptr::drop_in_place(slice::from_raw_parts_mut(self.inner, self.len())); + } } } } @@ -2206,26 +2208,14 @@ impl SpecFrom> for Vec { } } -fn write_in_place(src_end: *const T) -> impl FnMut(*mut T, T) -> Result<*mut T, !> { - move |mut dst, item| { - unsafe { - // the InPlaceIterable contract cannot be verified precisely here since - // try_fold has an exclusive reference to the source pointer - // all we can do is check if it's still in range - debug_assert!(dst as *const _ <= src_end, "InPlaceIterable contract violation"); - ptr::write(dst, item); - dst = dst.add(1); - } - Ok(dst) - } -} - fn write_in_place_with_drop( src_end: *const T, ) -> impl FnMut(InPlaceDrop, T) -> Result, !> { move |mut sink, item| { unsafe { - // same caveat as above + // the InPlaceIterable contract cannot be verified precisely here since + // try_fold has an exclusive reference to the source pointer + // all we can do is check if it's still in range debug_assert!(sink.dst as *const _ <= src_end, "InPlaceIterable contract violation"); ptr::write(sink.dst, item); sink.dst = sink.dst.add(1); @@ -2263,23 +2253,16 @@ where (inner.buf.as_ptr(), inner.buf.as_ptr() as *mut T, inner.end as *const T, inner.cap) }; - // use try-fold + // use try-fold since // - it vectorizes better for some iterator adapters // - unlike most internal iteration methods methods it only takes a &mut self - // - lets us thread the write pointer through its innards and get it back in the end - let dst = if mem::needs_drop::() { - // special-case drop handling since it forces us to lug that extra field around which - // can inhibit optimizations - let sink = InPlaceDrop { inner: dst_buf, dst: dst_buf }; - let sink = iterator - .try_fold::<_, _, Result<_, !>>(sink, write_in_place_with_drop(dst_end)) - .unwrap(); - // iteration succeeded, don't drop head - let sink = mem::ManuallyDrop::new(sink); - sink.dst - } else { - iterator.try_fold::<_, _, Result<_, !>>(dst_buf, write_in_place(dst_end)).unwrap() - }; + // - it lets us thread the write pointer through its innards and get it back in the end + let sink = InPlaceDrop { inner: dst_buf, dst: dst_buf }; + let sink = iterator + .try_fold::<_, _, Result<_, !>>(sink, write_in_place_with_drop(dst_end)) + .unwrap(); + // iteration succeeded, don't drop head + let dst = mem::ManuallyDrop::new(sink).dst; let src = unsafe { iterator.as_inner().as_into_iter() }; // check if SourceIter and InPlaceIterable contracts were upheld. From 64645865423990935a74abecc576a91f5b89f2cd Mon Sep 17 00:00:00 2001 From: The8472 Date: Sun, 30 Aug 2020 02:15:45 +0200 Subject: [PATCH 61/63] add explanation to specialization marker --- library/alloc/src/vec.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index ecc103fc98c0f..2e3f6ec60e5f1 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2227,6 +2227,12 @@ fn write_in_place_with_drop( #[rustc_unsafe_specialization_marker] trait SourceIterMarker: SourceIter {} +// The std-internal SourceIter/InPlaceIterable traits are only implemented by chains of +// Adapter>> (all owned by core/std). Additional bounds +// on the adapter implementations (beyond `impl Trait for Adapter`) only depend on other +// traits already marked as specialization traits (Copy, TrustedRandomAccess, FusedIterator). +// I.e. the marker does not depend on lifetimes of user-supplied types. Modulo the Copy hole, which +// several other specializations already depend on. impl SourceIterMarker for T where T: SourceIter + InPlaceIterable {} impl SpecFrom for Vec From 8e5fe5569b5ffd59f57e0dab4380ba04b063330e Mon Sep 17 00:00:00 2001 From: The8472 Date: Wed, 2 Sep 2020 21:40:19 +0200 Subject: [PATCH 62/63] improve comments and naming --- library/alloc/src/vec.rs | 81 +++++++++++++++++++++++++++------------- 1 file changed, 56 insertions(+), 25 deletions(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index 2e3f6ec60e5f1..5c474ab2cde57 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2014,7 +2014,7 @@ impl> IndexMut for Vec { impl FromIterator for Vec { #[inline] fn from_iter>(iter: I) -> Vec { - >::from_iter(iter.into_iter()) + >::from_iter(iter.into_iter()) } } @@ -2096,18 +2096,39 @@ impl Extend for Vec { } } -// Specialization trait used for Vec::from_iter -trait SpecFrom { +/// Specialization trait used for Vec::from_iter +/// +/// ## The delegation graph: +/// +/// ```text +/// +-------------+ +/// |FromIterator | +/// +-+-----------+ +/// | +/// v +/// +-+-------------------------------+ +---------------------+ +/// |SpecFromIter +---->+SpecFromIterNested | +/// |where I: | | |where I: | +/// | Iterator (default)----------+ | | Iterator (default) | +/// | vec::IntoIter | | | TrustedLen | +/// | SourceIterMarker---fallback-+ | | | +/// | slice::Iter | | | +/// | Iterator | +---------------------+ +/// +---------------------------------+ +/// +/// ``` +trait SpecFromIter { fn from_iter(iter: I) -> Self; } -// Another specialization trait for Vec::from_iter -// necessary to manually prioritize overlapping specializations -trait SpecFromNested { +/// Another specialization trait for Vec::from_iter +/// necessary to manually prioritize overlapping specializations +/// see [`SpecFromIter`] for details. +trait SpecFromIterNested { fn from_iter(iter: I) -> Self; } -impl SpecFromNested for Vec +impl SpecFromIterNested for Vec where I: Iterator, { @@ -2136,7 +2157,7 @@ where } } -impl SpecFromNested for Vec +impl SpecFromIterNested for Vec where I: TrustedLen, { @@ -2149,12 +2170,12 @@ where } } -impl SpecFrom for Vec +impl SpecFromIter for Vec where I: Iterator, { default fn from_iter(iterator: I) -> Self { - SpecFromNested::from_iter(iterator) + SpecFromIterNested::from_iter(iterator) } } @@ -2182,18 +2203,21 @@ impl Drop for InPlaceDrop { } } -impl SpecFrom> for Vec { +impl SpecFromIter> for Vec { fn from_iter(iterator: IntoIter) -> Self { // A common case is passing a vector into a function which immediately // re-collects into a vector. We can short circuit this if the IntoIter // has not been advanced at all. - // We can also reuse the memory and move the data to the front if - // allocating a new vector and moving to it would result in the same capacity - let non_zero_offset = iterator.buf.as_ptr() as *const _ != iterator.ptr; - if !non_zero_offset || iterator.len() >= iterator.cap / 2 { + // When it has been advanced We can also reuse the memory and move the data to the front. + // But we only do so when the resulting Vec wouldn't have more unused capacity + // than creating it through the generic FromIterator implementation would. That limitation + // is not strictly necessary as Vec's allocation behavior is intentionally unspecified. + // But it is a conservative choice. + let has_advanced = iterator.buf.as_ptr() as *const _ != iterator.ptr; + if !has_advanced || iterator.len() >= iterator.cap / 2 { unsafe { let it = ManuallyDrop::new(iterator); - if non_zero_offset { + if has_advanced { ptr::copy(it.ptr, it.buf.as_ptr(), it.len()); } return Vec::from_raw_parts(it.buf.as_ptr(), it.len(), it.cap); @@ -2224,6 +2248,12 @@ fn write_in_place_with_drop( } } +/// Specialization marker for collecting an iterator pipeline into a Vec while reusing the +/// source allocation, i.e. executing the pipeline in place. +/// +/// The SourceIter parent trait is necessary for the specializing function to access the allocation +/// which is to be reused. But it is not sufficient for the specialization to be valid. See +/// additional bounds on the impl. #[rustc_unsafe_specialization_marker] trait SourceIterMarker: SourceIter {} @@ -2235,7 +2265,7 @@ trait SourceIterMarker: SourceIter {} // several other specializations already depend on. impl SourceIterMarker for T where T: SourceIter + InPlaceIterable {} -impl SpecFrom for Vec +impl SpecFromIter for Vec where I: Iterator + SourceIterMarker, { @@ -2251,7 +2281,8 @@ where || mem::align_of::() != mem::align_of::<<::Source as AsIntoIter>::Item>() { - return SpecFromNested::from_iter(iterator); + // fallback to more generic implementations + return SpecFromIterNested::from_iter(iterator); } let (src_buf, dst_buf, dst_end, cap) = unsafe { @@ -2277,9 +2308,9 @@ where debug_assert!(dst as *const _ <= src.ptr, "InPlaceIterable contract violation"); // drop any remaining values at the tail of the source - src.drop_in_place(); + src.drop_remaining(); // but prevent drop of the allocation itself once IntoIter goes out of scope - src.forget_in_place(); + src.forget_allocation(); let vec = unsafe { let len = dst.offset_from(dst_buf) as usize; @@ -2290,17 +2321,17 @@ where } } -impl<'a, T: 'a, I> SpecFrom<&'a T, I> for Vec +impl<'a, T: 'a, I> SpecFromIter<&'a T, I> for Vec where I: Iterator, T: Clone, { default fn from_iter(iterator: I) -> Self { - SpecFrom::from_iter(iterator.cloned()) + SpecFromIter::from_iter(iterator.cloned()) } } -impl<'a, T: 'a> SpecFrom<&'a T, slice::Iter<'a, T>> for Vec +impl<'a, T: 'a> SpecFromIter<&'a T, slice::Iter<'a, T>> for Vec where T: Copy, { @@ -2824,7 +2855,7 @@ impl IntoIter { ptr::slice_from_raw_parts_mut(self.ptr as *mut T, self.len()) } - fn drop_in_place(&mut self) { + fn drop_remaining(&mut self) { if mem::needs_drop::() { unsafe { ptr::drop_in_place(self.as_mut_slice()); @@ -2835,7 +2866,7 @@ impl IntoIter { /// Relinquishes the backing allocation, equivalent to /// `ptr::write(&mut self, Vec::new().into_iter())` - fn forget_in_place(&mut self) { + fn forget_allocation(&mut self) { self.cap = 0; self.buf = unsafe { NonNull::new_unchecked(RawVec::NEW.ptr()) }; self.ptr = self.buf.as_ptr(); From 2f23a0fccad2f4d85d2ec923f0ac40b4b42211a8 Mon Sep 17 00:00:00 2001 From: The8472 Date: Thu, 3 Sep 2020 22:15:47 +0200 Subject: [PATCH 63/63] fix debug assertion The InPlaceIterable debug assert checks that the write pointer did not advance beyond the read pointer. But TrustedRandomAccess never advances the read pointer, thus triggering the assert. Skip the assert if the source pointer did not change during iteration. --- library/alloc/src/vec.rs | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index 5c474ab2cde57..9013e3fc16ab9 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -2285,9 +2285,15 @@ where return SpecFromIterNested::from_iter(iterator); } - let (src_buf, dst_buf, dst_end, cap) = unsafe { + let (src_buf, src_ptr, dst_buf, dst_end, cap) = unsafe { let inner = iterator.as_inner().as_into_iter(); - (inner.buf.as_ptr(), inner.buf.as_ptr() as *mut T, inner.end as *const T, inner.cap) + ( + inner.buf.as_ptr(), + inner.ptr, + inner.buf.as_ptr() as *mut T, + inner.end as *const T, + inner.cap, + ) }; // use try-fold since @@ -2302,10 +2308,18 @@ where let dst = mem::ManuallyDrop::new(sink).dst; let src = unsafe { iterator.as_inner().as_into_iter() }; - // check if SourceIter and InPlaceIterable contracts were upheld. + // check if SourceIter contract was upheld // caveat: if they weren't we may not even make it to this point debug_assert_eq!(src_buf, src.buf.as_ptr()); - debug_assert!(dst as *const _ <= src.ptr, "InPlaceIterable contract violation"); + // check InPlaceIterable contract. This is only possible if the iterator advanced the + // source pointer at all. If it uses unchecked access via TrustedRandomAccess + // then the source pointer will stay in its initial position and we can't use it as reference + if src.ptr != src_ptr { + debug_assert!( + dst as *const _ <= src.ptr, + "InPlaceIterable contract violation, write pointer advanced beyond read pointer" + ); + } // drop any remaining values at the tail of the source src.drop_remaining();