From 689c64c469152ab6de15c3950d5fe92ba95257f2 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Wed, 19 Jun 2019 01:39:51 +0200 Subject: [PATCH 01/13] Add basic 'shared_from_iter' impls. --- src/liballoc/rc.rs | 7 +++++++ src/liballoc/sync.rs | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index ee78839f7f003..df97b0efc6cdf 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -1213,6 +1213,13 @@ impl From> for Rc<[T]> { } } +#[stable(feature = "shared_from_iter", since = "1.37.0")] +impl core::iter::FromIterator for Rc<[T]> { + fn from_iter>(iter: I) -> Self { + iter.into_iter().collect::>().into() + } +} + /// `Weak` is a version of [`Rc`] that holds a non-owning reference to the /// managed value. The value is accessed by calling [`upgrade`] on the `Weak` /// pointer, which returns an [`Option`]`<`[`Rc`]`>`. diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index 6c23b3179ed68..a0b06fabe32e9 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -1785,6 +1785,13 @@ impl From> for Arc<[T]> { } } +#[stable(feature = "shared_from_iter", since = "1.37.0")] +impl core::iter::FromIterator for Arc<[T]> { + fn from_iter>(iter: I) -> Self { + iter.into_iter().collect::>().into() + } +} + #[cfg(test)] mod tests { use std::boxed::Box; From 19982f5653946b9d770f35560115ccc7a25356d4 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Wed, 19 Jun 2019 04:46:19 +0200 Subject: [PATCH 02/13] Rc: refactor away PhantomData noise. --- src/liballoc/rc.rs | 53 ++++++++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index df97b0efc6cdf..f9af752cae6cb 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -286,6 +286,19 @@ impl, U: ?Sized> CoerceUnsized> for Rc {} #[unstable(feature = "dispatch_from_dyn", issue = "0")] impl, U: ?Sized> DispatchFromDyn> for Rc {} +impl Rc { + fn from_inner(ptr: NonNull>) -> Self { + Self { + ptr, + phantom: PhantomData, + } + } + + unsafe fn from_ptr(ptr: *mut RcBox) -> Self { + Self::from_inner(NonNull::new_unchecked(ptr)) + } +} + impl Rc { /// Constructs a new `Rc`. /// @@ -298,18 +311,15 @@ impl Rc { /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn new(value: T) -> Rc { - Rc { - // there is an implicit weak pointer owned by all the strong - // pointers, which ensures that the weak destructor never frees - // the allocation while the strong destructor is running, even - // if the weak pointer is stored inside the strong one. - ptr: Box::into_raw_non_null(box RcBox { - strong: Cell::new(1), - weak: Cell::new(1), - value, - }), - phantom: PhantomData, - } + // There is an implicit weak pointer owned by all the strong + // pointers, which ensures that the weak destructor never frees + // the allocation while the strong destructor is running, even + // if the weak pointer is stored inside the strong one. + Self::from_inner(Box::into_raw_non_null(box RcBox { + strong: Cell::new(1), + weak: Cell::new(1), + value, + })) } /// Constructs a new `Pin>`. If `T` does not implement `Unpin`, then @@ -422,10 +432,7 @@ impl Rc { let fake_ptr = ptr as *mut RcBox; let rc_ptr = set_data_ptr(fake_ptr, (ptr as *mut u8).offset(-offset)); - Rc { - ptr: NonNull::new_unchecked(rc_ptr), - phantom: PhantomData, - } + Self::from_ptr(rc_ptr) } /// Consumes the `Rc`, returning the wrapped pointer as `NonNull`. @@ -683,7 +690,7 @@ impl Rc { if (*self).is::() { let ptr = self.ptr.cast::>(); forget(self); - Ok(Rc { ptr, phantom: PhantomData }) + Ok(Rc::from_inner(ptr)) } else { Err(self) } @@ -731,7 +738,7 @@ impl Rc { // Free the allocation without dropping its contents box_free(box_unique); - Rc { ptr: NonNull::new_unchecked(ptr), phantom: PhantomData } + Self::from_ptr(ptr) } } } @@ -758,7 +765,7 @@ impl Rc<[T]> { &mut (*ptr).value as *mut [T] as *mut T, v.len()); - Rc { ptr: NonNull::new_unchecked(ptr), phantom: PhantomData } + Self::from_ptr(ptr) } } @@ -800,7 +807,7 @@ impl RcFromSlice for Rc<[T]> { // Pointer to first element let elems = &mut (*ptr).value as *mut [T] as *mut T; - let mut guard = Guard{ + let mut guard = Guard { mem: NonNull::new_unchecked(mem), elems: elems, layout: layout, @@ -815,7 +822,7 @@ impl RcFromSlice for Rc<[T]> { // All clear. Forget the guard so it doesn't free the new RcBox. forget(guard); - Rc { ptr: NonNull::new_unchecked(ptr), phantom: PhantomData } + Self::from_ptr(ptr) } } } @@ -907,7 +914,7 @@ impl Clone for Rc { #[inline] fn clone(&self) -> Rc { self.inc_strong(); - Rc { ptr: self.ptr, phantom: PhantomData } + Self::from_inner(self.ptr) } } @@ -1463,7 +1470,7 @@ impl Weak { None } else { inner.inc_strong(); - Some(Rc { ptr: self.ptr, phantom: PhantomData }) + Some(Rc::from_inner(self.ptr)) } } From 2efbc9e5a2c5ae36ed2de0b23f315a5d6853b747 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Wed, 19 Jun 2019 04:48:46 +0200 Subject: [PATCH 03/13] Rc: refactor data_offset{_sized}. --- src/liballoc/rc.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index f9af752cae6cb..252b1c5a6dc7e 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -2176,18 +2176,19 @@ impl AsRef for Rc { impl Unpin for Rc { } unsafe fn data_offset(ptr: *const T) -> isize { - // Align the unsized value to the end of the RcBox. + // Align the unsized value to the end of the `RcBox`. // Because it is ?Sized, it will always be the last field in memory. - let align = align_of_val(&*ptr); - let layout = Layout::new::>(); - (layout.size() + layout.padding_needed_for(align)) as isize + data_offset_align(align_of_val(&*ptr)) } -/// Computes the offset of the data field within ArcInner. +/// Computes the offset of the data field within `RcBox`. /// /// Unlike [`data_offset`], this doesn't need the pointer, but it works only on `T: Sized`. fn data_offset_sized() -> isize { - let align = align_of::(); + data_offset_align(align_of::()) +} + +fn data_offset_align(align: usize) -> isize { let layout = Layout::new::>(); (layout.size() + layout.padding_needed_for(align)) as isize } From bf8f6c399b886480a2f2710531cf76de46b1c0cd Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Wed, 19 Jun 2019 04:56:58 +0200 Subject: [PATCH 04/13] Rc: reduce duplicate calls. --- src/liballoc/rc.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index 252b1c5a6dc7e..970b4745c31c4 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -1674,14 +1674,16 @@ trait RcBoxPtr { #[inline] fn inc_strong(&self) { + let strong = self.strong(); + // We want to abort on overflow instead of dropping the value. // The reference count will never be zero when this is called; // nevertheless, we insert an abort here to hint LLVM at // an otherwise missed optimization. - if self.strong() == 0 || self.strong() == usize::max_value() { + if strong == 0 || strong == usize::max_value() { unsafe { abort(); } } - self.inner().strong.set(self.strong() + 1); + self.inner().strong.set(strong + 1); } #[inline] @@ -1696,14 +1698,16 @@ trait RcBoxPtr { #[inline] fn inc_weak(&self) { + let weak = self.weak(); + // We want to abort on overflow instead of dropping the value. // The reference count will never be zero when this is called; // nevertheless, we insert an abort here to hint LLVM at // an otherwise missed optimization. - if self.weak() == 0 || self.weak() == usize::max_value() { + if weak == 0 || weak == usize::max_value() { unsafe { abort(); } } - self.inner().weak.set(self.weak() + 1); + self.inner().weak.set(weak + 1); } #[inline] From 353c8eb828ec7a68621334b91bf0f01eaf3ed769 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Wed, 19 Jun 2019 09:36:20 +0200 Subject: [PATCH 05/13] shared_from_iter/Rc: Use specialization to elide allocation. --- src/liballoc/lib.rs | 1 + src/liballoc/rc.rs | 206 +++++++++++++++++++++++++++++++++++--------- 2 files changed, 168 insertions(+), 39 deletions(-) diff --git a/src/liballoc/lib.rs b/src/liballoc/lib.rs index 5fc58c8ab5a7b..b679abacdc8d1 100644 --- a/src/liballoc/lib.rs +++ b/src/liballoc/lib.rs @@ -102,6 +102,7 @@ #![feature(try_reserve)] #![feature(unboxed_closures)] #![feature(unicode_internals)] +#![feature(untagged_unions)] #![feature(unsize)] #![feature(unsized_locals)] #![feature(allocator_internals)] diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index 970b4745c31c4..43b6b3cea15d6 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -238,12 +238,13 @@ use core::cmp::Ordering; use core::fmt; use core::hash::{Hash, Hasher}; use core::intrinsics::abort; +use core::iter; use core::marker::{self, Unpin, Unsize, PhantomData}; use core::mem::{self, align_of, align_of_val, forget, size_of_val}; use core::ops::{Deref, Receiver, CoerceUnsized, DispatchFromDyn}; use core::pin::Pin; use core::ptr::{self, NonNull}; -use core::slice::from_raw_parts_mut; +use core::slice::{self, from_raw_parts_mut}; use core::convert::From; use core::usize; @@ -698,21 +699,29 @@ impl Rc { } impl Rc { - // Allocates an `RcBox` with sufficient space for an unsized value - unsafe fn allocate_for_ptr(ptr: *const T) -> *mut RcBox { - // Calculate layout using the given value. + // Allocates an `RcBox` with sufficient space for + // an unsized value where the value has the layout provided. + // + // The function `mem_to_rcbox` is called with the data pointer + // and must return back a (potentially fat)-pointer for the `RcBox`. + unsafe fn allocate_for_unsized( + value_layout: Layout, + mem_to_rcbox: impl FnOnce(*mut u8) -> *mut RcBox + ) -> *mut RcBox { + // Calculate layout using the given value layout. // Previously, layout was calculated on the expression // `&*(ptr as *const RcBox)`, but this created a misaligned // reference (see #54908). let layout = Layout::new::>() - .extend(Layout::for_value(&*ptr)).unwrap().0 + .extend(value_layout).unwrap().0 .pad_to_align().unwrap(); + // Allocate for the layout. let mem = Global.alloc(layout) .unwrap_or_else(|_| handle_alloc_error(layout)); // Initialize the RcBox - let inner = set_data_ptr(ptr as *mut T, mem.as_ptr() as *mut u8) as *mut RcBox; + let inner = mem_to_rcbox(mem.as_ptr()); debug_assert_eq!(Layout::for_value(&*inner), layout); ptr::write(&mut (*inner).strong, Cell::new(1)); @@ -721,6 +730,15 @@ impl Rc { inner } + // Allocates an `RcBox` with sufficient space for an unsized value + unsafe fn allocate_for_ptr(ptr: *const T) -> *mut RcBox { + // Allocate for the `RcBox` using the given value. + Self::allocate_for_unsized( + Layout::for_value(&*ptr), + |mem| set_data_ptr(ptr as *mut T, mem) as *mut RcBox, + ) + } + fn from_box(v: Box) -> Rc { unsafe { let box_unique = Box::into_unique(v); @@ -743,6 +761,32 @@ impl Rc { } } +impl Rc<[T]> { + // Allocates an `RcBox<[T]>` with the given length. + unsafe fn allocate_for_slice(len: usize) -> *mut RcBox<[T]> { + // FIXME(#60667): Deduplicate. + fn slice_from_raw_parts_mut(data: *mut T, len: usize) -> *mut [T] { + #[repr(C)] + union Repr { + rust_mut: *mut [T], + raw: FatPtr, + } + + #[repr(C)] + struct FatPtr { + data: *const T, + len: usize, + } + unsafe { Repr { raw: FatPtr { data, len } }.rust_mut } + } + + Self::allocate_for_unsized( + Layout::array::(len).unwrap(), + |mem| slice_from_raw_parts_mut(mem as *mut T, len) as *mut RcBox<[T]>, + ) + } +} + // Sets the data pointer of a `?Sized` raw pointer. // // For a slice/trait object, this sets the `data` field and leaves the rest @@ -757,8 +801,7 @@ impl Rc<[T]> { // // Unsafe because the caller must either take ownership or bind `T: Copy` unsafe fn copy_from_slice(v: &[T]) -> Rc<[T]> { - let v_ptr = v as *const [T]; - let ptr = Self::allocate_for_ptr(v_ptr); + let ptr = Self::allocate_for_slice(v.len()); ptr::copy_nonoverlapping( v.as_ptr(), @@ -767,15 +810,11 @@ impl Rc<[T]> { Self::from_ptr(ptr) } -} -trait RcFromSlice { - fn from_slice(slice: &[T]) -> Self; -} - -impl RcFromSlice for Rc<[T]> { - #[inline] - default fn from_slice(v: &[T]) -> Self { + /// Constructs an `Rc<[T]>` from an iterator known to be of a certain size. + /// + /// Behavior is undefined should the size be wrong. + unsafe fn from_iter_exact(iter: impl iter::Iterator, len: usize) -> Rc<[T]> { // Panic guard while cloning T elements. // In the event of a panic, elements that have been written // into the new RcBox will be dropped, then the memory freed. @@ -797,32 +836,42 @@ impl RcFromSlice for Rc<[T]> { } } - unsafe { - let v_ptr = v as *const [T]; - let ptr = Self::allocate_for_ptr(v_ptr); + let ptr = Self::allocate_for_slice(len); - let mem = ptr as *mut _ as *mut u8; - let layout = Layout::for_value(&*ptr); + let mem = ptr as *mut _ as *mut u8; + let layout = Layout::for_value(&*ptr); - // Pointer to first element - let elems = &mut (*ptr).value as *mut [T] as *mut T; + // Pointer to first element + let elems = &mut (*ptr).value as *mut [T] as *mut T; - let mut guard = Guard { - mem: NonNull::new_unchecked(mem), - elems: elems, - layout: layout, - n_elems: 0, - }; + let mut guard = Guard { + mem: NonNull::new_unchecked(mem), + elems, + layout, + n_elems: 0, + }; - for (i, item) in v.iter().enumerate() { - ptr::write(elems.add(i), item.clone()); - guard.n_elems += 1; - } + for (i, item) in iter.enumerate() { + ptr::write(elems.add(i), item); + guard.n_elems += 1; + } - // All clear. Forget the guard so it doesn't free the new RcBox. - forget(guard); + // All clear. Forget the guard so it doesn't free the new RcBox. + forget(guard); - Self::from_ptr(ptr) + Self::from_ptr(ptr) + } +} + +trait RcFromSlice { + fn from_slice(slice: &[T]) -> Self; +} + +impl RcFromSlice for Rc<[T]> { + #[inline] + default fn from_slice(v: &[T]) -> Self { + unsafe { + Self::from_iter_exact(v.iter().cloned(), v.len()) } } } @@ -1221,9 +1270,88 @@ impl From> for Rc<[T]> { } #[stable(feature = "shared_from_iter", since = "1.37.0")] -impl core::iter::FromIterator for Rc<[T]> { - fn from_iter>(iter: I) -> Self { - iter.into_iter().collect::>().into() +impl iter::FromIterator for Rc<[T]> { + /// Takes each element in the `Iterator` and collects it into an `Rc<[T]>`. + /// + /// # Performance characteristics + /// + /// ## The general case + /// + /// In the general case, collecting into `Rc<[T]>` is done by first + /// collecting into a `Vec`. That is, when writing the following: + /// + /// ```rust + /// # use std::rc::Rc; + /// let evens: Rc<[u8]> = (0..10).filter(|&x| x % 2 == 0).collect(); + /// # assert_eq!(&*evens, &[0, 2, 4, 6, 8]); + /// ``` + /// + /// this behaves as if we wrote: + /// + /// ```rust + /// # use std::rc::Rc; + /// let evens: Rc<[u8]> = (0..10).filter(|&x| x % 2 == 0) + /// .collect::>() // The first set of allocations happens here. + /// .into(); // A second allocation for `Rc<[T]>` happens here. + /// # assert_eq!(&*evens, &[0, 2, 4, 6, 8]); + /// ``` + /// + /// This will allocate as many times as needed for constructing the `Vec` + /// and then it will allocate once for turning the `Vec` into the `Rc<[T]>`. + /// + /// ## Iterators of known length + /// + /// When your `Iterator` implements `TrustedLen` and is of an exact size, + /// a single allocation will be made for the `Rc<[T]>`. For example: + /// + /// ```rust + /// # use std::rc::Rc; + /// let evens: Rc<[u8]> = (0..10).collect(); // Just a single allocation happens here. + /// # assert_eq!(&*evens, &*(0..10).collect::>()); + /// ``` + fn from_iter>(iter: I) -> Self { + RcFromIter::from_iter(iter.into_iter()) + } +} + +/// Specialization trait used for collecting into `Rc<[T]>`. +trait RcFromIter { + fn from_iter(iter: I) -> Self; +} + +impl> RcFromIter for Rc<[T]> { + default fn from_iter(iter: I) -> Self { + iter.collect::>().into() + } +} + +impl> RcFromIter for Rc<[T]> { + default fn from_iter(iter: I) -> Self { + // This is the case for a `TrustedLen` iterator. + let (low, high) = iter.size_hint(); + if let Some(high) = high { + debug_assert_eq!( + low, high, + "TrustedLen iterator's size hint is not exact: {:?}", + (low, high) + ); + + unsafe { + // SAFETY: We need to ensure that the iterator has an exact length and we have. + Rc::from_iter_exact(iter, low) + } + } else { + // Fall back to normal implementation. + iter.collect::>().into() + } + } +} + +impl<'a, T: 'a + Clone> RcFromIter<&'a T, slice::Iter<'a, T>> for Rc<[T]> { + fn from_iter(iter: slice::Iter<'a, T>) -> Self { + // Delegate to `impl From<&[T]> for Rc<[T]>` + // which will use `ptr::copy_nonoverlapping`. + iter.as_slice().into() } } From 27f5d0f208e12176c614d6ffbd410f7b53ed9eed Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Wed, 19 Jun 2019 09:56:34 +0200 Subject: [PATCH 06/13] Arc: refactor away PhantomData noise. --- src/liballoc/sync.rs | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index a0b06fabe32e9..de47e164c9201 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -206,6 +206,19 @@ impl, U: ?Sized> CoerceUnsized> for Arc {} #[unstable(feature = "dispatch_from_dyn", issue = "0")] impl, U: ?Sized> DispatchFromDyn> for Arc {} +impl Arc { + fn from_inner(ptr: NonNull>) -> Self { + Self { + ptr, + phantom: PhantomData, + } + } + + unsafe fn from_ptr(ptr: *mut ArcInner) -> Self { + Self::from_inner(NonNull::new_unchecked(ptr)) + } +} + /// `Weak` is a version of [`Arc`] that holds a non-owning reference to the /// managed value. The value is accessed by calling [`upgrade`] on the `Weak` /// pointer, which returns an [`Option`]`<`[`Arc`]`>`. @@ -290,7 +303,7 @@ impl Arc { weak: atomic::AtomicUsize::new(1), data, }; - Arc { ptr: Box::into_raw_non_null(x), phantom: PhantomData } + Self::from_inner(Box::into_raw_non_null(x)) } /// Constructs a new `Pin>`. If `T` does not implement `Unpin`, then @@ -403,10 +416,7 @@ impl Arc { let fake_ptr = ptr as *mut ArcInner; let arc_ptr = set_data_ptr(fake_ptr, (ptr as *mut u8).offset(-offset)); - Arc { - ptr: NonNull::new_unchecked(arc_ptr), - phantom: PhantomData, - } + Self::from_ptr(arc_ptr) } /// Consumes the `Arc`, returning the wrapped pointer as `NonNull`. @@ -617,7 +627,7 @@ impl Arc { // Free the allocation without dropping its contents box_free(box_unique); - Arc { ptr: NonNull::new_unchecked(ptr), phantom: PhantomData } + Self::from_ptr(ptr) } } } @@ -644,7 +654,7 @@ impl Arc<[T]> { &mut (*ptr).data as *mut [T] as *mut T, v.len()); - Arc { ptr: NonNull::new_unchecked(ptr), phantom: PhantomData } + Self::from_ptr(ptr) } } @@ -702,7 +712,7 @@ impl ArcFromSlice for Arc<[T]> { // All clear. Forget the guard so it doesn't free the new ArcInner. mem::forget(guard); - Arc { ptr: NonNull::new_unchecked(ptr), phantom: PhantomData } + Self::from_ptr(ptr) } } } @@ -760,7 +770,7 @@ impl Clone for Arc { } } - Arc { ptr: self.ptr, phantom: PhantomData } + Self::from_inner(self.ptr) } } @@ -1039,7 +1049,7 @@ impl Arc { if (*self).is::() { let ptr = self.ptr.cast::>(); mem::forget(self); - Ok(Arc { ptr, phantom: PhantomData }) + Ok(Arc::from_inner(ptr)) } else { Err(self) } @@ -1260,11 +1270,7 @@ impl Weak { // Relaxed is valid for the same reason it is on Arc's Clone impl match inner.strong.compare_exchange_weak(n, n + 1, Relaxed, Relaxed) { - Ok(_) => return Some(Arc { - // null checked above - ptr: self.ptr, - phantom: PhantomData, - }), + Ok(_) => return Some(Arc::from_inner(self.ptr)), // null checked above Err(old) => n = old, } } From 59ecff915ce3fbef44ca5591d69c2c908a88ca0b Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Wed, 19 Jun 2019 10:00:24 +0200 Subject: [PATCH 07/13] Arc: refactor data_offset{_sized}. --- src/liballoc/sync.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index de47e164c9201..127068284d33b 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -2298,20 +2298,21 @@ impl AsRef for Arc { #[stable(feature = "pin", since = "1.33.0")] impl Unpin for Arc { } -/// Computes the offset of the data field within ArcInner. +/// Computes the offset of the data field within `ArcInner`. unsafe fn data_offset(ptr: *const T) -> isize { - // Align the unsized value to the end of the ArcInner. - // Because it is ?Sized, it will always be the last field in memory. - let align = align_of_val(&*ptr); - let layout = Layout::new::>(); - (layout.size() + layout.padding_needed_for(align)) as isize + // Align the unsized value to the end of the `ArcInner`. + // Because it is `?Sized`, it will always be the last field in memory. + data_offset_align(align_of_val(&*ptr)) } -/// Computes the offset of the data field within ArcInner. +/// Computes the offset of the data field within `ArcInner`. /// /// Unlike [`data_offset`], this doesn't need the pointer, but it works only on `T: Sized`. fn data_offset_sized() -> isize { - let align = align_of::(); + data_offset_align(align_of::()) +} + +fn data_offset_align(align: usize) -> isize { let layout = Layout::new::>(); (layout.size() + layout.padding_needed_for(align)) as isize } From b1dbf15bb5d06b8213b8ed3c61b0f92faf1c001c Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Wed, 19 Jun 2019 10:23:28 +0200 Subject: [PATCH 08/13] shared_from_iter/Arc: Use specialization to elide allocation. --- src/liballoc/sync.rs | 207 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 167 insertions(+), 40 deletions(-) diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index 127068284d33b..caf727081b11f 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -12,6 +12,7 @@ use core::sync::atomic::Ordering::{Acquire, Relaxed, Release, SeqCst}; use core::borrow; use core::fmt; use core::cmp::{self, Ordering}; +use core::iter; use core::intrinsics::abort; use core::mem::{self, align_of, align_of_val, size_of_val}; use core::ops::{Deref, Receiver, CoerceUnsized, DispatchFromDyn}; @@ -21,7 +22,7 @@ use core::marker::{Unpin, Unsize, PhantomData}; use core::hash::{Hash, Hasher}; use core::{isize, usize}; use core::convert::From; -use core::slice::from_raw_parts_mut; +use core::slice::{self, from_raw_parts_mut}; use crate::alloc::{Global, Alloc, Layout, box_free, handle_alloc_error}; use crate::boxed::Box; @@ -587,21 +588,28 @@ impl Arc { } impl Arc { - // Allocates an `ArcInner` with sufficient space for an unsized value - unsafe fn allocate_for_ptr(ptr: *const T) -> *mut ArcInner { - // Calculate layout using the given value. + // Allocates an `ArcInner` with sufficient space for + // an unsized value where the value has the layout provided. + // + // The function `mem_to_arcinner` is called with the data pointer + // and must return back a (potentially fat)-pointer for the `ArcInner`. + unsafe fn allocate_for_unsized( + value_layout: Layout, + mem_to_arcinner: impl FnOnce(*mut u8) -> *mut ArcInner + ) -> *mut ArcInner { + // Calculate layout using the given value layout. // Previously, layout was calculated on the expression // `&*(ptr as *const ArcInner)`, but this created a misaligned // reference (see #54908). let layout = Layout::new::>() - .extend(Layout::for_value(&*ptr)).unwrap().0 + .extend(value_layout).unwrap().0 .pad_to_align().unwrap(); let mem = Global.alloc(layout) .unwrap_or_else(|_| handle_alloc_error(layout)); // Initialize the ArcInner - let inner = set_data_ptr(ptr as *mut T, mem.as_ptr() as *mut u8) as *mut ArcInner; + let inner = mem_to_arcinner(mem.as_ptr()); debug_assert_eq!(Layout::for_value(&*inner), layout); ptr::write(&mut (*inner).strong, atomic::AtomicUsize::new(1)); @@ -610,6 +618,15 @@ impl Arc { inner } + // Allocates an `ArcInner` with sufficient space for an unsized value + unsafe fn allocate_for_ptr(ptr: *const T) -> *mut ArcInner { + // Allocate for the `ArcInner` using the given value. + Self::allocate_for_unsized( + Layout::for_value(&*ptr), + |mem| set_data_ptr(ptr as *mut T, mem) as *mut ArcInner, + ) + } + fn from_box(v: Box) -> Arc { unsafe { let box_unique = Box::into_unique(v); @@ -632,6 +649,32 @@ impl Arc { } } +impl Arc<[T]> { + // Allocates an `ArcInner<[T]>` with the given length. + unsafe fn allocate_for_slice(len: usize) -> *mut ArcInner<[T]> { + // FIXME(#60667): Deduplicate. + fn slice_from_raw_parts_mut(data: *mut T, len: usize) -> *mut [T] { + #[repr(C)] + union Repr { + rust_mut: *mut [T], + raw: FatPtr, + } + + #[repr(C)] + struct FatPtr { + data: *const T, + len: usize, + } + unsafe { Repr { raw: FatPtr { data, len } }.rust_mut } + } + + Self::allocate_for_unsized( + Layout::array::(len).unwrap(), + |mem| slice_from_raw_parts_mut(mem as *mut T, len) as *mut ArcInner<[T]>, + ) + } +} + // Sets the data pointer of a `?Sized` raw pointer. // // For a slice/trait object, this sets the `data` field and leaves the rest @@ -646,8 +689,7 @@ impl Arc<[T]> { // // Unsafe because the caller must either take ownership or bind `T: Copy` unsafe fn copy_from_slice(v: &[T]) -> Arc<[T]> { - let v_ptr = v as *const [T]; - let ptr = Self::allocate_for_ptr(v_ptr); + let ptr = Self::allocate_for_slice(v.len()); ptr::copy_nonoverlapping( v.as_ptr(), @@ -656,16 +698,11 @@ impl Arc<[T]> { Self::from_ptr(ptr) } -} -// Specialization trait used for From<&[T]> -trait ArcFromSlice { - fn from_slice(slice: &[T]) -> Self; -} - -impl ArcFromSlice for Arc<[T]> { - #[inline] - default fn from_slice(v: &[T]) -> Self { + /// Constructs an `Arc<[T]>` from an iterator known to be of a certain size. + /// + /// Behavior is undefined should the size be wrong. + unsafe fn from_iter_exact(iter: impl iter::Iterator, len: usize) -> Arc<[T]> { // Panic guard while cloning T elements. // In the event of a panic, elements that have been written // into the new ArcInner will be dropped, then the memory freed. @@ -687,32 +724,43 @@ impl ArcFromSlice for Arc<[T]> { } } - unsafe { - let v_ptr = v as *const [T]; - let ptr = Self::allocate_for_ptr(v_ptr); + let ptr = Self::allocate_for_slice(len); + + let mem = ptr as *mut _ as *mut u8; + let layout = Layout::for_value(&*ptr); - let mem = ptr as *mut _ as *mut u8; - let layout = Layout::for_value(&*ptr); + // Pointer to first element + let elems = &mut (*ptr).data as *mut [T] as *mut T; - // Pointer to first element - let elems = &mut (*ptr).data as *mut [T] as *mut T; + let mut guard = Guard { + mem: NonNull::new_unchecked(mem), + elems, + layout, + n_elems: 0, + }; - let mut guard = Guard{ - mem: NonNull::new_unchecked(mem), - elems: elems, - layout: layout, - n_elems: 0, - }; + for (i, item) in iter.enumerate() { + ptr::write(elems.add(i), item); + guard.n_elems += 1; + } - for (i, item) in v.iter().enumerate() { - ptr::write(elems.add(i), item.clone()); - guard.n_elems += 1; - } + // All clear. Forget the guard so it doesn't free the new ArcInner. + mem::forget(guard); - // All clear. Forget the guard so it doesn't free the new ArcInner. - mem::forget(guard); + Self::from_ptr(ptr) + } +} - Self::from_ptr(ptr) +// Specialization trait used for From<&[T]> +trait ArcFromSlice { + fn from_slice(slice: &[T]) -> Self; +} + +impl ArcFromSlice for Arc<[T]> { + #[inline] + default fn from_slice(v: &[T]) -> Self { + unsafe { + Self::from_iter_exact(v.iter().cloned(), v.len()) } } } @@ -1792,9 +1840,88 @@ impl From> for Arc<[T]> { } #[stable(feature = "shared_from_iter", since = "1.37.0")] -impl core::iter::FromIterator for Arc<[T]> { - fn from_iter>(iter: I) -> Self { - iter.into_iter().collect::>().into() +impl iter::FromIterator for Arc<[T]> { + /// Takes each element in the `Iterator` and collects it into an `Arc<[T]>`. + /// + /// # Performance characteristics + /// + /// ## The general case + /// + /// In the general case, collecting into `Arc<[T]>` is done by first + /// collecting into a `Vec`. That is, when writing the following: + /// + /// ```rust + /// # use std::sync::Arc; + /// let evens: Arc<[u8]> = (0..10).filter(|&x| x % 2 == 0).collect(); + /// # assert_eq!(&*evens, &[0, 2, 4, 6, 8]); + /// ``` + /// + /// this behaves as if we wrote: + /// + /// ```rust + /// # use std::sync::Arc; + /// let evens: Arc<[u8]> = (0..10).filter(|&x| x % 2 == 0) + /// .collect::>() // The first set of allocations happens here. + /// .into(); // A second allocation for `Arc<[T]>` happens here. + /// # assert_eq!(&*evens, &[0, 2, 4, 6, 8]); + /// ``` + /// + /// This will allocate as many times as needed for constructing the `Vec` + /// and then it will allocate once for turning the `Vec` into the `Arc<[T]>`. + /// + /// ## Iterators of known length + /// + /// When your `Iterator` implements `TrustedLen` and is of an exact size, + /// a single allocation will be made for the `Arc<[T]>`. For example: + /// + /// ```rust + /// # use std::sync::Arc; + /// let evens: Arc<[u8]> = (0..10).collect(); // Just a single allocation happens here. + /// # assert_eq!(&*evens, &*(0..10).collect::>()); + /// ``` + fn from_iter>(iter: I) -> Self { + ArcFromIter::from_iter(iter.into_iter()) + } +} + +/// Specialization trait used for collecting into `Arc<[T]>`. +trait ArcFromIter { + fn from_iter(iter: I) -> Self; +} + +impl> ArcFromIter for Arc<[T]> { + default fn from_iter(iter: I) -> Self { + iter.collect::>().into() + } +} + +impl> ArcFromIter for Arc<[T]> { + default fn from_iter(iter: I) -> Self { + // This is the case for a `TrustedLen` iterator. + let (low, high) = iter.size_hint(); + if let Some(high) = high { + debug_assert_eq!( + low, high, + "TrustedLen iterator's size hint is not exact: {:?}", + (low, high) + ); + + unsafe { + // SAFETY: We need to ensure that the iterator has an exact length and we have. + Arc::from_iter_exact(iter, low) + } + } else { + // Fall back to normal implementation. + iter.collect::>().into() + } + } +} + +impl<'a, T: 'a + Clone> ArcFromIter<&'a T, slice::Iter<'a, T>> for Arc<[T]> { + fn from_iter(iter: slice::Iter<'a, T>) -> Self { + // Delegate to `impl From<&[T]> for Arc<[T]>` + // which will use `ptr::copy_nonoverlapping`. + iter.as_slice().into() } } From 4b44ad9038ec421b21a971429e55475f1c1bd81e Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 20 Jun 2019 09:28:01 +0200 Subject: [PATCH 09/13] deduplicate slice_from_raw_parts_mut. --- src/liballoc/lib.rs | 2 +- src/liballoc/rc.rs | 18 +----------------- src/liballoc/sync.rs | 18 +----------------- 3 files changed, 3 insertions(+), 35 deletions(-) diff --git a/src/liballoc/lib.rs b/src/liballoc/lib.rs index b679abacdc8d1..cef41f974e436 100644 --- a/src/liballoc/lib.rs +++ b/src/liballoc/lib.rs @@ -94,6 +94,7 @@ #![feature(ptr_offset_from)] #![feature(rustc_attrs)] #![feature(receiver_trait)] +#![feature(slice_from_raw_parts)] #![feature(specialization)] #![feature(staged_api)] #![feature(std_internals)] @@ -102,7 +103,6 @@ #![feature(try_reserve)] #![feature(unboxed_closures)] #![feature(unicode_internals)] -#![feature(untagged_unions)] #![feature(unsize)] #![feature(unsized_locals)] #![feature(allocator_internals)] diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index 43b6b3cea15d6..1e9cd3ff39694 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -764,25 +764,9 @@ impl Rc { impl Rc<[T]> { // Allocates an `RcBox<[T]>` with the given length. unsafe fn allocate_for_slice(len: usize) -> *mut RcBox<[T]> { - // FIXME(#60667): Deduplicate. - fn slice_from_raw_parts_mut(data: *mut T, len: usize) -> *mut [T] { - #[repr(C)] - union Repr { - rust_mut: *mut [T], - raw: FatPtr, - } - - #[repr(C)] - struct FatPtr { - data: *const T, - len: usize, - } - unsafe { Repr { raw: FatPtr { data, len } }.rust_mut } - } - Self::allocate_for_unsized( Layout::array::(len).unwrap(), - |mem| slice_from_raw_parts_mut(mem as *mut T, len) as *mut RcBox<[T]>, + |mem| ptr::slice_from_raw_parts_mut(mem as *mut T, len) as *mut RcBox<[T]>, ) } } diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index caf727081b11f..07eae91689487 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -652,25 +652,9 @@ impl Arc { impl Arc<[T]> { // Allocates an `ArcInner<[T]>` with the given length. unsafe fn allocate_for_slice(len: usize) -> *mut ArcInner<[T]> { - // FIXME(#60667): Deduplicate. - fn slice_from_raw_parts_mut(data: *mut T, len: usize) -> *mut [T] { - #[repr(C)] - union Repr { - rust_mut: *mut [T], - raw: FatPtr, - } - - #[repr(C)] - struct FatPtr { - data: *const T, - len: usize, - } - unsafe { Repr { raw: FatPtr { data, len } }.rust_mut } - } - Self::allocate_for_unsized( Layout::array::(len).unwrap(), - |mem| slice_from_raw_parts_mut(mem as *mut T, len) as *mut ArcInner<[T]>, + |mem| ptr::slice_from_raw_parts_mut(mem as *mut T, len) as *mut ArcInner<[T]>, ) } } From 85978d028ad6edb1530c494682c05b46f26900f6 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 20 Jun 2019 23:13:06 +0200 Subject: [PATCH 10/13] data_offset_align: add inline attribute. --- src/liballoc/rc.rs | 1 + src/liballoc/sync.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index 1e9cd3ff39694..a222dbc93a81e 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -2304,6 +2304,7 @@ fn data_offset_sized() -> isize { data_offset_align(align_of::()) } +#[inline] fn data_offset_align(align: usize) -> isize { let layout = Layout::new::>(); (layout.size() + layout.padding_needed_for(align)) as isize diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index 07eae91689487..f5110905a118f 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -2423,6 +2423,7 @@ fn data_offset_sized() -> isize { data_offset_align(align_of::()) } +#[inline] fn data_offset_align(align: usize) -> isize { let layout = Layout::new::>(); (layout.size() + layout.padding_needed_for(align)) as isize From 6b8417b55c59f3412e97131fd1f99e34bdd8f5d2 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 20 Jun 2019 23:20:21 +0200 Subject: [PATCH 11/13] shared_from_iter: Clarify slice::Iter specialization impl. --- src/liballoc/rc.rs | 10 ++++++++-- src/liballoc/sync.rs | 10 ++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index a222dbc93a81e..9b653fe2d75a2 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -1333,8 +1333,14 @@ impl> RcFromIter for Rc<[T]> { impl<'a, T: 'a + Clone> RcFromIter<&'a T, slice::Iter<'a, T>> for Rc<[T]> { fn from_iter(iter: slice::Iter<'a, T>) -> Self { - // Delegate to `impl From<&[T]> for Rc<[T]>` - // which will use `ptr::copy_nonoverlapping`. + // Delegate to `impl From<&[T]> for Rc<[T]>`. + // + // In the case that `T: Copy`, we get to use `ptr::copy_nonoverlapping` + // which is even more performant. + // + // In the fall-back case we have `T: Clone`. This is still better + // than the `TrustedLen` implementation as slices have a known length + // and so we get to avoid calling `size_hint` and avoid the branching. iter.as_slice().into() } } diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index f5110905a118f..672481ca0defd 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -1903,8 +1903,14 @@ impl> ArcFromIter for Arc<[T]> { impl<'a, T: 'a + Clone> ArcFromIter<&'a T, slice::Iter<'a, T>> for Arc<[T]> { fn from_iter(iter: slice::Iter<'a, T>) -> Self { - // Delegate to `impl From<&[T]> for Arc<[T]>` - // which will use `ptr::copy_nonoverlapping`. + // Delegate to `impl From<&[T]> for Rc<[T]>`. + // + // In the case that `T: Copy`, we get to use `ptr::copy_nonoverlapping` + // which is even more performant. + // + // In the fall-back case we have `T: Clone`. This is still better + // than the `TrustedLen` implementation as slices have a known length + // and so we get to avoid calling `size_hint` and avoid the branching. iter.as_slice().into() } } From 8bbf1abd0ec903ff4408238e86d712aeba1cbd7f Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 21 Jun 2019 03:52:38 +0200 Subject: [PATCH 12/13] shared_from_iter: Add more tests. --- src/liballoc/tests/arc.rs | 119 ++++++++++++++++++++++++++++++++++++++ src/liballoc/tests/lib.rs | 2 + src/liballoc/tests/rc.rs | 117 +++++++++++++++++++++++++++++++++++++ 3 files changed, 238 insertions(+) diff --git a/src/liballoc/tests/arc.rs b/src/liballoc/tests/arc.rs index 2759b1b1cac27..ce64e2de01377 100644 --- a/src/liballoc/tests/arc.rs +++ b/src/liballoc/tests/arc.rs @@ -2,6 +2,8 @@ use std::any::Any; use std::sync::{Arc, Weak}; use std::cell::RefCell; use std::cmp::PartialEq; +use std::iter::TrustedLen; +use std::mem; #[test] fn uninhabited() { @@ -85,3 +87,120 @@ fn eq() { assert!(!(x != x)); assert_eq!(*x.0.borrow(), 0); } + +type Rc = Arc; + +const SHARED_ITER_MAX: u16 = 100; + +fn assert_trusted_len(_: &I) {} + +#[test] +fn shared_from_iter_normal() { + // Exercise the base implementation for non-`TrustedLen` iterators. + { + // `Filter` is never `TrustedLen` since we don't + // know statically how many elements will be kept: + let iter = (0..SHARED_ITER_MAX).filter(|x| x % 2 == 0).map(Box::new); + + // Collecting into a `Vec` or `Rc<[T]>` should make no difference: + let vec = iter.clone().collect::>(); + let rc = iter.collect::>(); + assert_eq!(&*vec, &*rc); + + // Clone a bit and let these get dropped. + { + let _rc_2 = rc.clone(); + let _rc_3 = rc.clone(); + let _rc_4 = Rc::downgrade(&_rc_3); + } + } // Drop what hasn't been here. +} + +#[test] +fn shared_from_iter_trustedlen_normal() { + // Exercise the `TrustedLen` implementation under normal circumstances + // where `size_hint()` matches `(_, Some(exact_len))`. + { + let iter = (0..SHARED_ITER_MAX).map(Box::new); + assert_trusted_len(&iter); + + // Collecting into a `Vec` or `Rc<[T]>` should make no difference: + let vec = iter.clone().collect::>(); + let rc = iter.collect::>(); + assert_eq!(&*vec, &*rc); + assert_eq!(mem::size_of::>() * SHARED_ITER_MAX as usize, mem::size_of_val(&*rc)); + + // Clone a bit and let these get dropped. + { + let _rc_2 = rc.clone(); + let _rc_3 = rc.clone(); + let _rc_4 = Rc::downgrade(&_rc_3); + } + } // Drop what hasn't been here. + + // Try a ZST to make sure it is handled well. + { + let iter = (0..SHARED_ITER_MAX).map(|_| ()); + let vec = iter.clone().collect::>(); + let rc = iter.collect::>(); + assert_eq!(&*vec, &*rc); + assert_eq!(0, mem::size_of_val(&*rc)); + { + let _rc_2 = rc.clone(); + let _rc_3 = rc.clone(); + let _rc_4 = Rc::downgrade(&_rc_3); + } + } +} + +#[test] +#[should_panic = "I've almost got 99 problems."] +fn shared_from_iter_trustedlen_panic() { + // Exercise the `TrustedLen` implementation when `size_hint()` matches + // `(_, Some(exact_len))` but where `.next()` drops before the last iteration. + let iter = (0..SHARED_ITER_MAX) + .map(|val| { + match val { + 98 => panic!("I've almost got 99 problems."), + _ => Box::new(val), + } + }); + assert_trusted_len(&iter); + let _ = iter.collect::>(); + + panic!("I am unreachable."); +} + +#[test] +fn shared_from_iter_trustedlen_no_fuse() { + // Exercise the `TrustedLen` implementation when `size_hint()` matches + // `(_, Some(exact_len))` but where the iterator does not behave in a fused manner. + struct Iter(std::vec::IntoIter>>); + + unsafe impl TrustedLen for Iter {} + + impl Iterator for Iter { + fn size_hint(&self) -> (usize, Option) { + (2, Some(2)) + } + + type Item = Box; + + fn next(&mut self) -> Option { + self.0.next().flatten() + } + } + + let vec = vec![ + Some(Box::new(42)), + Some(Box::new(24)), + None, + Some(Box::new(12)), + ]; + let iter = Iter(vec.into_iter()); + assert_trusted_len(&iter); + assert_eq!( + &[Box::new(42), Box::new(24)], + &*iter.collect::>() + ); +} diff --git a/src/liballoc/tests/lib.rs b/src/liballoc/tests/lib.rs index ddb3120e89d78..5a43c8e09a2a8 100644 --- a/src/liballoc/tests/lib.rs +++ b/src/liballoc/tests/lib.rs @@ -2,8 +2,10 @@ #![feature(box_syntax)] #![feature(drain_filter)] #![feature(exact_size_is_empty)] +#![feature(option_flattening)] #![feature(pattern)] #![feature(repeat_generic_slice)] +#![feature(trusted_len)] #![feature(try_reserve)] #![feature(unboxed_closures)] #![deny(rust_2018_idioms)] diff --git a/src/liballoc/tests/rc.rs b/src/liballoc/tests/rc.rs index 18f82e8041008..7854ca0fc16b2 100644 --- a/src/liballoc/tests/rc.rs +++ b/src/liballoc/tests/rc.rs @@ -2,6 +2,8 @@ use std::any::Any; use std::rc::{Rc, Weak}; use std::cell::RefCell; use std::cmp::PartialEq; +use std::mem; +use std::iter::TrustedLen; #[test] fn uninhabited() { @@ -85,3 +87,118 @@ fn eq() { assert!(!(x != x)); assert_eq!(*x.0.borrow(), 0); } + +const SHARED_ITER_MAX: u16 = 100; + +fn assert_trusted_len(_: &I) {} + +#[test] +fn shared_from_iter_normal() { + // Exercise the base implementation for non-`TrustedLen` iterators. + { + // `Filter` is never `TrustedLen` since we don't + // know statically how many elements will be kept: + let iter = (0..SHARED_ITER_MAX).filter(|x| x % 2 == 0).map(Box::new); + + // Collecting into a `Vec` or `Rc<[T]>` should make no difference: + let vec = iter.clone().collect::>(); + let rc = iter.collect::>(); + assert_eq!(&*vec, &*rc); + + // Clone a bit and let these get dropped. + { + let _rc_2 = rc.clone(); + let _rc_3 = rc.clone(); + let _rc_4 = Rc::downgrade(&_rc_3); + } + } // Drop what hasn't been here. +} + +#[test] +fn shared_from_iter_trustedlen_normal() { + // Exercise the `TrustedLen` implementation under normal circumstances + // where `size_hint()` matches `(_, Some(exact_len))`. + { + let iter = (0..SHARED_ITER_MAX).map(Box::new); + assert_trusted_len(&iter); + + // Collecting into a `Vec` or `Rc<[T]>` should make no difference: + let vec = iter.clone().collect::>(); + let rc = iter.collect::>(); + assert_eq!(&*vec, &*rc); + assert_eq!(mem::size_of::>() * SHARED_ITER_MAX as usize, mem::size_of_val(&*rc)); + + // Clone a bit and let these get dropped. + { + let _rc_2 = rc.clone(); + let _rc_3 = rc.clone(); + let _rc_4 = Rc::downgrade(&_rc_3); + } + } // Drop what hasn't been here. + + // Try a ZST to make sure it is handled well. + { + let iter = (0..SHARED_ITER_MAX).map(|_| ()); + let vec = iter.clone().collect::>(); + let rc = iter.collect::>(); + assert_eq!(&*vec, &*rc); + assert_eq!(0, mem::size_of_val(&*rc)); + { + let _rc_2 = rc.clone(); + let _rc_3 = rc.clone(); + let _rc_4 = Rc::downgrade(&_rc_3); + } + } +} + +#[test] +#[should_panic = "I've almost got 99 problems."] +fn shared_from_iter_trustedlen_panic() { + // Exercise the `TrustedLen` implementation when `size_hint()` matches + // `(_, Some(exact_len))` but where `.next()` drops before the last iteration. + let iter = (0..SHARED_ITER_MAX) + .map(|val| { + match val { + 98 => panic!("I've almost got 99 problems."), + _ => Box::new(val), + } + }); + assert_trusted_len(&iter); + let _ = iter.collect::>(); + + panic!("I am unreachable."); +} + +#[test] +fn shared_from_iter_trustedlen_no_fuse() { + // Exercise the `TrustedLen` implementation when `size_hint()` matches + // `(_, Some(exact_len))` but where the iterator does not behave in a fused manner. + struct Iter(std::vec::IntoIter>>); + + unsafe impl TrustedLen for Iter {} + + impl Iterator for Iter { + fn size_hint(&self) -> (usize, Option) { + (2, Some(2)) + } + + type Item = Box; + + fn next(&mut self) -> Option { + self.0.next().flatten() + } + } + + let vec = vec![ + Some(Box::new(42)), + Some(Box::new(24)), + None, + Some(Box::new(12)), + ]; + let iter = Iter(vec.into_iter()); + assert_trusted_len(&iter); + assert_eq!( + &[Box::new(42), Box::new(24)], + &*iter.collect::>() + ); +} From 85def307fc83f8c0d164b1506bb855dfaed5f8b5 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 21 Jun 2019 23:01:48 +0200 Subject: [PATCH 13/13] shared_from_iter: Polish internal docs. --- src/liballoc/rc.rs | 29 +++++++++++++++-------------- src/liballoc/sync.rs | 32 ++++++++++++++++---------------- src/liballoc/tests/arc.rs | 2 ++ 3 files changed, 33 insertions(+), 30 deletions(-) diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index 9b653fe2d75a2..94cc0b18133ee 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -699,11 +699,11 @@ impl Rc { } impl Rc { - // Allocates an `RcBox` with sufficient space for - // an unsized value where the value has the layout provided. - // - // The function `mem_to_rcbox` is called with the data pointer - // and must return back a (potentially fat)-pointer for the `RcBox`. + /// Allocates an `RcBox` with sufficient space for + /// an unsized value where the value has the layout provided. + /// + /// The function `mem_to_rcbox` is called with the data pointer + /// and must return back a (potentially fat)-pointer for the `RcBox`. unsafe fn allocate_for_unsized( value_layout: Layout, mem_to_rcbox: impl FnOnce(*mut u8) -> *mut RcBox @@ -730,7 +730,7 @@ impl Rc { inner } - // Allocates an `RcBox` with sufficient space for an unsized value + /// Allocates an `RcBox` with sufficient space for an unsized value unsafe fn allocate_for_ptr(ptr: *const T) -> *mut RcBox { // Allocate for the `RcBox` using the given value. Self::allocate_for_unsized( @@ -762,7 +762,7 @@ impl Rc { } impl Rc<[T]> { - // Allocates an `RcBox<[T]>` with the given length. + /// Allocates an `RcBox<[T]>` with the given length. unsafe fn allocate_for_slice(len: usize) -> *mut RcBox<[T]> { Self::allocate_for_unsized( Layout::array::(len).unwrap(), @@ -771,19 +771,19 @@ impl Rc<[T]> { } } -// Sets the data pointer of a `?Sized` raw pointer. -// -// For a slice/trait object, this sets the `data` field and leaves the rest -// unchanged. For a sized raw pointer, this simply sets the pointer. +/// Sets the data pointer of a `?Sized` raw pointer. +/// +/// For a slice/trait object, this sets the `data` field and leaves the rest +/// unchanged. For a sized raw pointer, this simply sets the pointer. unsafe fn set_data_ptr(mut ptr: *mut T, data: *mut U) -> *mut T { ptr::write(&mut ptr as *mut _ as *mut *mut u8, data as *mut u8); ptr } impl Rc<[T]> { - // Copy elements from slice into newly allocated Rc<[T]> - // - // Unsafe because the caller must either take ownership or bind `T: Copy` + /// Copy elements from slice into newly allocated Rc<[T]> + /// + /// Unsafe because the caller must either take ownership or bind `T: Copy` unsafe fn copy_from_slice(v: &[T]) -> Rc<[T]> { let ptr = Self::allocate_for_slice(v.len()); @@ -847,6 +847,7 @@ impl Rc<[T]> { } } +/// Specialization trait used for `From<&[T]>`. trait RcFromSlice { fn from_slice(slice: &[T]) -> Self; } diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index 672481ca0defd..0a9ce43797853 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -588,11 +588,11 @@ impl Arc { } impl Arc { - // Allocates an `ArcInner` with sufficient space for - // an unsized value where the value has the layout provided. - // - // The function `mem_to_arcinner` is called with the data pointer - // and must return back a (potentially fat)-pointer for the `ArcInner`. + /// Allocates an `ArcInner` with sufficient space for + /// an unsized value where the value has the layout provided. + /// + /// The function `mem_to_arcinner` is called with the data pointer + /// and must return back a (potentially fat)-pointer for the `ArcInner`. unsafe fn allocate_for_unsized( value_layout: Layout, mem_to_arcinner: impl FnOnce(*mut u8) -> *mut ArcInner @@ -618,7 +618,7 @@ impl Arc { inner } - // Allocates an `ArcInner` with sufficient space for an unsized value + /// Allocates an `ArcInner` with sufficient space for an unsized value. unsafe fn allocate_for_ptr(ptr: *const T) -> *mut ArcInner { // Allocate for the `ArcInner` using the given value. Self::allocate_for_unsized( @@ -650,7 +650,7 @@ impl Arc { } impl Arc<[T]> { - // Allocates an `ArcInner<[T]>` with the given length. + /// Allocates an `ArcInner<[T]>` with the given length. unsafe fn allocate_for_slice(len: usize) -> *mut ArcInner<[T]> { Self::allocate_for_unsized( Layout::array::(len).unwrap(), @@ -659,19 +659,19 @@ impl Arc<[T]> { } } -// Sets the data pointer of a `?Sized` raw pointer. -// -// For a slice/trait object, this sets the `data` field and leaves the rest -// unchanged. For a sized raw pointer, this simply sets the pointer. +/// Sets the data pointer of a `?Sized` raw pointer. +/// +/// For a slice/trait object, this sets the `data` field and leaves the rest +/// unchanged. For a sized raw pointer, this simply sets the pointer. unsafe fn set_data_ptr(mut ptr: *mut T, data: *mut U) -> *mut T { ptr::write(&mut ptr as *mut _ as *mut *mut u8, data as *mut u8); ptr } impl Arc<[T]> { - // Copy elements from slice into newly allocated Arc<[T]> - // - // Unsafe because the caller must either take ownership or bind `T: Copy` + /// Copy elements from slice into newly allocated Arc<[T]> + /// + /// Unsafe because the caller must either take ownership or bind `T: Copy`. unsafe fn copy_from_slice(v: &[T]) -> Arc<[T]> { let ptr = Self::allocate_for_slice(v.len()); @@ -735,7 +735,7 @@ impl Arc<[T]> { } } -// Specialization trait used for From<&[T]> +/// Specialization trait used for `From<&[T]>`. trait ArcFromSlice { fn from_slice(slice: &[T]) -> Self; } @@ -1903,7 +1903,7 @@ impl> ArcFromIter for Arc<[T]> { impl<'a, T: 'a + Clone> ArcFromIter<&'a T, slice::Iter<'a, T>> for Arc<[T]> { fn from_iter(iter: slice::Iter<'a, T>) -> Self { - // Delegate to `impl From<&[T]> for Rc<[T]>`. + // Delegate to `impl From<&[T]> for Arc<[T]>`. // // In the case that `T: Copy`, we get to use `ptr::copy_nonoverlapping` // which is even more performant. diff --git a/src/liballoc/tests/arc.rs b/src/liballoc/tests/arc.rs index ce64e2de01377..cf2ad2a8e6033 100644 --- a/src/liballoc/tests/arc.rs +++ b/src/liballoc/tests/arc.rs @@ -88,6 +88,8 @@ fn eq() { assert_eq!(*x.0.borrow(), 0); } +// The test code below is identical to that in `rc.rs`. +// For better maintainability we therefore define this type alias. type Rc = Arc; const SHARED_ITER_MAX: u16 = 100;