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.

Sign up for GitHub

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

Prepare global allocators for stabilization #1974

Merged
merged 7 commits into from
Jun 18, 2017
Merged

Prepare global allocators for stabilization #1974

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
df69ec1
Global allocators round 3
sfackler Feb 19, 2017
56d6820
Tweaks from feedback
sfackler Apr 18, 2017
770abea
Use a trait instead of free functions
sfackler May 1, 2017
d80313c
Updates and refinements to various apis and guarantees
alexcrichton May 22, 2017
c405fd9
Merge pull request #2 from alexcrichton/more-allocators
sfackler May 23, 2017
b60f63b
More updates
sfackler Jun 8, 2017
22fe7cb
Typo
sfackler Jun 8, 2017
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
56 changes: 53 additions & 3 deletions text/0000-global-allocators.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ are tagged with attributes:
/// Behavior is undefined if the requested size is 0 or the alignment is not a
/// power of 2. The alignment must be no larger than the largest supported page
/// size on the platform.
///
/// This function is required.
#[allocator(allocate)]
pub fn allocate(size: usize, align: usize) -> *mut u8 {
...
Expand All @@ -92,6 +94,9 @@ pub fn allocate(size: usize, align: usize) -> *mut u8 {
/// Behavior is undefined if the requested size is 0 or the alignment is not a
/// power of 2. The alignment must be no larger than the largest supported page
/// size on the platform.
///
/// This function is optional. If not provided, `allocate` will be called and
/// the resulting buffer will be zerored.
#[allocator(allocate_zeroed)]
pub fn allocate_zeroed(size: usize, align: usize) -> *mut u8 {
...
Expand All @@ -103,6 +108,8 @@ pub fn allocate_zeroed(size: usize, align: usize) -> *mut u8 {
///
/// The `old_size` and `align` parameters are the parameters that were used to
Copy link

@hanna-kruppe hanna-kruppe Apr 17, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@ruuda made a good point in the discussion of the allocator traits: It can be sensible to allocate over-aligned data, but this information is not necessarily carried along until deallocation, so there's a good reason deallocate shouldn't require the same alignment that was used to allocate.

This requirement was supposed to allow optimizations in the allocator, but AFAIK nobody could name a single existing allocator design that can use alignment information for deallocation.

Copy link
Member

Choose a reason for hiding this comment

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

I wrote an allocator for an OS kernel once that would have benefited greatly from alignment info.

Copy link

Choose a reason for hiding this comment

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

That would be very relevant to both this RFC and the allocators design, so could you write up some details?

Copy link
Member

Choose a reason for hiding this comment

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

Hmmm... It seems that I was very mistaken... I have to appologize 🤕

Actually, when I went back and looked at the code, I found the exact opposite. The allocator interface actually does pass the alignment to free, and my implementation of free ignores it for exactly the reasons mentioned above (more later). That said, passing alignment into the alloc function is useful (and required for correctness), so I assume that this discussion is mostly about if free should take align or not.

The code is here. It's a bit old and not very well-written since I was learning rust when I wrote it. Here is a simple description of what it does:

Assumptions

  • The kernel is the only entity using this allocator. (The user-mode allocator lives in user-mode).
  • The kernel is only using this allocator through Box, so the parameters size and align are trusted to be correct, since they are generated by the compiler.

Objective

Use as little metadata as possible.

Blocks

  • All blocks are a multiple of the smallest possible block size, which is based on the size of the free-block metadata (16B on a 32-bit machine).
  • All blocks have a minimum alignment which is the same as minimum block size (16B).
  • The allocator keeps a free-list which is simply a singly linked list of blocks.
  • Free blocks are used to store their own metadata.
  • Active blocks have no header/footer. This means that their is no header/footer overhead at all.

alloc

Allocating memory just grabs the first free block with required size and alignment, removes it from the free list, splits it if needed, and returns a pointer to its beginning. The size of the block allocated is a function of the alignment and size.

free

Freeing memory requires very little effort, it turns out. Since we assume that the parameters size and ptr are valid, we simply create block metadata and add to the linked list. If possible, we can merge with free blocks after the block we are freeing.

In fact, the alignment passed into free is ignored here because the ptr should already be aligned. The takeaway seems to be the opposite from what I said above (again, sorry). When I thought about it some more, it makes sense. A ptr inherently conveys some alignment information, so passing this information in as an argument actually seems somewhat redundant.

Copy link

Choose a reason for hiding this comment

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

I'm actually quite relieved to hear that 😄 Yes, allocation and reallocation should have alignment arguments, it's just deallocation that shouldn't use alignment information. It's not quite true that "ptr inherently conveys alignment information", because the pointer might just happen to have more alignment than was requested, but it's true that it's always aligned as requested at allocation time (since it must be the exact pointer returned by allocation, not a pointer into the allocation).

/// create the allocation referenced by `ptr`.
///
/// This function is required.
#[allocator(deallocate)]
pub fn deallocate(ptr: *mut u8, old_size: usize, align: usize) {
...
Expand All @@ -122,6 +129,11 @@ pub fn deallocate(ptr: *mut u8, old_size: usize, align: usize) {
///
/// The `old_size` and `align` parameters are the parameters that were used to
/// create the allocation referenced by `ptr`.
///
/// This function is optional. If not provided, an implementation will be
/// generated which calls `allocate` to obtain a new buffer, copies the old
/// memory contents to the new buffer, and then calls `deallocate` on the old
/// buffer.
#[allocator(reallocate)]
pub fn reallocate(ptr: *mut u8, old_size: usize, size: usize, align: usize) -> *mut u8 {
...
Expand Down Expand Up @@ -155,9 +167,9 @@ The allocator functions must be publicly accessible, but can have any name and
be defined in any module. However, it is recommended to use the names above in
the crate root to minimize confusion.

An allocator must provide all functions with the exception of
`reallocate_inplace`. New functions can be added to the API in the future in a
similar way to `reallocate_inplace`.
Note that new functions can be added to this API in the future as long as they
can have default implementations in a similar manner to other optional
functions.

## Using an allocator

Expand Down Expand Up @@ -240,9 +252,47 @@ The allocator APIs could be simplified to a more "traditional"
malloc/calloc/free API at the cost of an efficiency loss when using allocators
with more powerful APIs.

The global allocator could be an instance of the `Allocator` trait. Since that
trait's methods take `&mut self`, things are a bit complicated however. The
allocator would most likely need to be a `const` type implementing `Allocator`
since it wouldn't be sound to interact with a static. This may cause confusion
Copy link

@hanna-kruppe hanna-kruppe Apr 18, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

This is not true. Unlike static muts, plain statics are perfectly safe to access and can, in fact, maintain state. It's just that all mutation needs to happen via thread safe interior mutability.

With an eye towards the potential confusion described in the following sentence ("a new instance will be created for each use"), a static makes much more sense than a const — the latter is a value that gets copied everywhere, while the former is a unique object with an identity, which seems more appropritate for a global allocator (besides permitting allocator state, as mentioned before).

Copy link
Member

Choose a reason for hiding this comment

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

Especially if you have access to unstable features, static with interior mutability is idiomatic and can be wrapped in a safe abstraction, while static mut is quite worse.

Copy link
Member

Choose a reason for hiding this comment

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

I agree completely. Allocators are inherently stateful since they need to keep track of allocations for correctness. Static + interior mutability is needed.

However, this raises a new question: initializing the global allocator. How does this happen? Is there a special constructor called? Does the constructor have to be a const fn? The RFC doesn't specify this.

Copy link
Member Author

Choose a reason for hiding this comment

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

I may be missing something here, but the issue is that you may not obtain a mutable reference to a static, but every method on Allocator takes &mut self:

struct MyAllocator;

impl MyAllocator {
    fn alloc(&mut self) { }
}

static ALLOCATOR: MyAllocator = MyAllocator;

fn main() {
    ALLOCATOR.alloc();
}
error: cannot borrow immutable static item as mutable
  --> <anon>:10:5
   |
10 |     ALLOCATOR.alloc();
   |     ^^^^^^^^^

error: aborting due to previous error

@mark-i-m There is no constructor.

Copy link
Member

Choose a reason for hiding this comment

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

I may be missing something here, but the issue is that you may not obtain a mutable reference to a static, but every method on Allocator takes &mut self

Ah, I see. So then, does the Allocator trait have to change? Or do we make the allocator unsafe and use static mut? If neither is possible, then we might need to switch back to the attributes approach or write a new trait with the hope of coalescing them some time...

@mark-i-m There is no constructor.

Most allocators need some setup, though. Is the intent to just do something like lazy_static? That would annoy me, but it would work, I guess. Alternately, we could add a method to the interface to do this sort of set up...

Copy link

@hanna-kruppe hanna-kruppe Apr 19, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Oh, yeah, I totally overlooked the &mut self issue 😢 We could side-step this by changing the trait (I'm not a fan of that, for reasons I'll outline separately) or by changing how the allocator is accessed. By the latter I mean, for example, tagging a static X: MyAllocator as the global allocator creates an implicit const X_REF: &MyAllocator = &X; and all allocation calls get routed through that. This feels extremely hacky, though, and brings back the aforementioned identity confusion.

Copy link
Member Author

Choose a reason for hiding this comment

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

switch back to the attributes approach

The RFC has never switched away from the attributes approach. This is an alternative.

Most allocators need some setup, though.

Neither system allocators nor jemalloc need explicit setup steps. If you're in an environment where your allocator needs setup, you can presumably call whatever functions are necessary at the start of execution.

since the allocator itself will therefor not be allowed to maintain any state
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: s/therefor/therefore/

internally since a new instance will be created for each use. In addition, the
`Allocator` trait uses a `Layout` type as a higher level encapsulation of the
requested alignment and size of the allocation. The larger API surface area
will most likely cause this feature to have a significantly longer stabilization
Copy link

@hanna-kruppe hanna-kruppe Apr 18, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not so sure about this any more. At least the piece of the API surface named here (Layout) doesn't seem very likely to delay anything. I don't recall any unresolved questions about it (there's questions about what alignment means for some functions, but that's independent of whether it's wrapped in a Layout type).

Copy link
Member Author

Choose a reason for hiding this comment

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

The allocators RFC hasn't even been implemented yet. We have literally zero experience using the Allocator trait or the Layout type. In contrast, alloc::heap and the basic structure of global allocators have been implemented and used for the last couple of years.

period.

# Unresolved questions
[unresolved]: #unresolved-questions

It is currently forbidden to pass a null pointer to `deallocate`, though this is
guaranteed to be a noop with libc's `free` at least. Some kinds of patterns in C
are cleaner when null pointers can be `free`d - is the same true for Rust?

The `Allocator` trait defines several methods that do not have corresponding
implementations here:

* `oom`, which is called after a failed allocation to provide any allocator
specific messaging that may exist.
* `usable_size`, which is mentioned above as being unused, and should probably
be removed from this trait as well.
* `alloc_excess`, which is like `alloc` but returns the entire usable size
including any extra space beyond the requested size.
* Some other higher level convenience methods like `alloc_array`.

Should any of these be added to the global allocator as well? It may make sense
to add `alloc_excess` to the allocator API. This can either have a default
implementation which simply calls `allocate` and returns the input size, or
calls `allocate` followed by `reallocate_inplace`.

The existing `usable_size` function (proposed for removal) only takes a size and
align. A similar, but potentially more useful API is one that takes a pointer
to a heap allocated region and returns the usable size of it. This is supported
as a GNU extension `malloc_useable_size` in the system allocator, and in
jemalloc as well. An [issue][usable_size] has been filed to add this to support
this to aid heap usage diagnostic crates. It would presumably have to return an
`Option` to for allocators that do not have such a function, but this limits
its usefulness if support can't be guaranteed.

[usable_size]: https://github.com/rust-lang/rust/issues/32075