gfx-rs / gfx

[maintenance mode] A low-overhead Vulkan-like GPU API for Rust.
http://gfx-rs.github.io/
Apache License 2.0
5.35k stars 547 forks source link

Safe and convenient resource destruction #288

Closed kvark closed 9 years ago

kvark commented 10 years ago

Would be nice to have our resources self-destructed, though I'm not sure we can really do that with our architecture and potential other device back-ends.

The path seems to be mostly cleared now (at last!). Here are the steps we will take:

PR #633 covers the basics. Issue #636 will cover Rc usage.

kvark commented 9 years ago

Actually, screw RAII. We really just need linear types for the resources.

Edit: that was a silly idea! Linear types are great, but not for our case.

bvssvni commented 9 years ago

Does Rust support linear types?

kvark commented 9 years ago

AFAIK, not yet, unfortunately. I hope someone will post the status of linear types support here.

brendanzab commented 9 years ago

Could we cc the linear type RFC? Would be good to show them a real use case.

kvark commented 9 years ago

@bjz what RFC is that? I only found pre-RFC some time ago and already commented on that.

brendanzab commented 9 years ago

Ohh, I rembered it, but forgot it wasn't up on Github yet.

kvark commented 9 years ago

@bjz I find it weird that linear types are still in pre-RFC stage. For our purpose, it would suffice to just be able to disable going out-of-scope for some type: generate a compile error whenever a variable of this type goes out of scope. The only way to destruct it would be to deconstruct it, and since the type is provided by our library, we can prevent user from deconstructing it by adding any private fields.

kvark commented 9 years ago

Here is a use-case shared by @XMPPwocky:

it's nice to be able to, for example, have a loaded mesh be a single struct and then you can Arc that

Unfortunately, linear types will not allow this case. Time to brainstorm it again?

kvark commented 9 years ago

Notes by @amaranth:

You also need to tie resources to command buffers and make sure they live on the CPU until the GPU has processed them Otherwise you might delete something from the GPU before it gets done using it Submitting it and then forgetting it isn't enough You'll drop all the references and generate destroy resource calls that'll happen before the command is actually completed GL will handle that for you I think so it won't break but apparently Vulkan at least won't You actually need this for bindless resources in GL too afaik

By @tomaka :

yeah you need a fence for vulcan yeah, glium already creates fences automatically when you have persistent mapped buffers

kvark commented 9 years ago

I gave it a little think time, and came up with the following plan (extending @csherratt's proposal):

Plan

Every device-associated resource is stored by Arc<T>. Take Shader, for example:

#[derive(Copy, Clone, PartialEq, Debug)]
pub struct Shader<R: Resources>(Arc<<R as Resources>::Shader>, shade::Stage);

Analysis

It's not clear how much of this code we'll be able to store in the device-independent part. Hopefully - all of it, but that leaves the question open about user-facing delete_X() routines. Perhaps, they will simply not do anything if the resource still have references.

One obvious drawback is performance. I don't know how much Arc would cost us. However, I don't see any other way to provide resource safety either, so we've got to accept the tradeoff. Note: linear types wouldn't help here simply because it would be impossible to use/reference a resource inside a command buffer (at least, without Rc/Arc).

Speaking of the costs, it would be great to allow Rc instead of Arc for single-threaded apps. Does this need HKT to get correctly? Or, perhaps, we can go with a build configuration.

Problems

How do we check that an Arc<T> or Rc<T> is the last reference (and transform it back into T)? At some point long ago (when I was doing exactly the same resource management approach for Claymore, btw) I filed a https://github.com/rust-lang/rust/pull/14908 . Now that I'm looking at it, I see std::rc::is_unique() (here) but nothing like that for Arc. Time for another PR!

ghost commented 9 years ago

There is another option that I have thought about. It has a different set of trade-offs from what is described above.

Like @kvark's suggestion this would probably have to use Arc pointers to track resources.

There are some advantages:

Some disadvantages:

Some differences: