kripken / box2d.js

Port of Box2D to JavaScript using Emscripten
1.33k stars 196 forks source link

Memory leak in repeated b2Body.GetLinearVelocity calls #83

Open erbridge opened 7 years ago

erbridge commented 7 years ago

I'm finding that when calling b2Body.GetLinearVelocity in a loop, the memory footprint expands. We have ~2000 bodies (a lot I know), and call GetLinearVelocity on each of them once every frame, and Box2D runs out of memory very quickly (in seconds).

I have tried caching the return value of the GetLinearVelocity, but it doesn't seem to be updating the LengthSquared unless I make another call to GetLinearVelocity (even if the result of that call is discarded).

Is this possibly related to the fact that GetLinearVelocity returns a reference?

kripken commented 7 years ago

I assume you meant it returns a value, not a reference? Returning a value is ok - our glue code does this:

b2Vec2* EMSCRIPTEN_KEEPALIVE emscripten_bind_b2Body_GetLinearVelocity_0(b2Body* self) {
  static b2Vec2 temp;
  return (temp = self->GetLinearVelocity(), &temp);
}

So it doesn't allocate a new object each time (but the objects are just valid until the next call).

When you say the memory footprint expands, how are you measuring it?

erbridge commented 7 years ago

Honestly, the most obvious is that seconds of interaction cause Box2D to crash, complaining about running out of memory. Commenting out the single (repeated) GetLinearVelocity call means it runs fine for ages.

kripken commented 7 years ago

What complains? Browser, app, or OS? What is the message?

erbridge commented 7 years ago

In the browser console:

Cannot enlarge memory arrays. Either (1) compile with  -s TOTAL_MEMORY=X  with X higher than the current value 16777216, (2) compile with  -s ALLOW_MEMORY_GROWTH=1  which adjusts the size at runtime but prevents some optimizations, (3) set Module.TOTAL_MEMORY to a higher value before the program runs, or if you want malloc to return NULL (0) instead of this abort, compile with  -s ABORTING_MALLOC=0
kripken commented 7 years ago

I see. Surprising, I have no idea what could cause that. Can you perhaps make a small testcase that shows the issue deterministically?

erbridge commented 7 years ago

I don't appear to be easily able to reproduce. Perhaps it would be helpful to explain the full situation.

We have a body that we drag around with world, colliding with a dense field of other bodies. It's pretty slow (due to there being so many bodies), so I was hoping to be able to improve performance by sleeping bodies that are moving at velocities below a certain threshold. To do this, I'm iterating through all the bodies, checking their velocity, and if it's below the threshold, calling SetAwake(false) on them. Any ideas why that might be causing issues?

kripken commented 7 years ago

No idea, sorry. But it must be somehow connected to the other operations around it, as I don't see how GetLinearVelocity by itself could cause that.

Or, perhaps it's not related to the wrapper code at all - maybe this is a bug in box2d in native builds as well, i.e., maybe GetLinearVelocity allocates something that isn't freed.

erbridge commented 7 years ago

That seems the most likely explanation to me, but I can't see any obvious alarm bells (though I am not a C programmer).