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

It is easy to leak mesh data #464

Closed
brendanzab opened this issue Nov 29, 2014 · 15 comments
Closed

It is easy to leak mesh data #464

brendanzab opened this issue Nov 29, 2014 · 15 comments

Comments

@brendanzab
Copy link
Contributor

loop {
    let mesh = graphics.device.create_mesh(vertices.as_slice());
    let slice = terrain_mesh.to_slice(gfx::TriangleList);
    let state = gfx::DrawState::new().depth(gfx::state::LessEqual, true);
    let batch: shader::Batch = graphics.make_batch(&program, &mesh, slice, &state).unwrap();
}

Not sure how to fix this.

@brendanzab
Copy link
Contributor Author

This could be another instance where HKT would be useful. You could either get a batch which borrows a mesh, or maybe have an Rc pointer to a mesh... dunno how that would work though... :S

@brendanzab
Copy link
Contributor Author

Woops, thanks @cmr!

@TyOverby
Copy link

TyOverby commented Jan 3, 2015

I'm running into this right now. 100% memory usage after 10 mins using my library. The main issue is that I'm basically getting a mesh and then throwing it away immediately. Is there a better way to do this or is there a way to delete meshes?

@emberian
Copy link
Contributor

emberian commented Jan 3, 2015

To delete a mesh:

for attr in mesh.attributes.into_iter() {
    device.delete_buffer(attr.buffer);
}

@emberian
Copy link
Contributor

emberian commented Jan 3, 2015

(Although I think this will still leak VAOs, I don't remember)

@TyOverby
Copy link

TyOverby commented Jan 3, 2015

@cmr That code snippet doesn't work, but this compiles:

for attr in mesh.attributes.into_iter() {
    self.graphics.device.delete_buffer_raw(BufferHandle::from_raw(attr.buffer));
}

But now my program crashes with

thread '<main>' panicked at 'Error after executing command BindAttribute(1, 1, Format { elem_count: 4, elem_type: Float(Default, F32), offset: 8, stride: 32, instance_rate: 0 }): InvalidOperation', ../gl_device/lib.rs:177

@TyOverby
Copy link

TyOverby commented Jan 3, 2015

This is apparently happening when I call self.graphics.end_frame().

Full stacktrace:

Breakpoint 1, 0x000055555570a880 in rust_panic ()
(gdb) backtrace
#0  0x000055555570a880 in rust_panic ()
#1  0x00005555556fc69a in rt::unwind::begin_unwind_inner::h622221e44b516048spz ()
#2  0x00005555556fc9ae in rt::unwind::begin_unwind_fmt::h74b6394a531b4fee7mz ()
#3  0x00005555556447c4 in device::gl_device::GlDevice::check (self=0x7fffffffb1d8, cmd=0x7ffff2ea4268) at <std macros>:16
#4  0x000055555564ae0c in device::gl_device::GlDevice::process (self=0x7fffffffb1d8, cmd=0x7ffff2ea4268, data_buf=0x7fffffffddc8) at ../gl_device/lib.rs:555
#5  0x000055555564c10e in device::gl_device::GlDevice.Device<GlCommandBuffer>::submit (self=0x7fffffffb1d8, cb=0x7fffffffddb0, db=0x7fffffffddc8)
    at ../gl_device/lib.rs:574
#6  0x00005555555c9930 in Graphics$LT$D$C$$u{20}C$GT$::end_frame::h4082753395382841626 ()
#7  0x00005555555c9265 in lux::glutin_window::Window::render (self=0x7fffffffb1b8) at src/glutin_window.rs:259
#8  0x000055555556aa33 in lux_demo::main () at src/bin.rs:51
#9  0x000055555570f679 in rust_try_inner ()
#10 0x000055555570f666 in rust_try ()
#11 0x000055555570cd3f in rt::lang_start::h17879a8d5205229eeFz ()
#12 0x000055555556ab85 in main ()


@kvark
Copy link
Member

kvark commented Jan 8, 2015

Looks like you deleted the mesh buffers and then executed a draw list that used them. We should not allow that, ideally...

@ghost
Copy link

ghost commented Jan 25, 2015

I think we could add a reference count to buffer/texture ect. This would let us track of the buffer was still in use and would be freed automatically when the user no longer had access to the buffer and the buffer had flushed its way through any pending draw lists. If the user wants to reuse the same buffer between frames to avoid allocation, this model would naturally work.

The down side is that it adds considerable overhead to the buffer objects. You need a reference count and a pointer back to device in order to free the the buffer when it is done.

There is also a trade off between Rc and Arc pointers. Rc being faster, but making it more difficult to figure out how to use multiple threads.

@kvark
Copy link
Member

kvark commented Feb 18, 2015

Related to #288 and #570

What I was thinking is: we could have a backend-agnostic wrapper around the device (might as well just be our Graphics thing), which would produce handles with reference counting (what @csherratt described), which also carry something (say, ()) and implement ResourceDestructor, thus being able to kill them selves properly. It's still a bit cheesy though, but conceptually doable.

@kvark
Copy link
Member

kvark commented Feb 18, 2015

Actually, the problem with stand-alone resource deletion is not just conceptional integrity of the device. It also needs to fix/notify whatever caches that rely on the resource. This might be compatible with Renderer caches, because this one may just hold the references until it gets reset, but the Device may also want to cache it.

@kvark
Copy link
Member

kvark commented Feb 18, 2015

Just re-capping the possible scenarios of our unsafe resource handling:

  1. User drops a resource, forgetting to release it
  2. User releases a resource, and then tries to execute a draw list that uses it
  3. User looses or re-creates the device, while the old resource is still alive

So far it looks like @csherratt proposal mostly solves it. It would be nice to have it as an option.

Also related to #530, cc @dylanede, @XMPPwocky

@ghost
Copy link

ghost commented Feb 18, 2015

3 . User looses or re-creates the device, while the old resource is still alive

This should probably be detection only. I don't think we should hold onto a large mesh or texture for longer then we need to.

@kvark
Copy link
Member

kvark commented Mar 8, 2015

It is partially addressed by #633

@kvark
Copy link
Member

kvark commented May 26, 2015

Radically fixed in #757

@kvark kvark closed this as completed May 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants