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

Working around Arc<Mutex<Allocator>> #43

Open
nice-sprite opened this issue May 29, 2024 · 0 comments
Open

Working around Arc<Mutex<Allocator>> #43

nice-sprite opened this issue May 29, 2024 · 0 comments

Comments

@nice-sprite
Copy link
Contributor

Hello,

The point of this issue is to open discussion about the use of Arc<Mutex<Allocator>>. The main problem, as a user, is that it forces me to arc-mutex wrap my allocator inside my own engine, even if I am only writing single-threaded code. Based on the //TODO: Another way? comment, I guess I am not alone. What was the use case that required arc-mutex when designing the library?
I am curious to know your thoughts on workarounds or alternative designs, but I'd also like to share my own.

Instead of storing the allocator, why not pass it to all functions that can possibly allocate? Making allocating functions explicitly take an allocator gives the user more context. Then, the user can decide whether or not they need to put that call inside a critical section. It's also worth mentioning VulkanMemoryAllocator is "internally thread-safe".

For example, it would look like this:

Single-Threaded Use Case

We don't have any worker threads doing texture/buffer uploads. No need for any synchronization.

struct Engine {
    ...
    vma_allocator: vk_mem::Allocator,
    ...
}
// inside fn draw_frame()
imgui_renderer.cmd_draw(&engine.vma_allocator, cmd, imgui_draw_data);

Multi-Threaded Use Case

A common case here would be using the allocator to allocate/upload textures on a worker thread, while our main render thread is still trying to draw imgui. The allocator would have some kind of sync around it, but it isn't the imgui renderers job to care about outside details.

struct Engine {
    ...
    vma_allocator: Arc<Mutex<vk_mem::Allocator>>, // User can opt-in to sync
    ...
}

// inside fn draw_frame()
engine.vma_allocator.try_lock().map(|ref mut alloc| {
    imgui_renderer.cmd_draw(alloc, cmd, imgui_draw_data);
});

Renderer would no longer store the allocator, just borrow it when it needs to. The main issue with this is that Drop becomes impossible because you cannot pass parameters to Drop. One solution would be to actually re-purpose Drop into a reminder, and have the user call a Renderer::destroy(...) function that handles destroying the resources. Inside Drop, you would then simply put checks to make sure they called destroy before the struct actually goes out of scope, and give a helpful message if they haven't.

A bonus

As I was typing this, I realized it's possible to trim down Renderer by doing the same with ash::Device. At the cost of adding a parameter, you can trim off 1488 bytes from the Renderer struct bringing it down to just 192 bytes (if you also remove the allocator). Another approach would be to just copy out the device functions that are actually used and only store those if there are some tricky borrow checker issues that arise.

  • nice_sprite
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

1 participant