From 41730b7e2e2c28a13fe6d08a7ad47d31d68eccea Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Wed, 27 Jun 2018 18:09:21 +0200 Subject: [PATCH] Rc: remove unused allocation from Weak::new() Same as https://github.com/rust-lang/rust/pull/50357 --- src/liballoc/rc.rs | 59 +++++++++++++++++++++++++++----------------- src/liballoc/sync.rs | 2 +- 2 files changed, 37 insertions(+), 24 deletions(-) diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index 32d624e8fbc79..3f32abe1ea959 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -253,7 +253,7 @@ use core::hash::{Hash, Hasher}; use core::intrinsics::abort; use core::marker; use core::marker::{Unsize, PhantomData}; -use core::mem::{self, align_of_val, forget, size_of_val, uninitialized}; +use core::mem::{self, align_of_val, forget, size_of_val}; use core::ops::Deref; use core::ops::CoerceUnsized; use core::ptr::{self, NonNull}; @@ -261,6 +261,7 @@ use core::convert::From; use alloc::{Global, Alloc, Layout, box_free, handle_alloc_error}; use string::String; +use sync::is_dangling; use vec::Vec; struct RcBox { @@ -1153,6 +1154,10 @@ impl From> for Rc<[T]> { /// [`None`]: ../../std/option/enum.Option.html#variant.None #[stable(feature = "rc_weak", since = "1.4.0")] pub struct Weak { + // This is a `NonNull` to allow optimizing the size of this type in enums, + // but it is not necessarily a valid pointer. + // `Weak::new` sets this to a dangling pointer so that it doesn’t need + // to allocate space on the heap. ptr: NonNull>, } @@ -1165,8 +1170,8 @@ impl !marker::Sync for Weak {} impl, U: ?Sized> CoerceUnsized> for Weak {} impl Weak { - /// Constructs a new `Weak`, allocating memory for `T` without initializing - /// it. Calling [`upgrade`] on the return value always gives [`None`]. + /// Constructs a new `Weak`, without allocating any memory. + /// Calling [`upgrade`] on the return value always gives [`None`]. /// /// [`upgrade`]: struct.Weak.html#method.upgrade /// [`None`]: ../../std/option/enum.Option.html @@ -1181,14 +1186,8 @@ impl Weak { /// ``` #[stable(feature = "downgraded_weak", since = "1.10.0")] pub fn new() -> Weak { - unsafe { - Weak { - ptr: Box::into_raw_non_null(box RcBox { - strong: Cell::new(0), - weak: Cell::new(1), - value: uninitialized(), - }), - } + Weak { + ptr: NonNull::dangling(), } } } @@ -1222,13 +1221,25 @@ impl Weak { /// ``` #[stable(feature = "rc_weak", since = "1.4.0")] pub fn upgrade(&self) -> Option> { - if self.strong() == 0 { + let inner = self.inner()?; + if inner.strong() == 0 { None } else { - self.inc_strong(); + inner.inc_strong(); Some(Rc { ptr: self.ptr, phantom: PhantomData }) } } + + /// Return `None` when the pointer is dangling and there is no allocated `RcBox`, + /// i.e. this `Weak` was created by `Weak::new` + #[inline] + fn inner(&self) -> Option<&RcBox> { + if is_dangling(self.ptr) { + None + } else { + Some(unsafe { self.ptr.as_ref() }) + } + } } #[stable(feature = "rc_weak", since = "1.4.0")] @@ -1258,12 +1269,14 @@ impl Drop for Weak { /// assert!(other_weak_foo.upgrade().is_none()); /// ``` fn drop(&mut self) { - unsafe { - self.dec_weak(); + if let Some(inner) = self.inner() { + inner.dec_weak(); // the weak count starts at 1, and will only go to zero if all // the strong pointers have disappeared. - if self.weak() == 0 { - Global.dealloc(self.ptr.cast(), Layout::for_value(self.ptr.as_ref())); + if inner.weak() == 0 { + unsafe { + Global.dealloc(self.ptr.cast(), Layout::for_value(self.ptr.as_ref())); + } } } } @@ -1284,7 +1297,9 @@ impl Clone for Weak { /// ``` #[inline] fn clone(&self) -> Weak { - self.inc_weak(); + if let Some(inner) = self.inner() { + inner.inc_weak() + } Weak { ptr: self.ptr } } } @@ -1317,7 +1332,7 @@ impl Default for Weak { } } -// NOTE: We checked_add here to deal with mem::forget safety. In particular +// NOTE: We checked_add here to deal with mem::forget safely. In particular // if you mem::forget Rcs (or Weaks), the ref-count can overflow, and then // you can free the allocation while outstanding Rcs (or Weaks) exist. // We abort because this is such a degenerate scenario that we don't care about @@ -1370,12 +1385,10 @@ impl RcBoxPtr for Rc { } } -impl RcBoxPtr for Weak { +impl RcBoxPtr for RcBox { #[inline(always)] fn inner(&self) -> &RcBox { - unsafe { - self.ptr.as_ref() - } + self } } diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index abc0befeb947b..ea8616bf1d05c 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -1038,7 +1038,7 @@ impl Weak { } } -fn is_dangling(ptr: NonNull) -> bool { +pub(crate) fn is_dangling(ptr: NonNull) -> bool { let address = ptr.as_ptr() as *mut () as usize; let align = align_of_val(unsafe { ptr.as_ref() }); address == align