Resource management

[kvark]

Closes #22
Looks a bit ugly with all those unwrap() things (cleanup PRs are welcome!), but functional and safe from the user perspective. Also - Storage needs tests.

[emberian]

Please don't merge this without me 🐈

[emberian]

Travis failure is benign but annoying...

[emberian]

Comments extracted from a commit, so that github doesn't eat them as soon as you push a new version:

---

So, my understanding of generation is that when a handle is created for the first time, the room it is using is initialized with generation 0. However, the handle may be freely cloned and copied, so that when the resource is later freed via remove, handles can still refer to that slot. So the generation is incremented, and if a handle with an older generation tries to access the room, it is rejected, because that old handle is now invalid. If the handle hits the max value, instead of overflowing, we just never reuse the room.

If this is accurate, please add it all as a doc comment somewhere in here :)

---

One way to avoid this runtime checking is to make Handle non-copyable by embedding a NoCopy member (and don't implement Clone), and require remove to take the handle by value. This ensures that only one handle to the data can exist, and once removed, we can always safely delete it. This is possibly desirable: it makes the Storage implementation a lot simpler, and if the user actually needs multiple references to a handle (which is probably common!) they can wrap it with Rc, essentially moving the fallible-refcounting-via-generation and making it infallible such that if you have a handle, you can always access the data associated with that handle.

---

For long-running applications I can see the following pattern being very common: create a handle, then destroy it. Next time a handle is requested, it will go in that room, increasing the generation count. Do this 2^16 times, and you're slowly "leaking" memory by freezing slots in the storage. This is very unfortunate, and is avoided by the above treatment of Handle as an affine resource.

---

One thing you don't necessarily want with Handle is automatically removing itself in the destructor. Then to clean itself up, it either needs to:

a. store a &mut Storage<T>, borrowing the handle manager (yuck)
b. store an Rc<RefCell<Storage<T>>> (yuck)

So, it probably should have a destructor that does fail!("ERROR: Handle leaked!") for debugging. It's unfortunate that Rust doesn't have true linear types (what we have today, but without having destructors implicitly called to destroy the value, requiring it to be moved into a dedicated cleanup function), since that would remove the need to have any checking at all.

[brendanzab]

Not sure how I feel about the banner comments...

[kvark]

We sure have groups of functions (creation, modification, deletion, binding). How do we mark them as such?

[kvark]

@cmr where were you when I asked for feedback for a gist yesterday?.. Eh.
Also, the travis error is very hard to fix, it seems.

[kvark]

@cmr

So, my understanding of generation is...

This is correct and described nicely. I'll add it as a comment if we accept on this approach.

One way to avoid this runtime checking is to make Handle non-copyable

I don't believe we can do this, because handles are the means of communication between the user and gfx-rs. E.g. if Copy/Clone is not allowed, how do we construct an IndexSlice every frame? gfx itself may store multiple copies of the same handle, for example in the Mesh definition.

and you're slowly "leaking" memory by freezing slots in the storage

True, but how much do we really leak? Roughly, X resource allocations per frame means X/65K bytes per frame, means X/1000 bytes per second means approx 3.6X bytes per hour. Doesn't seem bad to me to care enough.

[photex]

The memory leak is not sustainable. I figured this was a good place to start before we can figure the best way to resolve the issue (we need something in place). Consider long running installations, or simulations. You want gfx to be appropriate for many kinds of app.

The suggestions by @cmr seem pretty good at first and second read. Not sure I follow the last part, but it should be discussed. Wrapping it in RC to make it copyable... does that put us basically in the same boat with regards to invalidating handles that parts of the app might still have a reference to? /me goes to the rust docs to catch up....

[brendanzab]

Needs a rebase.