alloc: stabilise Allocator

[nia-e]

View all comments

Allocator stabilisation report

Reference PR:

• Document Box & allocator interactions reference#2364

This is the stabilisation report for a subset of the feature allocator_api, with tracking issue #32838 under the purview of wg-allocators, initially proposeed by RFC #1398. The remainder of the feature will be renamed to allocator_ext.

This was a collaborative effort of t-libs, wg-allocators, members of t-types, t-lang, and t-opsem, alongside interested parties in the ecosystem and contributors to the initial attempt at stabilisation on GitHub.

See also the new wg-allocators roadmap on the matter.

Summary

The following is a proposal following several conversations, in-person and online, with libs team members and interested ecosystem participants and represents an attempt at stabilising an MVP for the Allocator trait and its implementation safety requirements, alongside minimal functionality to make its use possible in the standard library.

While an effort was made to align with the stated positions of the team, the opinions and rationale stated are the author's own, and should not be seen as representative of the libs(-api) team as a whole except insofar as individual members therein choose to endorse the contents of report. Any mention of "we", "us", etc. should be understood to refer to the author alongside those who have explicitly expressed agreement.

API & considerations

The stabilised API surface consists of:

unsafe trait Allocator {
    // Required methods
    fn allocate(&self, layout: Layout) -> Result<NonNull<[u8]>, AllocError>;
    unsafe fn deallocate(&self, ptr: NonNull<u8>, layout: Layout);

    // Provided methods
    fn allocate_zeroed(
        &self,
        layout: Layout,
    ) -> Result<NonNull<[u8]>, AllocError> { ... }
    unsafe fn grow(
        &self,
        ptr: NonNull<u8>,
        old_layout: Layout,
        new_layout: Layout,
    ) -> Result<NonNull<[u8]>, AllocError> { ... }
    unsafe fn grow_zeroed(
        &self,
        ptr: NonNull<u8>,
        old_layout: Layout,
        new_layout: Layout,
    ) -> Result<NonNull<[u8]>, AllocError> { ... }
    unsafe fn shrink(
        &self,
        ptr: NonNull<u8>,
        old_layout: Layout,
        new_layout: Layout,
    ) -> Result<NonNull<[u8]>, AllocError> { ... }
}

// N.B.: This particular point is lang-relevant since `Box`
// would be stabilised without being fundamental over `A`.
struct Box<T, #[stable(...)] A: Allocator>(...)

impl<T, A: Allocator> Box<T, A> {
    fn new_in(x: T, alloc: A) -> Box<T, A>;
}

impl<T: ?Sized, A: Allocator> Box<T, A> {
    unsafe fn from_raw_in(raw: *mut T, alloc: A) -> Self;
    unsafe fn from_non_null_in(raw: NonNull<T>, alloc: A) -> Self;
    fn into_raw_with_allocator(b: Self) -> (*mut T, A);
    fn into_non_null_with_allocator(b: Self) -> (NonNull<T>, A);
    fn allocator(b: &Self) -> &A;
}

struct Vec<T, #[stable(...)] A: Allocator> { ... }

impl<T, A: Allocator> Vec<T, A> {
    fn new_in(alloc: A) -> Vec<T, A>;
    fn with_capacity_in(capacity: usize, alloc: A) -> Self;
    unsafe fn from_raw_parts_in(
        ptr: *mut T,
        length: usize,
        capacity: usize,
        alloc: A,
    ) -> Self;
    unsafe fn from_parts_in(
        ptr: NonNull<T>,
        length: usize,
        capacity: usize,
        alloc: A,
    ) -> Self;
    fn into_raw_parts_with_allocator(self) -> (*mut T, usize, usize, A);
    fn into_parts_with_allocator(self) -> (NonNull<T>, usize, usize, A);
    fn allocator(&self) -> &A;
}

struct Global; // implementor of Allocator
struct System; // implementor of Allocator

unsafe impl<A: Allocator + ?Sized> Allocator for &A { ... }
unsafe impl<A: Allocator + ?Sized> Allocator for &mut A { ... }
unsafe impl<A: Allocator + ?Sized> Allocator for Box<A, _> { ... }
unsafe impl<A: Allocator + ?Sized> Allocator for Rc<A, _> { ... }
unsafe impl<A: Allocator + ?Sized> Allocator for Arc<A, _> { ... }

The by_ref method on Allocator was removed, as it was only a postfix syntax convenience (equivalent to writing (&alloc)).

The safety requirements on implementors of Allocator were tightened to the most restrictive sound form we expect to possibly want, in order to enable us to iterate on the design in the future and relax these bounds if it is deemed possible. Notable changes from the form assumed before the stabilisation effort started:

• the implicit requirement that the standard library's Clone for Arc<T, A> implementation relied on but was improperly documented, for implementors not to invalidate allocated memory on drop or mutable access was made explicit;
• implementors are now required not unwind from any of the methods on the trait or from drop;
• the safety invariants implementors of Allocator + Clone must uphold were moved to their own unstable marker subtrait as the requirement was deemed unjustifiable.

Additionally, it was decided that Allocator will be dyn-compatible, as the resulting constraints on the design were deemed acceptable given the significant increased flexibility for users.

Soundness developments

The process of attempting stabilisation resulted in several soundness issues arising, especially with regard to the interaction between custom allocators and Boxes. Thus, some points had to be adjusted:

• there existed a requirement for trait implementors to obey certain semantics if an implementor of Allocator is also Clone, which constituted possible UB if broken. These have been dropped, as unsafe implementors cannot guard against possible unsoundness from incorrect implementations in downstream safe code, but the equivalent functionality may be added backwards-compatibly with an unsafe marker trait or language mechanism;
• Box::into_pin will not yet be possible with custom allocators. This is because of a soundness bug relating to an interaction between the possibility of manually implementing Clone for Box<T, A> and Box being covariant over A, allowing for a pinned box to be cloned with a non-'static allocator from one with a correct 'static allocator subtyped to a non-static one. Making Box invariant over the allocator was considered, but was deemed far too limiting and would technically be a breaking change to reverse later. Thus, for now, an unstable and unsafe marker trait StaticAllocator will be introduced to mark an allocator as guaranteeing that its allocations live for 'static (i.e. will never be lost unless explicitly de/reallocated). This will be implemented for the Global and System allocators;
  • due to Pin's preexisting implementation of a safe Pin::new for any pointer type where <Ptr as Deref>::Target: Unpin, it will be stably possible to call Pin::new() on a box with a custom allocator as changing this would require significant special-casing in trait resolution. Experiments in this direction surfaced a soundness bug, addressed by tightening the requirements of impl PinSafePointer for Box to necessitate a pin-safe StaticAllocator;
  • further discussion revealed that these same semantics are necessary for integrating custom allocators into LLVM's proposed semantics for allocator intrinsics, as below;
• a preexisting hack whereby Box had noalias semantics for its pointer if and only if the allocator is Global - alongside a similar hack to make "unleaking" work - is to be moved to an unstable wrapper type NativeAllocator<A: StaticAllocator>, which enables us to make use of LLVM's new allocator intrinsics. Global would then be equivalent to NativeAllocator<A> with the concrete allocator substituted in. Per a conversation with members of opsem, this appeared to be a reasonable way forward;
• unwinding out of many allocation-related methods was found to be a pervasive source of unsoundness. Thus, language on allocator cloning and allocation methods was expanded so as to ensure unwinds never come out of methods on an Allocator, clones, or drops.

Backwards-compatible changes

Several designs were considered to extend or modify the trait's semantics. We have opted to defer full consideration of many of these for later, as we have determined they can be added backwards-compatibly to the existing API. A list of these is present below, alongside rationale for their postponement.

Store API

This is an alternative, more complex proposal for custom allocators (see the draft RFC). Per a conversation in-person with one of the authors of the Store proposal, we have established that it could be added backwards-compatibly (in Store terminology, the stabilised Allocator trait is effectively a storage with pointer handles). The details were thus deferred for potential post-stabilisation changes.

Split Deallocator trait

Supertrait item shadowing alongside a blanket impl<A: Allocator + ?Sized> Deallocator for A will allow us to add a Deallocator supertrait backwards-compatibly, and to relax the requirements for collection types to insted hold a Deallocator. Conversations with those involved in the above issue suggest it is likely for a PR implementing this to be merged in the near future.

fn reallocate()

The current design uses dedicated grow, grow_zeroed, and shrink methods instead of a way to reallocate between arbitrary sizes. However, such a function could be added with a defaulted body in the future, forwarding to the extant grow/shrink implementations.

Conditional reentrancy in std

Not all allocators will be reentrant in std, and thus the standard library may want to be able to conditionally call the global allocator in areas it has otherwise promised not to. Thus, the proposed unstable GlobalAllocator: Allocator marker trait could be extended with a defaulted associated constant REENTRANT_IN_STD: bool = true wherein implementors could promise that a certain allocator never calls any part of std.

Possible but less clean additions

Several options appeared to signal compelling usecases, but were sufficiently niche that we did not consider them to be blocking for an MVP stabilisation so long as it was realistically possible to express their semantics.

Associated constants

Several usecases would be facilitated by having certain associated items on the Allocator trait; notably, const MIN_ALIGN: usize for the minimum alignment an allocator is always guaranteed to return. Adding the semantics of these backwards-compatibly would rely on maybe trait bounds being stabilised, which per conversations with the lang & types teams we believe is feasible in the near future. Alternatively, much of the same functionality could be added with defaulted const methods, which are also on the stabilisation path.

grow_in_place()

There is currently no obvious way to signal through the API whether a move of the data is acceptable when reallocating memory. Though messy, a way to express these semantics with the current design does exist, even if non-obvious:

struct A;
// `grow`/`grow_zeroed` have non-in-place semantics
unsafe impl Allocator for A { /* ... */ }

struct B(A);
// in-place-grow semantics
unsafe impl Allocator for B { /* ... */ }

impl A {
    fn as_pinning(self) -> B { B(self) }
}

impl B {
    fn as_nonpinning(self) -> A { self.0 }
}

We have decided that this is acceptable, given that it is "only" a point of design and not underlying functionality. A cleaner way to signal such semantics would be of interest for future extensions to the trait.

Notably, in-place growing and/or shrinking without invalidating preexisting pointers (i.e. actually changing the size of the allocation in the abstract machine) needs proper support from LLVM which may not happen in the near future.

Allocation flags

A similar transmute-based mechanism as for the above can be used to reference a local inside of the allocator, though this could be UB-prone. Alternatively, and much more nicely, argument splatting could allow us to backwards-compatibly extend the trait (assuming implementors as well as callers may ignore optional fields). However, this would depend on the details of such a proposal.

The main stakeholder who approached us with concerns on this topic - Rust for Linux - signalled willingness to maintain a downstream extension trait for such functionality for the time being.

Rejected alternative proposals

The following changes were explicitly not made to the API pre-stabilisation, despite it being unlikely that their semantics could be nicely expressed in the (near) future. In all cases, notable arguments existed to make the requested change, but we decided they were not sufficiently compelling. Should a way to express these semantics emerge in the future backwards-compatibly, we would be open to re-reviewing them.

NonZeroLayout arguments

An idea had been proposed to change the signature of the allocating/freeing methods to take a Layout that is guaranteed to have a nonzero size.

We determined that API cleanliness and potential simplification of library code (once const Traits are stable, collection types could drop special-case logic when using a const Allocator at zero capacity) outweigh the arguments for not allowing zero-sized allocations. As we see it, in the cases where it would genuinely be problematic, this will only move the branch on zero-sized allocation to the other side of the call. At worst, it would put marginally more pressure on a branch predictor.

Though the possibility of zero-size allocations being probematic is often mentioned, we have not seen sufficiently convincing concrete cases where this is the case. One pointed-to example was that of highly performance-sensitive allocators (e.g. bump allocators); however, it appears most of these cases can trivially support zero-size allocations (e.g. bumping by zero). Consequently, we have decided to keep the nicer logic for downstream users of the trait. An argument had also been made around jemalloc being unable to correctly handle zero-sized allocations, but this appears to only apply to internal APIs.

A similar idea wherein allocate was an unsafe method and support for zero-sized allocations was implementation-defined was rejected on similar usability grounds. Several members of the libs team expressed their opinion that the design of GlobalAlloc (featuring a similar unsafe allocating method wherein the caller must guarantee the size is nonzero) was not desirable in hindsight.

NonNull<u8> return type

Lacking a better way to signal returned vs. requested capacity, and not wishing to duplicate all of the allocating methods, we have decided that we would prefer to keep the wide-pointer return value and potentially use that logic to determine capacity in returned allocations. This will never be an issue with regard to performance, as no architecture allocates expects fewer than two registers to be clobbered by a function call, and so there is no cost to returning the wide pointer.

Some callers will elect to ignore extra capacity; similarly, some implementors will elect not to offer it. The language around implementation safety ensures that these cases are supported, and recommends that implementors not signal extra capacity if it would be expensive to do so. That is to say, both the caller and implementor must cooperate for the excess to be meaningfully usable; otherwise there is no performance impact in a correct implementation.

Associated types

Having an associated type, especially for the returned error on allocation failure, had been mentioned as a possible addition; however, doing so would add significant complexity to the trait while also making dyn-compatibility impossible. Few concrete usecases came up where the allocator itself has meaningful error information that would be actionable to callers, and therefore it was elected to keep the current ZST AllocError.

Future work

A large part of the standard library will need review as we determine what the correct way is for various collection and pointer types to work with custom allocators.

Notably, there are multiple outstanding proposals for integrating fallible allocation APIs into the standard library, and a stable mechanism needs to be decided on for exposing the Allocator + Clone interaction.

Outlined potential extensions

The following is a possible future outline of what the Allocator trait and related might look like under this proposal, assuming both of supertrait item shadowing and defaulted associated items being added:

unsafe trait Allocator: Deallocator {    
    fn allocate(
        &self,
        layout: Layout,
    ) -> Result<NonNull<[u8]>, AllocError>;
    unsafe fn deallocate(
        &self,
        ptr: NonNull<u8>,
        layout: Layout,
    );

    // Provided methods
    unsafe fn reallocate(
        &self,
        ptr: NonNull<u8>,
        old_layout: Layout,
        new_layout: Layout,
    ) -> Result<NonNull<[u8]>, AllocError> { ... }
    
    /// Minimum alignment that will always be returned, regardless
    /// of what alignment is requested.
    const fn min_align(&self) -> usize {
        1
    }
}

unsafe trait Deallocator {
    unsafe fn deallocate(&self, ptr: NonNull<u8>, layout: Layout);
}

// Enabled by supertrait item shadowing.
impl<A: Allocator> Deallocator for A {
    unsafe fn deallocate(&self, ptr: NonNull<u8>, layout: Layout) {
        <Self as Allocator>::deallocate(self, ptr, layout)
    }
}

/// The allocator is suitable for use as the global allocator.
///
/// # Safety
///
/// `reentrant_in_std` must never be `false` incorrectly, and
/// if `true`, the allocator may only use those parts of  `std`
/// which explicitly allow themselves to be called from the global
/// allocator (such as thread-locals).
unsafe trait GlobalAllocator: Allocator + Sync + 'static {
    const REENTRANT_IN_STD: bool;
}

/// `Clone` will create an equivalent (de)allocator (i.e. both
/// can deallocate the same memory), and `Copy` either is
/// the same as clone (i.e. `Clone` is a memcpy) or impossible
/// to implement.
unsafe trait AllocatorClone: Deallocator + Clone {}

/// Polls allocator equivalence.
unsafe trait AllocatorEq<Other: AllocatorEq = Self>: Deallocator {
    /// If `other` can free something, so can `self`.
    /// Implementors must never incorrectly return `true`,
    /// and equality must be transitive and reflexive.
    fn is_equivalent(&self, other: &Other) -> bool;
}

/// The allocator in question will not break `Pin` guarantees
/// even if subtyped with a shorter lifetime; that is, memory
/// is never deallocated except via an explicit call to `deallocate`
/// (and not via dropping the allocator, etc.).
unsafe trait StaticAllocator: Allocator {}

impl<T, A, D> Box<T, D>
where
    A: Allocator + AllocatorEq<D>,
    D: AllocatorEq<A>,
{
    // bikeshed better names
    fn new_in_with(x: T, alloc: A, dealloc: D) -> Self {
        if dealloc.is_equivalent(&alloc) {
            unsafe { Box::new_in_with_unchecked(...) }
        }
    }

    fn with_dealloc(boxed: Box<T, A>, dealloc: D) -> Self { ... }
}

impl<T, A: StaticAllocator> Box<T, A> {
    fn into_pin(boxed: Box<T, A>) -> Pin<Box<T, A>> { ... }
}

/// Calls to this allocator are not considered part of program
/// behaviour, and thus may be elided or created by the optimiser.
/// Additionally, such an allocator will never return an excess
/// and makes no promises about alignment beyond what is requested.
#[lang = "native_allocator"]
struct NativeAllocator<A: StaticAllocator>(A);

impl<A: StaticAllocator> Allocator for NativeAllocator<A> { ... }
impl<A: StaticAllocator> StaticAllocator for NativeAllocator<A> {}

cc @rust-lang/libs @rust-lang/libs-api @rust-lang/opsem

r? libs

[nia-e]

r? @Amanieu

[theemathas]

I believe the safety requirements are not yet correct. See #156544

[bstrie]

Do we have the chance to change them in the next edition?

APIs can be "renamed" without using an edition, it involves making a new API with the desired name and deprecating the old one, e.g. slice::connect became slice::join: https://doc.rust-lang.org/std/primitive.slice.html#method.connect (In theory you could use an edition to not only warn on deprecated APIs but deny access to the deprecated API altogether, if you really wanted to.)

I'm sympathetic both to the idea that it would feel somewhat silly to deprecate an API shortly after stabilizing it (like with the aforementioned slice::connect, renamed in 1.3), but also to the idea that bikeshedding has historically threatened to be endlessly interminable, which denies users the ability to make use of the APIs for relatively frivolous reasons. Are the APIs in question expected to be used frequently? Because an infrequently-used API can probably tolerate a relatively verbose name.

[clarfonthey]

Also worth noting that if you're using into_non_null_with_allocator in more than a couple places in your program, you're probably not doing something right. These method names are long because they are unlikely to show up often, and so, should be as descriptive as possible.

The idea about having space to change this was not really about renaming them (although we could deprecate + add aliases), but rather about the idea of having a more generic API that doesn't require composing method names together. Such a generic API would need its own ACP and would ultimately just be a simpler form of the methods, so, in libs we generally prefer just having individual methods to trying to come up with a trait that covers every use case: people can provide their own traits if they think they're useful, and only the methods are strictly required.

(Also, as an aside, I am looking to update our documentation on stabilisations to explain stuff like this a bit better, since we historically haven't been good about maintaining that documentation.)

[maxdexh]

From the code I've written in std with these functions: moving stuff out of a collection/smart ptr into another one and then using from_*_in on a reference to the allocator is a pretty common pattern.
However, for the into_*_with_allocator functions, even if the names were shorter, that wouldn't really help, as the only thing you can realistically do with the tuple they return is deconstruct it using let, which requires its own statement.
I also didn't mind that the long names encouraged me to use more let bindings, which is probably a good idea anyway. There is too much code that nests from_raw(some_operation(into_raw())).

[clarfonthey]

Also explicitly stating that I trust Nia to mention if anything weird comes up during rebase and you can r=me once FCP passes.

[tgross35]

Could you also paste the stabilization report into the PR description before merge so it gets committed?

View changes since this review

[tgross35]

Should this get a fresh tracking issue since that one is pretty old and cluttered?

(rename+alias could optionally merge now separately, to trim this PR to the FCP-relevant bits)

[nia-e]

yeah splitting up the issues should probably be its own PR but I'm happy to do it. i think keeping the stuff in this first stabilisation as allocator_api makes sense (at least symbolically)

[rust-rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

[nia-e]

in the midst of it being so over, i found there was within me an insurmountable "we're so back"

[bstrie]

Now that the FCP has ended, I assume the last remaining task is to rebase this PR? I'm not suggesting that there's any cause to rush, though I will note that September 25 is the branch date for Rust 1.100, if there was any desire to have allocators stable for Rust's auspicious triple-digit release. :P

[tgross35]

though I will note that September 25 is the branch date for Rust 1.100, if there was any desire to have allocators stable for Rust's auspicious triple-digit release. :P

I know this wasn't fully serious but I'd rather have it at the start of a release cycle so, if needed, we can adjust or add to / subtract from the newly stable API without needing riskier backports. 101 is still a cool number! The release blog post could allocate dalmatians.

[clarfonthey]

I mean, I do recall Nia specifically being rather invested in getting this out in 1.100 but she's technically been on vacation for the past two weeks. We've poured over the semantics of allocators for the past several weeks and it does seem like we would be fine if we merged this right before the beta branch. And besides, if there are regressions, we do have an entire release cycle to backport fixes, or backport the stabilisation if it's really that bad.

[nia-e]

i had been knocked out cold by rustconf but i'm back ^^ i'll rebase this asap & separate out the relevant issues so this can be merged

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[nia-e]

rebase is done, we have a new issue for allocator_ext, fcp passed... and now i can sleep.

this will conflict with a lot of stuff, but should be fine to rollup if it doesn't touch allocator stuff.

@bors r=clarfonthey

[rust-bors]

📋 This PR cannot be approved because it currently has the following label: needs-reference-pr.

[clarfonthey]

Looks like the main reference ask is for updating fundamental's rules for boxes and noalias only applying to Global.

[clarfonthey]

@bors r+

[rust-bors]

📌 Commit 4eb7bb3 has been approved by clarfonthey

It is now in the queue for this repository.

[tgross35]

Could you also paste the stabilization report into the PR description before merge so it gets committed?

Psst 🙂

[nia-e]

oopers ^^ done, ty!