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.

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

Support realloc without copying? #40

Open
hbgl opened this issue Feb 25, 2020 · 9 comments
Open

Support realloc without copying? #40

hbgl opened this issue Feb 25, 2020 · 9 comments

Comments

@hbgl
Copy link

hbgl commented Feb 25, 2020

When using realloc it will commonly fallback to alloc, copy, and free if growing the initial allocation is impossible (see mimalloc's realloc for an easy to read example). I think it would be useful to have a specialization of realloc that would leave the copying to the user code.

A good use case would be allocating a buffer for a Vec<Option<T>> where None represents a tombstone i.e. a slot that was previously occupied but is now empty (think of PHP array or Python dict). When growing the buffer it would sometimes make sense to only copy Ts and skip the tombstones. I would expect the usage of the new function to look something like this:

let allocation = realloc_manually_copy(ptr, layout, new_size)?;
if allocation.needs_copy() {
    let old_slice = slice::from_raw_parts(allocation.old_ptr, len);
    let vec = Vec::from_raw_parts(allocation.new_ptr, 0, new_size);
    vec.extend(slice.filter(|x| x.is_some());
    // ...
}
drop(allocation); // Will free the old memory

I do not know of an allocator interface which supports this special realloc operation but I thought it might be useful. The current implementations of the PHP array and Python dict do not bother with realloc and just use malloc instead.

@Lokathor
Copy link

I think the cost of all the checks and branching is going to overwhelm any savings from not copying a few bytes of None

@Amanieu
Copy link
Member

Amanieu commented Feb 25, 2020

If you really want this then you can do it with grow_in_place and manually copying to a new allocation if the call fails.

@hbgl
Copy link
Author

hbgl commented Feb 25, 2020

@Lokathor
I agree that the branching will increase the cost of growing the buffer but it would be faster than copying unconditionally and then doing the branching later. It's a trade-off for sure.

@Amanieu
Does grow_in_place imply pointer stability? That would be a stronger guarantee than realloc has.

@hbgl hbgl closed this as completed Feb 25, 2020
@hbgl hbgl reopened this Feb 25, 2020
@Amanieu
Copy link
Member

Amanieu commented Feb 25, 2020

Yes, grow_in_place guarantees pointer stability if it succeeds.

@hbgl
Copy link
Author

hbgl commented Feb 25, 2020

@Amanieu That means though that realloc_manually_copy allows the allocator to be more efficient than grow_in_place plus fallback because it is less constrained. I see that realloc_manually_copy might be troublesome to implement because the allocator would be in a weird state being interrupted by user code in the middle of a regular realloc.

It would be interesting to know if someone already pursued this idea and what the outcome was. I have not found much online and I have no perf data to justify an extra function. I guess this is more a question about whether it is worth checking out at all.

@Amanieu
Copy link
Member

Amanieu commented Feb 25, 2020

If the allocator has to move your data, it is already making a separate allocation and freeing the old one. This is exactly the same as what you should do if grow_in_place fails: just alloc, copy your data, then free the old allocation.

@the8472
Copy link
Member

the8472 commented Apr 25, 2024

Is grow_in_place a hypothetical method or was it removed at some point?

@Amanieu
Copy link
Member

Amanieu commented Apr 25, 2024

It was removed because in practice no allocators supported it.

@the8472
Copy link
Member

the8472 commented Apr 25, 2024

mimalloc has mi_expand which guarantees to either perform in-place expansion or returns null.

So it would be good to have a way to add future support, e.g. via flags as #58 proposed.

And it seems obvious to me that anything that uses mmap under the hood could support this at least in principle via MREMAP_FIXED on platforms that have that. So even if our most commonly used allocators today don't have it the API design should provide some forward-compatibility here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants