Skip to content

Commit

Permalink
extern the methods of LeakMitigator, so Closure doesn't eliminate the…
Browse files Browse the repository at this point in the history
…m. bind methods. use Array.from to get access to forEach method. prove LeakMitigator in integration test. catch the leaked NULL. drop source-map-support. update ts-node. ignore WASM maps.
  • Loading branch information
Birch-san committed Nov 28, 2021
1 parent 7ba2ed0 commit fdfdacb
Show file tree
Hide file tree
Showing 7 changed files with 158 additions and 85 deletions.
4 changes: 1 addition & 3 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,7 @@
"--loader",
"ts-node/esm",
"--experimental-specifier-resolution=node",
"--harmony",
"-r",
"source-map-support/register"
"--harmony"
],
"skipFiles": [
"<node_internals>/**"
Expand Down
4 changes: 4 additions & 0 deletions box2d-wasm/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,15 @@
/dist/Box2D.d.ts
/dist/es/Box2D.js
/dist/es/Box2D.wasm
/dist/es/Box2D.wasm.map
/dist/es/Box2D.simd.js
/dist/es/Box2D.simd.wasm
/dist/es/Box2D.simd.wasm.map
/dist/umd/Box2D.js
/dist/umd/Box2D.wasm
/dist/umd/Box2D.wasm.map
/dist/umd/Box2D.simd.js
/dist/umd/Box2D.simd.wasm
/dist/umd/Box2D.simd.wasm.map
# integration test for AMD modules which I didn't feel like putting anywhere more formal
/amd/
120 changes: 64 additions & 56 deletions box2d-wasm/glue_stub.js
Original file line number Diff line number Diff line change
Expand Up @@ -454,62 +454,70 @@ Module['allocateArray'] = (ctor, elementSizeBytes, elements=1) => {
* // delete the reference to the `ground` JS object in b2Body's __cache__
* freeFromCache(ground);
*/
Module['LeakMitigator'] = class LeakMitigator {
constructor() {
this.instances = new Map()
}
function LeakMitigator() {
this.instances = new Map()
this['recordLeak'] = this['recordLeak'].bind(this);
this['safeWrapPointer'] = this['safeWrapPointer'].bind(this);
this['freeLeaked'] = this['freeLeaked'].bind(this);
}
LeakMitigator.prototype.constructor = LeakMitigator;

LeakMitigator['freeFromCache'] =
/**
* Convenience method to free an object from an Emscripten class's __cache__
*/
function freeFromCache(
instance,
b2Class = Module['getClass'](instance)
) {
const cache = Module['getCache'](b2Class)
delete cache[Module['getPointer'](instance)]
}

LeakMitigator.prototype['recordLeak'] =
/**
* wrap this around any Emscripten method which returns an object.
* records the instance, so that we can free it from cache later
*/
function recordLeak(
instance,
b2Class = Module['getClass'](instance)
) {
const instances = this.instances.get(b2Class) ?? new Set()
instances.add(instance)
this.instances.set(b2Class, instances)
return instance
}

LeakMitigator.prototype['safeWrapPointer'] =
/**
* prefer this over {@link Box2D.wrapPointer}.
* records the instance that's created, so that we can free it from cache later
*/
function safeWrapPointer(
pointer,
targetType
) {
return this.recordLeak(
Module['wrapPointer'](pointer, targetType),
targetType
)
}

/**
* Convenience method to free an object from an Emscripten class's __cache__
*/
static freeFromCache(
instance,
b2Class = Module['getClass'](instance)
) {
LeakMitigator.prototype['freeLeaked'] =
/**
* access the cache structure of each Emscripten class for which we recorded instances,
* then free from cache every instance that we recorded.
*/
function freeLeaked() {
// using Array#forEach because acorn optimizer couldn't parse the for..of
Array.from(this.instances.entries()).forEach(([b2Class, instances]) => {
const cache = Module['getCache'](b2Class)
delete cache[Module['getPointer'](instance)]
}

/**
* wrap this around any Emscripten method which returns an object.
* records the instance, so that we can free it from cache later
*/
recordLeak(
instance,
b2Class = Module['getClass'](instance)
) {
const instances = this.instances.get(b2Class) ?? new Set()
instances.add(instance)
this.instances.set(b2Class, instances)
return instance
}

/**
* prefer this over {@link Box2D.wrapPointer}.
* records the instance that's created, so that we can free it from cache later
*/
safeWrapPointer(
pointer,
targetType
) {
return this.recordLeak(
Module['wrapPointer'](pointer, targetType),
targetType
)
}
for (const instance of instances) {
delete cache[Module['getPointer'](instance)]
}
})
this.instances.clear()
};

/**
* access the cache structure of each Emscripten class for which we recorded instances,
* then free from cache every instance that we recorded.
*/
freeLeaked() {
// using Array#forEach because acorn optimizer couldn't parse the for..of
this.instances.entries().forEach(([b2Class, instances]) => {
const cache = Module['getCache'](b2Class)
for (const instance of instances) {
delete cache[Module['getPointer'](instance)]
}
})
this.instances.clear()
}
}
Module['LeakMitigator'] = LeakMitigator
26 changes: 20 additions & 6 deletions docs/memory-model.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,16 @@ destroy(tmp);

world.Step(1/60, 1, 1);

// strictly speaking we could consider wrapping world#GetBodyList() and body#GetNext()
// with recordLeak() (because they're methods which return JS objects)
// but we already recorded these bodies in our LeakMitigator when we received them
// via world#CreateBody()
for (let body = world.GetBodyList(); getPointer(body) !== getPointer(NULL); body = body.GetNext()) {
// we wrap world#GetBodyList() and body#GetNext() with recordLeak() because they're
// methods which return JS objects.
// some of these bodies are already recorded in our LeakMitigator (because we recorded the body
// leaked by world#CreateBody()). LeakMitigator ignores duplicates, so this is fine.
// the list ends with a reference to NULL. this is reference leaks too, so we record it.
for (
let body = recordLeak(world.GetBodyList());
getPointer(body) !== getPointer(NULL);
body = recordLeak(body.GetNext())
) {
// what happens when we invoke a getter which returns an object?
// that's right, we need recordLeak().
// but we don't need `destroy()` (this b2Vec2 wasn't created via `new`).
Expand Down Expand Up @@ -157,7 +162,16 @@ destroy(bd_ground)

// fast-forward to later, where we tear down the Box2D experiment...

for (let body = world.GetBodyList(); getPointer(body) !== getPointer(NULL); body = body.GetNext()) {
// we wrap world#GetBodyList() and body#GetNext() with recordLeak() because they're
// methods which return JS objects.
// some of these bodies are already recorded in our LeakMitigator (because we recorded the body
// leaked by world#CreateBody()). LeakMitigator ignores duplicates, so this is fine.
// the list ends with a reference to NULL. this is reference leaks too, so we record it.
for (
let body = recordLeak(world.GetBodyList());
getPointer(body) !== getPointer(NULL);
body = recordLeak(body.GetNext())
) {
// this b2Body was created with b2World#CreateBody(), so Box2D manages the memory, not us.
// we should not use destroy(body). instead we should use b2World#DestroyBody()
// this also destroys all fixtures on the body.
Expand Down
5 changes: 2 additions & 3 deletions integration-test-backend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"type": "module",
"main": "dist/index.js",
"scripts": {
"start": "node --loader ts-node/esm --experimental-specifier-resolution=node --harmony -r source-map-support/register src/index.ts",
"start": "node --loader ts-node/esm --experimental-specifier-resolution=node --harmony src/index",
"build": "tsc",
"test": "echo \"Error: no test specified\" && exit 1",
"lint": "eslint src/**/*.ts"
Expand All @@ -23,8 +23,7 @@
"@typescript-eslint/eslint-plugin": "^4.4.1",
"@typescript-eslint/parser": "^4.4.1",
"eslint": "^7.11.0",
"source-map-support": "^0.5.19",
"ts-node": "^10.0.0",
"ts-node": "^10.4.0",
"typescript": "^4.3.0"
},
"dependencies": {
Expand Down
31 changes: 28 additions & 3 deletions integration-test-backend/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,20 @@
import Box2DFactory from 'box2d-wasm';
import { assertFloatEqual } from './assertFloatEqual';

const { b2BodyDef, b2_dynamicBody, b2PolygonShape, b2Vec2, b2World }: typeof Box2D & EmscriptenModule = await Box2DFactory();
const {
b2BodyDef,
b2_dynamicBody,
b2PolygonShape,
b2Vec2,
b2World,
destroy,
getPointer,
LeakMitigator,
NULL
}: typeof Box2D & EmscriptenModule = await Box2DFactory();

const { freeFromCache } = LeakMitigator;
const { recordLeak, freeLeaked } = new LeakMitigator();

const gravity = new b2Vec2(0, 10);
const world = new b2World(gravity);
Expand All @@ -16,10 +29,13 @@ const bd = new b2BodyDef();
bd.set_type(b2_dynamicBody);
bd.set_position(zero);

const body = world.CreateBody(bd);
body.CreateFixture(square, 1);
const body = recordLeak(world.CreateBody(bd));
destroy(bd);
freeFromCache(body.CreateFixture(square, 1));
destroy(square);
body.SetTransform(zero, 0);
body.SetLinearVelocity(zero);
destroy(zero);
body.SetAwake(true);
body.SetEnabled(true);

Expand All @@ -42,4 +58,13 @@ for (let i=0; i<iterations; i++) {
world.Step(timeStepMillis, velocityIterations, positionIterations);
}

destroy(gravity);

for (let body = recordLeak(world.GetBodyList()); getPointer(body) !== getPointer(NULL); body = recordLeak(body.GetNext())) {
world.DestroyBody(body);
}

destroy(world);
freeLeaked();

console.log(`👍 Ran ${iterations} iterations of a falling body. Body had the expected position on each iteration.`);
53 changes: 39 additions & 14 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit fdfdacb

Please sign in to comment.