fitzgen / bumpalo

A fast bump allocation arena for Rust
https://docs.rs/bumpalo
Apache License 2.0
1.36k stars 111 forks source link

Allow iter_allocated_chunks on a &self #121

Closed ndmitchell closed 2 years ago

ndmitchell commented 2 years ago

In starlark-rust we use Bumpalo as the backing for a garbage collector. Starlark has the property that after a module has been interpreted, its heap is frozen, which leads us to have struct FrozenHeap(Arc<Bumpalo>). We'd like to add a feature to iterate over the heap to produce memory profile reports, but alas, iter_allocated_chunks takes a &mut self, which we can't produce given an immutable Bumpalo in an Arc. Would it be possible to provide an iter_allocated_chunks variant which took a non-mut self, perhaps in unsafe if necessary. While I'm happy to call unsafe, converting my &Bumpalo into an &mut Bumpalo given many owners is instant undefined behaviour in Rust.

fitzgen commented 2 years ago

iter_allocated_chunks gives our shared references to the underlying chunks which, if we allowed &self on iter_allocated_chunks, could have active exclusive references from allocated things, thereby breaking Rust's exlusive xor shared rules and causing undefined behavior. So I think iter_allocated_chunks is a non-starter here.

However, we could have something like iter_allocated_chunk_ptrs that just returns raw *mut u8s and leaves it up to you to figure out whether it is safe to turn those things into a reference or not. I think this would still have to be unsafe because nothing would stop you from allocating from the bump while iterating over the chunks, which will mess with the iterator. It could technically be possible to handle this correctly, but it seems like a lot of unnecessary maintenance overhead for a use case that shouldn't really be supported. I'd rather just mark it unsafe and document that part of the unsafety contract is that allocating from the Bump while one of these iterators is active is unsafe.

ndmitchell commented 2 years ago

The *mut version would work for me.

poconn commented 2 years ago

I'm working on this and for the starlark-rust use case we need the chunk size also. I could return a struct containing the pointer and size, but I think it's actually fine to just return slices? As you say it will need to be unsafe either way and the caller needs to ensure no modifications while the iterator is alive, and as long as this is upheld then constructing slices should be fine, right? Makes the implementation much simpler too.

fitzgen commented 2 years ago

&[MaybeUninit<u8>] items should be fine to yield from the iterator.