Skip to content

Commit

Permalink
Auto merge of #64062 - petertodd:2019-new-in-place, r=<try>
Browse files Browse the repository at this point in the history
Attempt to make Box::default() construct in-place + Box::new_in_place()

To start with, I'll say straight up this is more of a sketch of a possible API then something I'm sure we should merge.

The first part here is the `Box::new_in_place()` API. Basically the idea is that we could provide a way to reliably create large boxed values in-place via return-value-optimization. This is something that often is *never* possible to optimize, even in theory, because allocation is fallible and if the creation of the value is itself fallible, the optimizer can't move the allocation to happen first because that would be an observable change.

Unfortunately this doesn't quite work if the closure can't be inlined sufficiently due to optimizer limitations. But hopefully this at least shows a way forward that might be interesting to others.

Secondly, I use this `Box::new_in_place()` API to optimize `Box::default()`. I'm not totally sure this is a good idea to actually merge, as currently this is kind of an observable change in behavior for things like large arrays that would otherwise blow up the stack; people might rely on it, and currently the optimization is unreliable.

Finally, this of course this could be extended to `Rc` and `Arc` as well in much the same fashion.
  • Loading branch information
bors committed Sep 5, 2019
2 parents f257c40 + 9b57963 commit a683cc0
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 3 deletions.
39 changes: 38 additions & 1 deletion src/liballoc/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,43 @@ impl<T> Box<T> {
Box(ptr.cast().into())
}

/// Allocates memory on the heap *then* constructs `T` in-place.
///
/// If the allocation fails, the initialization closure won't be called; because the allocation
/// can fail, the optimizer often couldn't construct `T` in-place even in theory because having
/// the allocation happen first is an observable change if the code that creates the value has
/// side-effects such as being able to panic.
///
/// FIXME: This is intended to work via return-value-optimization, but due to compiler
/// limitations can't reliably do that yet.
#[inline(always)]
fn new_in_place(f: impl FnOnce() -> T) -> Box<T> {
let mut r: Box<mem::MaybeUninit<T>> = Box::new_uninit();
let uninit: &mut mem::MaybeUninit<T> = &mut *r;

unsafe {
// So why aren't we using ptr::write() here?
//
// For return-value-optimization to work, the compiler would have to call f with the
// pointer as the return value. But a pointer isn't guaranteed to actually point to
// valid memory: if the pointer was invalid, eg. due to being null, eliding the copy
// would change where the invalid memory access would occur, changing the behavior in
// an observable way.
//
// An invalid reference OTOH is undefined behavior, so the compiler is free to assume
// it is valid.
//
// Unfortunately, this optimization isn't actually implemented yet. Though if
// enough of f() can be inlined the compiler can generally elide the copy anyway.
//
// Finally, this is leak free because MaybeUninit::new() can't panic: either f()
// succesfully creates the value, and assume_init() is called, or f() panics, frees its
// resources, and r is deallocated as a Box<MaybeUninit<T>>
*uninit = mem::MaybeUninit::new(f());
r.assume_init()
}
}

/// Constructs a new `Pin<Box<T>>`. If `T` does not implement `Unpin`, then
/// `x` will be pinned in memory and unable to be moved.
#[stable(feature = "pin", since = "1.33.0")]
Expand Down Expand Up @@ -480,7 +517,7 @@ unsafe impl<#[may_dangle] T: ?Sized> Drop for Box<T> {
impl<T: Default> Default for Box<T> {
/// Creates a `Box<T>`, with the `Default` value for T.
fn default() -> Box<T> {
box Default::default()
Box::new_in_place(T::default)
}
}

Expand Down
14 changes: 13 additions & 1 deletion src/liballoc/rc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,18 @@ impl<T> Rc<T> {
}
}

#[inline(always)]
fn new_in_place(f: impl FnOnce() -> T) -> Rc<T> {
let mut r: Rc<mem::MaybeUninit<T>> = Rc::new_uninit();

unsafe {
let uninit: &mut mem::MaybeUninit<T> = Rc::get_mut_unchecked(&mut r);

*uninit = mem::MaybeUninit::new(f());
r.assume_init()
}
}

/// Constructs a new `Pin<Rc<T>>`. If `T` does not implement `Unpin`, then
/// `value` will be pinned in memory and unable to be moved.
#[stable(feature = "pin", since = "1.33.0")]
Expand Down Expand Up @@ -1146,7 +1158,7 @@ impl<T: Default> Default for Rc<T> {
/// ```
#[inline]
fn default() -> Rc<T> {
Rc::new(Default::default())
Rc::new_in_place(T::default)
}
}

Expand Down
14 changes: 13 additions & 1 deletion src/liballoc/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,18 @@ impl<T> Arc<T> {
}
}

#[inline(always)]
fn new_in_place(f: impl FnOnce() -> T) -> Arc<T> {
let mut r: Arc<mem::MaybeUninit<T>> = Arc::new_uninit();

unsafe {
let uninit: &mut mem::MaybeUninit<T> = Arc::get_mut_unchecked(&mut r);

*uninit = mem::MaybeUninit::new(f());
r.assume_init()
}
}

/// Constructs a new `Pin<Arc<T>>`. If `T` does not implement `Unpin`, then
/// `data` will be pinned in memory and unable to be moved.
#[stable(feature = "pin", since = "1.33.0")]
Expand Down Expand Up @@ -1933,7 +1945,7 @@ impl<T: Default> Default for Arc<T> {
/// assert_eq!(*x, 0);
/// ```
fn default() -> Arc<T> {
Arc::new(Default::default())
Arc::new_in_place(T::default)
}
}

Expand Down

0 comments on commit a683cc0

Please sign in to comment.