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

Box in Arena Allocator is !Send #114

Open
VictorBulba opened this issue Jun 16, 2023 · 5 comments
Open

Box in Arena Allocator is !Send #114

VictorBulba opened this issue Jun 16, 2023 · 5 comments

Comments

@VictorBulba
Copy link

Hey!

Recently I started migrating from bumpalo to allocator-api and encountered an issue.

bumpalo::boxed::Box is Send, but std::boxed::Box is Send if A: Send, and in the case of arena allocators A is &ArenaAlloc, so it requires ArenaAlloc: Sync, which is not the case for most of them.

So that, when you don't need to clone/realloc the box, and dealloc is a no-op (for arena allocators), there is no need to store the allocator inside Box.

There is already the issue #112 for splitting Allocator trait, to reduce Box overhead by using ZST deallocator type when you need it. This can also solve the requirement for the arena Allocator to be Sync, in favor of storing () deallocator.

So I am bringing another useful use case to this discussion.

@Amanieu
Copy link
Member

Amanieu commented Jun 16, 2023

This could also just be solved in your own code by passing &T to other threads instead of Box<T, A>. You can generate a reference tied to the lifetime of the allocator with Box::leak, which won't actually leak memory for an arena allocator.

@VictorBulba
Copy link
Author

But I need Drop

@Amanieu
Copy link
Member

Amanieu commented Jun 16, 2023

Perhaps this could be implemented as functionality in the arena allocator:

impl Arena {
    // DummyAlloc is ZST and only allows dealloc, panics on alloc.
    fn with_dummy_alloc<T>(b: Box<T, &'_ Arena>) -> Box<T, DummyAlloc>;
}

As I've mentioned in #112, I think that splitting the Allocator trait introduces too much API complexity.

@VictorBulba
Copy link
Author

I agree that splitting allocator trait may be too much complicated over benefits it gives :( But mapping box allocator is a bit a leaking abstraction.

@zakarumych
Copy link

Allocator that panics on allocations sound really nice. Nothing bad can happen.

One more optional trait that has only one method of Allocator doesn't sound like something incredibly complex. Especially in corner of the language where a lot of knowledge and care already required from programmers.

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

3 participants