kripken / box2d.js

Port of Box2D to JavaScript using Emscripten
1.32k stars 198 forks source link

Memory Leak #27

Closed Joncom closed 11 years ago

Joncom commented 11 years ago

I'm working on a Box2D.js plugin for the ImpactJS game engine. Ever since I made this commit, I've been getting this error message:

Uncaught Assertion: Cannot enlarge memory arrays in asm.js. Either (1) compile with -s TOTAL_MEMORY=X with X higher than the current value, or (2) set Module.TOTAL_MEMORY before the program runs.

It happens consistently after 2 minutes of running with about 250 entities. After doubling TOTAL_MEMORY, the error continues to occur, but after 7.5 minutes instead.

Before this commit, I could run the simulation for hours without any issue.

I suspect what's happening here is that instances of b2Vec2 are being created but not destroyed properly, and eventually all the memory is used up. What am I doing wrong here?


Here is the new troublesome method being called once per update by every entity:

limitVelocity: function() {
    var velocity = this.body.GetLinearVelocity();
    var x = velocity.get_x() / Box2D.b2SCALE;
    var y = velocity.get_y() / Box2D.b2SCALE;
    if(x < -this.maxVel.x)     x = -this.maxVel.x;
    else if(x > this.maxVel.x) x = this.maxVel.x;
    if(y < -this.maxVel.y)     y = -this.maxVel.y;
    else if(y > this.maxVel.y) y = this.maxVel.y;
    x *= Box2D.b2SCALE;
    y *= Box2D.b2SCALE;
    this.body.SetLinearVelocity( new Box2D.b2Vec2(x, y), this.body.GetPosition() );
}
Joncom commented 11 years ago

Found some useful info here:

JavaScript, specifically ECMA-262 Edition 5.1, does not support finalizers or weak references with callbacks. Thus, JavaScript code must explicitly delete any C++ object handles it has received. Otherwise the Emscripten heap will grow indefinitely.

With that in mind, I realized that I was creating new b2Vec2 instances, but never destroying the original one. I'm so used to letting the JavaScript garbage collector do that for me.

So instead of creating a new b2Vec on the last line, I re-used the one that already exists:

velocity.set_x(x);
velocity.set_y(y);
this.body.SetLinearVelocity( velocity, this.body.GetPosition() );

Perfect!

jagenjo commented 11 years ago

But is it possible to delete from Box2D the references we no longer need? because reusing is dangerous, I dont know if the reference I passed to box2D is being used internally so any changes could affect existing objects on the scene. I guess I could check if the functions use copy, reference or pointer for every call, but maybe there is an easier way to free memory.

kripken commented 11 years ago

destroy should free the C++ side of things.

On Thu, Jul 18, 2013 at 6:10 AM, jagenjo notifications@github.com wrote:

But it is possible to delete the references we no longer need? because reusing is dangerous, I dont know if the reference I passed to box2D is being used internally so any changes could affect existing objects on the scene.

— Reply to this email directly or view it on GitHubhttps://github.com/kripken/box2d.js/issues/27#issuecomment-21181935 .