Oh look, a memory leak!

Discuss any questions about BEPUphysics or problems encountered.
Post Reply
mcmonkey
Posts: 92
Joined: Fri Apr 17, 2015 11:42 pm

Oh look, a memory leak!

Post by mcmonkey »

So, I found a memory leak!

If you create one entity shape, and use it for many collidables... when those collidables go away... they don't go away, nor do data connected to them!
Because they leave an Action<CollisionShape> in the shape's event "ShapeChanged"!
This causes roughly ten gerjillion Action<CollisionShape> instances to exist, which contain a fair bit of data each...

As it is, I actually am doing exactly that... many many many many times per second!
I accidentally caused this to happen even more rapidly than normal, and looked at task manager... and bam, many gigs higher than it has any reason to be!

I managed to, eh, "fix"?, it...

Basically, I tried weakly to make it automatically clear via a finalizer because why not I don't know the BEPU code well enough to properly implement a disposable and also I was kinda hoping a finalizer would work except upon looking back, I realized it's data that references itself and as such it would never be finalized so rip that plan.
What did work, though, was adding a method to clear the ShapeChanged data, since I literally have zero use for that event as my shapes in question never change...
So I just call that every time I create a new instance of a colldidable and call it 'good enough'.
Here's relevant code if you care to see what I did bepu-side: https://github.com/mcmonkey4eva/bepuphy ... ac8e023d0f

CollisionShape -> ClearShapeChanged is the one that actually ... does anything :P

I'm hoping for a proper fix, which may potentially mean rewriting a fair bit of code to properly Dispose the collidables and entities that add ShapeChanged... I don't even know tbh.
Norbo
Site Admin
Posts: 4929
Joined: Tue Jul 04, 2006 4:45 am

Re: Oh look, a memory leak!

Post by Norbo »

Ah shoot. This is one of those things I've known about for a while, but I've just been secretly hoping no one would run into it until v2 was out and obviated any 'fix'. So close! :)

Some workarounds:
1) Pool collidable instances. This is probably a good idea regardless- recreating the collidables over time is just going to trigger unnecessary GCs.

2) Some types allow the Shape to be set to null. That'll clear out the event, if your collidable is such a type.

3) Just use something like your ClearShapeChanged. It's pretty close to optimal as far as effort vs. effectiveness at addressing the problem. I went ahead and implemented a Collidable.ShapeChangedHooked property which accomplishes nearly the same thing- it can just be set to false for any collidable where updates don't matter. Technically, collidables could be constructed with a flag that controls whether shape changed is hooked, or all collidables could gain the ability to set the Shape dynamically, or this or that complex solution, but all of these would require adding a lot of very error prone fiddly code (like the rest of the web of interrelationships). I'm not confident that even I could change this area quickly without introducing a bug somewhere.

4) ... delete everything related to OnShapeChanged in the engine and pretend it never existed. This is v2's approach. It drops the vast majority of the questionable helper metadata, like the shape changed event. The general idea is: if some bookkeeping task is doable externally with no extra performance overhead and is not fundamentally required by the operation of the engine, exclude it. The engine assumes shapes are just immutable data blobs until explicitly notified otherwise.
Post Reply