DestroyBody / recycled bodies

[hutzlibu]

I got very strange bugs, which I tracked down to the fact, that apparently Box2D in general or just this port via emscripten does recycle bodies. (but I never had this issue with the port from actionscript)

Meaning, unlike I thought

const bd = new b2BodyDef();
const body = world.CreateBody(bd);

does not necessarily gives you a fresh new body.
If you removed bodies before with

world.DestroyBody(body)

and those bodies had some custom set properties, those old properties are still there, when objects gets recycled!
(Which in my case led to bullets behaving like enemies for example)
This also means big memory leaks.

So the solution for me was to iterate through each ownproperty before destroying bodies and removing every custom property.

for (var i in body){
 // Su is the pointer, which we want to keep
 if(body.hasOwnProperty(i) && i != "Su"){
    delete body[i]
 }
}
world.DestroyBody(body)

I believe this should go to the docs ...

[Birch-san]

Thanks for investigating.

Yes, I saw something like this myself. Definitely need to figure out the best fix for it, and document it. I expect your technique is equivalent to invoking every setter with undefined (probably interpreted as 0).

I think Box2D APIs are only half of the story — we also need to use Emscripten APIs. we use Emscripten to allocate new Box2D objects on the WebAssembly heap, so we need to use Emscripten to de-allocate them too.

kripken/box2d.js#27 (comment)

Maybe something like this?

world.DestroyBody(body)
box2D.destroy(body);

[hutzlibu]

box2D.destroy(body);

Does not work, throws: "Cannot destroy object, did you create yourself?"

So it needs to be destroyed with world.DestroyBody

But good point about box2D.destroy and the link in general, I did not realize I even have to destroy all b2Vec2 after use, but it makes sense.

But this made me thinking, and this is where it gets complicated:
do I also have to destroy my fixtures attached to a body by hand, or does world.destroy handles this for me?
And what about bodydefinitions, etc.?

[hutzlibu]

Short answer, yes. Everything needs to be destroyed by hand.

For fixtures, every body has a DestroyFixture method.

So,
body.DestroyFixture(body.GetFixtureList())
seems to do the trick. And all the body definitions etc. needs to be destroyed with

box2D.destroy()

Or they cause crashes after a while, if you continue to create and destroy bodies, or just make a new b2Vec2 in your mainloop. Gotta clean up my code ..

[hutzlibu]

Things are complicated with wasm.

Cleanly removing unused objects (box2D.destroy) results in massive performance drops when a lot is going on.

Puffering the garbage objects and cleaning them up later, results in crashes.

Could be bugs in my code though, but hard to figure out.

[Birch-san]

Every Box2D object created with new, needs to be freed with box2D.destroy().

world.DestroyBody(body) is the correct way to destroy a body created via world.CreateBody(bd).
My suggestion to try destroy() in this situation was wrong (object was not created with new).

DestroyBody deletes every fixture in the body's fixture list, so I don't think body.DestroyFixture() is necessary. I wouldn't recommend body.DestroyFixture(body.GetFixtureList()) — this will only delete the first fixture in the list.

world.CreateBody() re-uses previously-freed memory, but the body created by invoking the b2Body constructor looks like it gets zero-initialised; you should receive a pretty clean instance.

If box2D.destroy() is too expensive, then you'll instead need to re-use your new-constructed objects. It's safe to re-use b2Vec2 instances. For example, see how the same b2Vec2 can be used twenty times. Box2D APIs tend to copy the objects you give them, rather than taking a pointer. So you don't need a fresh instance each time. Still worth destroy()ing when you're done with it, but maybe you only need one b2Vec2 for your entire program?

As for body definitions… when world.CreateBody() invokes the b2Body constructor, it copies the properties of the b2BodyDef instead of taking a pointer. So I think you're free to modify and re-use the b2BodyDef after that.

I haven't been able to figure out how the bodies you received via world.CreateBody() were polluted with properties from bodies previously-destroyed via world.DestroyBody(). If you're still experiencing that, I think I'd need a minimal example that reproduces it in order to investigate.

[hutzlibu]

If you're still experiencing that, I think I'd need a minimal example that reproduces it in order to investigate.

Well, just modifying the demo code a bit, with adding a createdBefore property and destroying it and checking after creating, gives me the same result:

const boxCount = 30;
  for (let i = 0; i < boxCount; i++) {
    const bd = new b2BodyDef();
    bd.set_type(b2_dynamicBody);
    bd.set_position(ZERO);
    const body = world.CreateBody(bd);
	if(body.createdBefore)
		debugger
    body.CreateFixture(i % 2 ? square : circle, 1);
    initPosition(body, i);
	
	body.createdBefore=1
	world.DestroyBody(body)
	
  }

[hutzlibu]

And regarding freeing memory and reusing:

I am working on a general approach for this and stop using new directly with all Box2D wasm objects.
So you could recycle objects internally or create with new as needed, but not have to deal with this mess on the surface
It will be a struggle though, to find out, which objects apart from B2Vec2 are really safe to recycle though.
Anyway, this is the approach I am starting with:

const recycleHeap={}
// keep track of total number of new created objects - should stay constant after a while with no memory leaks, as then new objects gets reused and not new allocated
const createCount=0
function make(type) {
	
	if(recycleHeap[type.name] && recycleHeap[type.name].length){
		let recycledObject = recycleHeap[type.name].pop()
		if(arguments[1]!=null && type.name=="r"){
			recycledObject.x=arguments[1]
			recycledObject.y=arguments[2]
		}
		return recycledObject
		
	}
	else{

                createCount++

		if(arguments[1]!=null){
			// for now only b2Vec2 supported, general approach more complicated, but maybe unnecessary?
			if(type.name!="r"){
				console.warn("not supported")
				return
			}else{
				return new type(arguments[1],arguments[2])
			}
			
		}
		return new type()
	}
}
function free(obj) {
       // do we already have a array for this type?
	if(!this.recycleHeap[obj.constructor.name])
                // if not create one
		this.recycleHeap[obj.constructor.name]=[]
	// here some cleaning up procedures might be implemented, none so far
	 //add the old object to our recycleHeap
         this.recycleHeap[obj.constructor.name].push(obj)
}

A new b2Vec2(3,4)
would be
let pos = make(b2Vec2, 3,4)
and cleaning it, just
free (pos)

Likewise with any other object.

This does work for me so far, but is not at all completely tested (I do only use a subset of box2D, no joints for example).
So far, I see no problems that it will work. If this turns out to be correct, (once I integrate this approach into my own project) and complete - maybe it makes sense, to integrate this approach into Box2D?

[hutzlibu]

Ok, so I integrated this approach and it works (with a minor fix) .
Tracking and managing my own heap was also useful in finding some bugs in my code.

But I am now not sure, whether destroy really was causing performance to drop, because I had some bugs in my code, where objects where destroyed too soon.

Quick compare with recycling vs. destroying now shows no drastic difference, but I am going to do some measured testing at some point.

[hutzlibu]

To reiterate that point:

managing all box2D wasm objects with the intermediary helper functions create and make is extremely helpful in tracking down memory leaks.
It was quite shocking to see a small memory leak pile up with it with the mainloop.

(code updated above)

[Birch-san]

Thanks for the repro.

Okay yeah, if you create an object without using new (e.g. via factory functions such as world.CreateBody or body.CreateFixture), then you don't own the memory. Box2D will construct an instance into memory allocated via its smallBlockAllocator, which recycles destroyed objects.

There is also reuse on the JavaScript side. The world.CreateBody JS function that Emscripten creates, reuses b2Body instances whenever the pointer matches:

/** @suppress {duplicate} (TODO: avoid emitting this multiple times, it is redundant)
    @param {*=} __class__ */
function getCache(__class__) {
  return (__class__ || WrapperObject).__cache__;
}
Module['getCache'] = getCache;

/** @suppress {duplicate} (TODO: avoid emitting this multiple times, it is redundant)
    @param {*=} __class__ */
function wrapPointer(ptr, __class__) {
  var cache = getCache(__class__);
  var ret = cache[ptr];
  if (ret) return ret;
  ret = Object.create((__class__ || WrapperObject).prototype);
  ret.ptr = ptr;
  return cache[ptr] = ret;
}
Module['wrapPointer'] = wrapPointer;

b2World.prototype['CreateBody'] = b2World.prototype.CreateBody = /** @suppress {undefinedVars, duplicate} @this{Object} */function(def) {
  var self = this.ptr;
  if (def && typeof def === 'object') def = def.ptr;
  return wrapPointer(_emscripten_bind_b2World_CreateBody_1(self, def), b2Body);
};;

In your repro (create + destroy body, 30 times), I found that all 30 bodies that had the same pointer, 5367560. This means that you'll be given the same JS object each time too.

I think it's best to assume "I don't own this JS object", and store your metadata elsewhere. Box2D actually has a concept of userData (see b2Body.GetUserData() and b2BodyUserData), but that system requires modifying the WASM heap (e.g. with API calls into the C++ program). I've not exposed any way to modify that userData because it's difficult and not very useful (you'd be very limited in what kinds of information you could store on it).

Instead, let's make our own metadata system.

/** @type {Object.<number, unknown}} */
const metadata = {}

// make falling boxes
const boxCount = 30;
for (let i = 0; i < boxCount; i++) {
  const bd = new b2BodyDef();
  bd.set_type(b2_dynamicBody);
  bd.set_position(ZERO);
  const body = world.CreateBody(bd);
  // this condition doesn't get hit any more!
  if(body.createdBefore)
    debugger
  body.CreateFixture(i % 2 ? square : circle, 1);
  initPosition(body, i);

  // create metadata for this body
  metadata[getPointer(body)] = {
    createdBefore: true
  }
  
  // whenever we destroy a body, we should also destroy its metadata
  world.DestroyBody(body)
  delete metadata[getPointer(body)]
}

You could even skip the getPointer(body), and take advantage of the fact that b2Body references are reused:

/** @type {Object.<Box2D.b2Body, unknown}} */
const metadata = {}
metadata[body] = {
    createdBefore: true
}
world.DestroyBody(body)
delete metadata[body]

And you can even create objects that look like "the original b2Body, plus the metadata":

/** @type {Object.<Box2D.b2Body, Box2D.b2Body}} */
const hydratedInstances = {}
hydratedInstances[body] = Object.setPrototypeOf({
  createdBefore: true
}, body)

// you can retrieve metadata
console.log(hydratedInstances[body].createdBefore)
// you can retrieve data from the body itself too
console.log(hydratedInstances[body].GetMass())

world.DestroyBody(body)
delete hydratedInstances[body]

I think this "storing metadata outside of the instance" eliminates a lot of the complexity compared to "cleaning up instances that we know are going to be reused".

[Birch-san]

It was quite shocking to see a small memory leak pile up with it with the mainloop.

Recognising memory leaks in Box2D is difficult. If you world.CreateBody(), Box2D's small block allocator will grow the heap. But if you world.DestroyBody() afterwards, the small block allocator will designate the memory as reusable but will not shrink the heap.

This is not necessarily a memory leak; your next world.CreateBody() may be able to reuse that memory without growing the heap further.

Side-note: I'm not super convinced that Emscripten's destroy() returns to us all the memory that's allocated when we use new. I took some measurements and found that I didn't always get all of my bytes back. I don't have an explanation for that. maybe inefficiencies due to fragmentation or alignment? or just a mistake in how I'm measuring the heap (i.e. build box2d-wasm with heap growth disabled, and allocate memory until you can't). but either way, the inner workings of Emscripten are beyond the remit of this project.

[Birch-san]

I've added some new docs to describe these:

https://github.com/Birch-san/box2d-wasm/tree/master/docs#reference

• Managing user data
• Memory model

[hutzlibu]

So far, I still use Metadata in my Box2D objects, but since I recycle and clean them by myself, I have no issues.
There are still memory leaks somewhere, though, as it crashes after running for hours, but that is not a big concern for me atm.
(I am pretty sure, the leak is not in my code, as I keep track of every b2 object, but yes, could be somewhere inside emscripten)

I think I make a small example project with this approach and see if anyone finds it useful. It is nice to have most of the recycling stuff done automatically.

Something else, I decided to also compile it from source. Mainly because I do need to to change max_translation in the end, and I see if I can change the original code - so I can change it while running with a helper method.
(I need to change at runtime, because with higher max_translation, things can go odd, and to prevent this, you can change the max_translation back to normal for one render cycle and then back high.)
But also, because I want to try out chromes- wasm - sourcecode integration, meaning apparently you can step through your wasm code, and see the relevant lines in the C++ code, as well.

So I would be gracious for a few hints on how to set up the emscripten toolchain. Also, where would be a better place to discuss this?
(It is not really a issue, but I could also open one.)

[Birch-san]

I want to try out chromes- wasm - sourcecode integration, meaning apparently you can step through your wasm code, and see the relevant lines in the C++ code

it can be made to work (that's what TARGET_TYPE='RelWithDebInfo' and TARGET_TYPE='Debug' are for):

the library I publish is built with TARGET_TYPE='Release', because the debug variants blow up if you try to build with -flto link-time optimization. so you'd need to compile it yourself.

it's very fiddly to set up. the source maps are constructed relative to the folder in which you compile them, but you're expected to serve the source code relative to (I think) where Box2D.wasm.map is served.

hints on how to set up the emscripten toolchain. Also, where would be a better place to discuss this?

we could try out the GitHub Discussions beta:
#16