Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expand in-place iteration specialization to Flatten, FlatMap and ArrayChunks #110353

Merged
merged 6 commits into from
Nov 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions library/alloc/src/collections/binary_heap/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@

use core::alloc::Allocator;
use core::fmt;
use core::iter::{FusedIterator, InPlaceIterable, SourceIter, TrustedLen};
use core::iter::{FusedIterator, InPlaceIterable, SourceIter, TrustedFused, TrustedLen};
use core::mem::{self, swap, ManuallyDrop};
use core::num::NonZeroUsize;
use core::ops::{Deref, DerefMut};
Expand Down Expand Up @@ -1540,6 +1540,10 @@ impl<T, A: Allocator> ExactSizeIterator for IntoIter<T, A> {
#[stable(feature = "fused", since = "1.26.0")]
impl<T, A: Allocator> FusedIterator for IntoIter<T, A> {}

#[doc(hidden)]
#[unstable(issue = "none", feature = "trusted_fused")]
unsafe impl<T, A: Allocator> TrustedFused for IntoIter<T, A> {}

#[stable(feature = "default_iters", since = "1.70.0")]
impl<T> Default for IntoIter<T> {
/// Creates an empty `binary_heap::IntoIter`.
Expand Down Expand Up @@ -1569,7 +1573,10 @@ unsafe impl<T, A: Allocator> SourceIter for IntoIter<T, A> {

#[unstable(issue = "none", feature = "inplace_iteration")]
#[doc(hidden)]
unsafe impl<I, A: Allocator> InPlaceIterable for IntoIter<I, A> {}
unsafe impl<I, A: Allocator> InPlaceIterable for IntoIter<I, A> {
const EXPAND_BY: Option<NonZeroUsize> = NonZeroUsize::new(1);
const MERGE_BY: Option<NonZeroUsize> = NonZeroUsize::new(1);
}

unsafe impl<I> AsVecIntoIter for IntoIter<I> {
type Item = I;
Expand Down
1 change: 1 addition & 0 deletions library/alloc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@
#![feature(std_internals)]
#![feature(str_internals)]
#![feature(strict_provenance)]
#![feature(trusted_fused)]
#![feature(trusted_len)]
#![feature(trusted_random_access)]
#![feature(try_trait_v2)]
Expand Down
133 changes: 108 additions & 25 deletions library/alloc/src/vec/in_place_collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
//! The specialization in this module applies to iterators in the shape of
//! `source.adapter().adapter().adapter().collect::<Vec<U>>()`
//! where `source` is an owning iterator obtained from [`Vec<T>`], [`Box<[T]>`][box] (by conversion to `Vec`)
//! or [`BinaryHeap<T>`], the adapters each consume one or more items per step
//! (represented by [`InPlaceIterable`]), provide transitive access to `source` (via [`SourceIter`])
//! and thus the underlying allocation. And finally the layouts of `T` and `U` must
//! have the same size and alignment, this is currently ensured via const eval instead of trait bounds
//! in the specialized [`SpecFromIter`] implementation.
//! or [`BinaryHeap<T>`], the adapters guarantee to consume enough items per step to make room
//! for the results (represented by [`InPlaceIterable`]), provide transitive access to `source`
//! (via [`SourceIter`]) and thus the underlying allocation.
//! And finally there are alignment and size constriants to consider, this is currently ensured via
//! const eval instead of trait bounds in the specialized [`SpecFromIter`] implementation.
//!
//! [`BinaryHeap<T>`]: crate::collections::BinaryHeap
//! [box]: crate::boxed::Box
Expand All @@ -35,11 +35,28 @@
//! the step of reading a value and getting a reference to write to. Instead raw pointers must be
//! used on the reader and writer side.
//!
//! That writes never clobber a yet-to-be-read item is ensured by the [`InPlaceIterable`] requirements.
//! That writes never clobber a yet-to-be-read items is ensured by the [`InPlaceIterable`] requirements.
//!
//! # Layout constraints
//!
//! [`Allocator`] requires that `allocate()` and `deallocate()` have matching alignment and size.
//! When recycling an allocation between different types we must uphold the [`Allocator`] contract
//! which means that the input and output Layouts have to "fit".
//!
//! To complicate things further `InPlaceIterable` supports splitting or merging items into smaller/
//! larger ones to enable (de)aggregation of arrays.
//!
//! Ultimately each step of the iterator must free up enough *bytes* in the source to make room
//! for the next output item.
//! If `T` and `U` have the same size no fixup is needed.
//! If `T`'s size is a multiple of `U`'s we can compensate by multiplying the capacity accordingly.
//! Otherwise the input capacity (and thus layout) in bytes may not be representable by the output
//! `Vec<U>`. In that case `alloc.shrink()` is used to update the allocation's layout.
//!
//! Alignments of `T` must be the same or larger than `U`. Since alignments are always a power
//! of two _larger_ implies _is a multiple of_.
//!
//! See `in_place_collectible()` for the current conditions.
//!
//! Additionally this specialization doesn't make sense for ZSTs as there is no reallocation to
//! avoid and it would make pointer arithmetic more difficult.
//!
Expand Down Expand Up @@ -137,44 +154,73 @@
//! }
//! vec.truncate(write_idx);
//! ```
use crate::alloc::{handle_alloc_error, Global};
use core::alloc::Allocator;
use core::alloc::Layout;
use core::iter::{InPlaceIterable, SourceIter, TrustedRandomAccessNoCoerce};
use core::mem::{self, ManuallyDrop, SizedTypeProperties};
use core::ptr::{self};
use core::num::NonZeroUsize;
use core::ptr::{self, NonNull};

use super::{InPlaceDrop, InPlaceDstBufDrop, SpecFromIter, SpecFromIterNested, Vec};

/// Specialization marker for collecting an iterator pipeline into a Vec while reusing the
/// source allocation, i.e. executing the pipeline in place.
#[rustc_unsafe_specialization_marker]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While the specialization-thicket is growing more complex and I encountered a bunch of ICEs along the way... relying on one fewer of these hopefully makes things a bit less precarious.

pub(super) trait InPlaceIterableMarker {}
const fn in_place_collectible<DEST, SRC>(
step_merge: Option<NonZeroUsize>,
step_expand: Option<NonZeroUsize>,
) -> bool {
if DEST::IS_ZST || mem::align_of::<SRC>() < mem::align_of::<DEST>() {
return false;
}

match (step_merge, step_expand) {
(Some(step_merge), Some(step_expand)) => {
// At least N merged source items -> at most M expanded destination items
// e.g.
// - 1 x [u8; 4] -> 4x u8, via flatten
// - 4 x u8 -> 1x [u8; 4], via array_chunks
mem::size_of::<SRC>() * step_merge.get() >= mem::size_of::<DEST>() * step_expand.get()
}
// Fall back to other from_iter impls if an overflow occurred in the step merge/expansion
// tracking.
_ => false,
}
}

impl<T> InPlaceIterableMarker for T where T: InPlaceIterable {}
/// This provides a shorthand for the source type since local type aliases aren't a thing.
#[rustc_specialization_trait]
trait InPlaceCollect: SourceIter<Source: AsVecIntoIter> + InPlaceIterable {
type Src;
}

impl<T> InPlaceCollect for T
where
T: SourceIter<Source: AsVecIntoIter> + InPlaceIterable,
{
type Src = <<T as SourceIter>::Source as AsVecIntoIter>::Item;
}

impl<T, I> SpecFromIter<T, I> for Vec<T>
where
I: Iterator<Item = T> + SourceIter<Source: AsVecIntoIter> + InPlaceIterableMarker,
I: Iterator<Item = T> + InPlaceCollect,
<I as SourceIter>::Source: AsVecIntoIter,
{
default fn from_iter(mut iterator: I) -> Self {
// See "Layout constraints" section in the module documentation. We rely on const
// optimization here since these conditions currently cannot be expressed as trait bounds
if T::IS_ZST
|| mem::size_of::<T>()
!= mem::size_of::<<<I as SourceIter>::Source as AsVecIntoIter>::Item>()
|| mem::align_of::<T>()
!= mem::align_of::<<<I as SourceIter>::Source as AsVecIntoIter>::Item>()
{
if const { !in_place_collectible::<T, I::Src>(I::MERGE_BY, I::EXPAND_BY) } {
// fallback to more generic implementations
return SpecFromIterNested::from_iter(iterator);
}

let (src_buf, src_ptr, dst_buf, dst_end, cap) = unsafe {
let (src_buf, src_ptr, src_cap, mut dst_buf, dst_end, dst_cap) = unsafe {
let inner = iterator.as_inner().as_into_iter();
(
inner.buf.as_ptr(),
inner.ptr,
inner.cap,
inner.buf.as_ptr() as *mut T,
inner.end as *const T,
inner.cap,
inner.cap * mem::size_of::<I::Src>() / mem::size_of::<T>(),
)
};

Expand All @@ -196,18 +242,55 @@ where
}

// The ownership of the allocation and the new `T` values is temporarily moved into `dst_guard`.
// This is safe because `forget_allocation_drop_remaining` immediately forgets the allocation
// This is safe because
// * `forget_allocation_drop_remaining` immediately forgets the allocation
// before any panic can occur in order to avoid any double free, and then proceeds to drop
// any remaining values at the tail of the source.
// * the shrink either panics without invalidating the allocation, aborts or
// succeeds. In the last case we disarm the guard.
//
// Note: This access to the source wouldn't be allowed by the TrustedRandomIteratorNoCoerce
// contract (used by SpecInPlaceCollect below). But see the "O(1) collect" section in the
// module documentation why this is ok anyway.
let dst_guard = InPlaceDstBufDrop { ptr: dst_buf, len, cap };
let dst_guard = InPlaceDstBufDrop { ptr: dst_buf, len, cap: dst_cap };
src.forget_allocation_drop_remaining();

// Adjust the allocation if the alignment didn't match or the source had a capacity in bytes
// that wasn't a multiple of the destination type size.
// Since the discrepancy should generally be small this should only result in some
// bookkeeping updates and no memmove.
if (const {
let src_sz = mem::size_of::<I::Src>();
src_sz > 0 && mem::size_of::<T>() % src_sz != 0
} && src_cap * mem::size_of::<I::Src>() != dst_cap * mem::size_of::<T>())
|| const { mem::align_of::<T>() != mem::align_of::<I::Src>() }
{
let alloc = Global;
unsafe {
// The old allocation exists, therefore it must have a valid layout.
let src_align = mem::align_of::<I::Src>();
let src_size = mem::size_of::<I::Src>().unchecked_mul(src_cap);
let old_layout = Layout::from_size_align_unchecked(src_size, src_align);

// The must be equal or smaller for in-place iteration to be possible
// therefore the new layout must be ≤ the old one and therefore valid.
let dst_align = mem::align_of::<T>();
let dst_size = mem::size_of::<T>().unchecked_mul(dst_cap);
let new_layout = Layout::from_size_align_unchecked(dst_size, dst_align);

let result = alloc.shrink(
NonNull::new_unchecked(dst_buf as *mut u8),
old_layout,
new_layout,
);
let Ok(reallocated) = result else { handle_alloc_error(new_layout) };
dst_buf = reallocated.as_ptr() as *mut T;
}
}

mem::forget(dst_guard);

let vec = unsafe { Vec::from_raw_parts(dst_buf, len, cap) };
let vec = unsafe { Vec::from_raw_parts(dst_buf, len, dst_cap) };

vec
}
Expand Down
12 changes: 10 additions & 2 deletions library/alloc/src/vec/into_iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ use crate::raw_vec::RawVec;
use core::array;
use core::fmt;
use core::iter::{
FusedIterator, InPlaceIterable, SourceIter, TrustedLen, TrustedRandomAccessNoCoerce,
FusedIterator, InPlaceIterable, SourceIter, TrustedFused, TrustedLen,
TrustedRandomAccessNoCoerce,
};
use core::marker::PhantomData;
use core::mem::{self, ManuallyDrop, MaybeUninit, SizedTypeProperties};
Expand Down Expand Up @@ -339,6 +340,10 @@ impl<T, A: Allocator> ExactSizeIterator for IntoIter<T, A> {
#[stable(feature = "fused", since = "1.26.0")]
impl<T, A: Allocator> FusedIterator for IntoIter<T, A> {}

#[doc(hidden)]
#[unstable(issue = "none", feature = "trusted_fused")]
unsafe impl<T, A: Allocator> TrustedFused for IntoIter<T, A> {}

#[unstable(feature = "trusted_len", issue = "37572")]
unsafe impl<T, A: Allocator> TrustedLen for IntoIter<T, A> {}

Expand Down Expand Up @@ -423,7 +428,10 @@ unsafe impl<#[may_dangle] T, A: Allocator> Drop for IntoIter<T, A> {
// also refer to the vec::in_place_collect module documentation to get an overview
#[unstable(issue = "none", feature = "inplace_iteration")]
#[doc(hidden)]
unsafe impl<T, A: Allocator> InPlaceIterable for IntoIter<T, A> {}
unsafe impl<T, A: Allocator> InPlaceIterable for IntoIter<T, A> {
const EXPAND_BY: Option<NonZeroUsize> = NonZeroUsize::new(1);
const MERGE_BY: Option<NonZeroUsize> = NonZeroUsize::new(1);
}

#[unstable(issue = "none", feature = "inplace_iteration")]
#[doc(hidden)]
Expand Down
1 change: 1 addition & 0 deletions library/alloc/tests/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![feature(allocator_api)]
#![feature(alloc_layout_extra)]
#![feature(iter_array_chunks)]
#![feature(assert_matches)]
#![feature(btree_extract_if)]
#![feature(cow_is_borrowed)]
Expand Down
45 changes: 43 additions & 2 deletions library/alloc/tests/vec.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use alloc::vec::Vec;
use core::alloc::{Allocator, Layout};
use core::assert_eq;
use core::iter::IntoIterator;
use core::{assert_eq, assert_ne};
use core::iter::{IntoIterator, Iterator};
use core::num::NonZeroUsize;
use core::ptr::NonNull;
use std::alloc::System;
Expand Down Expand Up @@ -1184,6 +1185,46 @@ fn test_from_iter_specialization_with_iterator_adapters() {
assert_eq!(srcptr, sinkptr as *const usize);
}

#[test]
fn test_in_place_specialization_step_up_down() {
fn assert_in_place_trait<T: InPlaceIterable>(_: &T) {}
let src = vec![[0u8; 4]; 256];
let srcptr = src.as_ptr();
let src_cap = src.capacity();
let iter = src.into_iter().flatten();
assert_in_place_trait(&iter);
let sink = iter.collect::<Vec<_>>();
let sinkptr = sink.as_ptr();
assert_eq!(srcptr as *const u8, sinkptr);
assert_eq!(src_cap * 4, sink.capacity());

let iter = sink.into_iter().array_chunks::<4>();
assert_in_place_trait(&iter);
let sink = iter.collect::<Vec<_>>();
let sinkptr = sink.as_ptr();
assert_eq!(srcptr, sinkptr);
assert_eq!(src_cap, sink.capacity());

let mut src: Vec<u8> = Vec::with_capacity(17);
let src_bytes = src.capacity();
src.resize(8, 0u8);
let sink: Vec<[u8; 4]> = src.into_iter().array_chunks::<4>().collect();
let sink_bytes = sink.capacity() * 4;
assert_ne!(src_bytes, sink_bytes);
assert_eq!(sink.len(), 2);

let src = vec![[0u8; 4]; 256];
let srcptr = src.as_ptr();
let iter = src
.into_iter()
.flat_map(|a| {
a.into_iter().map(|b| b.wrapping_add(1))
});
assert_in_place_trait(&iter);
let sink = iter.collect::<Vec<_>>();
assert_eq!(srcptr as *const u8, sink.as_ptr());
}

#[test]
fn test_from_iter_specialization_head_tail_drop() {
let drop_count: Vec<_> = (0..=2).map(|_| Rc::new(())).collect();
Expand Down
34 changes: 33 additions & 1 deletion library/core/src/iter/adapters/array_chunks.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
use crate::array;
use crate::iter::{ByRefSized, FusedIterator, Iterator, TrustedRandomAccessNoCoerce};
use crate::iter::adapters::SourceIter;
use crate::iter::{
ByRefSized, FusedIterator, InPlaceIterable, Iterator, TrustedFused, TrustedRandomAccessNoCoerce,
};
use crate::num::NonZeroUsize;
use crate::ops::{ControlFlow, NeverShortCircuit, Try};

/// An iterator over `N` elements of the iterator at a time.
Expand Down Expand Up @@ -159,6 +163,9 @@ where
#[unstable(feature = "iter_array_chunks", reason = "recently added", issue = "100450")]
impl<I, const N: usize> FusedIterator for ArrayChunks<I, N> where I: FusedIterator {}

#[unstable(issue = "none", feature = "trusted_fused")]
unsafe impl<I, const N: usize> TrustedFused for ArrayChunks<I, N> where I: TrustedFused + Iterator {}

#[unstable(feature = "iter_array_chunks", reason = "recently added", issue = "100450")]
impl<I, const N: usize> ExactSizeIterator for ArrayChunks<I, N>
where
Expand Down Expand Up @@ -229,3 +236,28 @@ where
accum
}
}

#[unstable(issue = "none", feature = "inplace_iteration")]
unsafe impl<I, const N: usize> SourceIter for ArrayChunks<I, N>
where
I: SourceIter + Iterator,
{
type Source = I::Source;

#[inline]
unsafe fn as_inner(&mut self) -> &mut I::Source {
// 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<I: InPlaceIterable + Iterator, const N: usize> InPlaceIterable for ArrayChunks<I, N> {
const EXPAND_BY: Option<NonZeroUsize> = I::EXPAND_BY;
const MERGE_BY: Option<NonZeroUsize> = const {
match (I::MERGE_BY, NonZeroUsize::new(N)) {
(Some(m), Some(n)) => m.checked_mul(n),
_ => None,
}
};
}
Loading
Loading