Skip to content

Commit

Permalink
[CLEANUP] Leverage native weakmaps
Browse files Browse the repository at this point in the history
  • Loading branch information
sivakumar-kailasam committed Nov 29, 2017
1 parent 78cb048 commit dfbdde1
Show file tree
Hide file tree
Showing 14 changed files with 31 additions and 359 deletions.
1 change: 0 additions & 1 deletion features.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
"features-stripped-test": null,
"ember-libraries-isregistered": null,
"ember-improved-instrumentation": null,
"ember-metal-weakmap": null,
"ember-glimmer-allow-backtracking-rerender": null,
"ember-routing-router-service": true,
"ember-engines-mount-params": true,
Expand Down
3 changes: 1 addition & 2 deletions packages/ember-glimmer/lib/utils/references.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import {
watchKey,
} from 'ember-metal';
import {
HAS_NATIVE_WEAKMAP,
symbol,
} from 'ember-utils';
import {
Expand All @@ -50,7 +49,7 @@ if (DEBUG) {
// performance penalty on Chrome (tested through 59).
//
// See: https://bugs.chromium.org/p/v8/issues/detail?id=6450
if (!Object.isFrozen(obj) && HAS_NATIVE_WEAKMAP) {
if (!Object.isFrozen(obj)) {
Object.freeze(obj);
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import { RenderingTest, moduleFor } from '../../utils/test-case';
import { runDestroy } from 'internal-test-helpers';
import { set } from 'ember-metal';
import { HAS_NATIVE_WEAKMAP } from 'ember-utils';

let assert = QUnit.assert;

Expand Down Expand Up @@ -634,7 +633,7 @@ let addingPropertyToFrozenObjectThrows = (() => {
}
})();

if (!EmberDev.runningProdBuild && HAS_NATIVE_WEAKMAP && (
if (!EmberDev.runningProdBuild && (
pushingIntoFrozenArrayThrows ||
assigningExistingFrozenPropertyThrows ||
addingPropertyToFrozenObjectThrows
Expand Down
1 change: 0 additions & 1 deletion packages/ember-metal/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ export {
set,
trySet
} from './property_set';
export { default as WeakMap, WeakMapPolyfill } from './weak_map';
export {
addListener,
hasListeners,
Expand Down
71 changes: 24 additions & 47 deletions packages/ember-metal/lib/meta.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {
HAS_NATIVE_WEAKMAP,
lookupDescriptor,
symbol,
toString
Expand Down Expand Up @@ -460,53 +459,35 @@ if (MANDATORY_SETTER) {
};
}

let setMeta, peekMeta;

// choose the one appropriate for given platform
if (HAS_NATIVE_WEAKMAP) {
let getPrototypeOf = Object.getPrototypeOf;
let metaStore = new WeakMap();
let getPrototypeOf = Object.getPrototypeOf;
let metaStore = new WeakMap();

setMeta = function WeakMap_setMeta(obj, meta) {
if (DEBUG) {
counters.setCalls++;
}
metaStore.set(obj, meta);
};

peekMeta = function WeakMap_peekParentMeta(obj) {
let pointer = obj;
let meta;
while (pointer !== undefined && pointer !== null) {
meta = metaStore.get(pointer);
// jshint loopfunc:true
if (DEBUG) {
counters.peekCalls++;
}
if (meta !== undefined) {
return meta;
}
export function setMeta(obj, meta) {
if (DEBUG) {
counters.setCalls++;
}
metaStore.set(obj, meta);
}

pointer = getPrototypeOf(pointer);
if (DEBUG) {
counters.peekPrototypeWalks++;
}
export function peekMeta(obj) {
let pointer = obj;
let meta;
while (pointer !== undefined && pointer !== null) {
meta = metaStore.get(pointer);
// jshint loopfunc:true
if (DEBUG) {
counters.peekCalls++;
}
};
} else {
setMeta = function Fallback_setMeta(obj, meta) {
if (obj.__defineNonEnumerable) {
obj.__defineNonEnumerable(EMBER_META_PROPERTY);
} else {
Object.defineProperty(obj, META_FIELD, META_DESC);
if (meta !== undefined) {
return meta;
}

obj[META_FIELD] = meta;
};

peekMeta = function Fallback_peekMeta(obj) {
return obj[META_FIELD];
};
pointer = getPrototypeOf(pointer);
if (DEBUG) {
counters.peekPrototypeWalks++;
}
}
}

/**
Expand Down Expand Up @@ -569,8 +550,4 @@ export function meta(obj) {
return newMeta;
}

export {
peekMeta,
setMeta,
counters
};
export { counters };
51 changes: 4 additions & 47 deletions packages/ember-metal/lib/transaction.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import { meta as metaFor } from './meta';
import WeakMap from './weak_map';
import { HAS_NATIVE_WEAKMAP } from 'ember-utils';
import { assert, deprecate } from 'ember-debug';
import { DEBUG } from 'ember-env-flags';
import {
Expand All @@ -14,27 +12,16 @@ let runInTransaction, didRender, assertNotRendered;
// detect-glimmer-allow-backtracking-rerender can be enabled in custom builds
if (EMBER_GLIMMER_DETECT_BACKTRACKING_RERENDER || EMBER_GLIMMER_ALLOW_BACKTRACKING_RERENDER) {

// there are 4 states
// there are 2 states

// NATIVE WEAKMAP AND DEBUG
// DEBUG
// tracks lastRef and lastRenderedIn per rendered object and key during a transaction
// release everything via normal weakmap semantics by just derefencing the weakmap

// NATIVE WEAKMAP AND RELEASE
// RELEASE
// tracks transactionId per rendered object and key during a transaction
// release everything via normal weakmap semantics by just derefencing the weakmap

// WEAKMAP POLYFILL AND DEBUG
// tracks lastRef and lastRenderedIn per rendered object and key during a transaction
// since lastRef retains a lot of app state (will have a ref to the Container)
// if the object rendered is retained (like a immutable POJO in module state)
// during acceptance tests this adds up and obfuscates finding other leaks.

// WEAKMAP POLYFILL AND RELEASE
// tracks transactionId per rendered object and key during a transaction
// leaks it because small and likely not worth tracking it since it will only
// be leaked if the object is retained

class TransactionRunner {
constructor() {
this.transactionId = 0;
Expand All @@ -44,12 +31,6 @@ if (EMBER_GLIMMER_DETECT_BACKTRACKING_RERENDER || EMBER_GLIMMER_ALLOW_BACKTRACKI
if (DEBUG) {
// track templates
this.debugStack = undefined;

if (!HAS_NATIVE_WEAKMAP) {
// DEBUG AND POLYFILL
// needs obj tracking
this.objs = [];
}
}
}

Expand Down Expand Up @@ -137,11 +118,6 @@ if (EMBER_GLIMMER_DETECT_BACKTRACKING_RERENDER || EMBER_GLIMMER_ALLOW_BACKTRACKI
createMap(object) {
let map = Object.create(null);
this.weakMap.set(object, map);
if (DEBUG && !HAS_NATIVE_WEAKMAP) {
// POLYFILL AND DEBUG
// requires tracking objects
this.objs.push(object);
}
return map;
}

Expand All @@ -166,26 +142,7 @@ if (EMBER_GLIMMER_DETECT_BACKTRACKING_RERENDER || EMBER_GLIMMER_ALLOW_BACKTRACKI
}

clearObjectMap() {
if (HAS_NATIVE_WEAKMAP) {
// NATIVE AND (DEBUG OR RELEASE)
// if we have a real native weakmap
// releasing the ref will allow the values to be GCed
this.weakMap = new WeakMap();
} else if (DEBUG) {
// POLYFILL AND DEBUG
// with a polyfill the weakmap keys must be cleared since
// they have the last reference, acceptance tests will leak
// the container if you render a immutable object retained
// in module scope.
let { objs, weakMap } = this;
this.objs = [];
for (let i = 0; i < objs.length; i++) {
weakMap.delete(objs[i]);
}
}
// POLYFILL AND RELEASE
// we leak the key map if the object is retained but this is
// a POJO of keys to transaction ids
this.weakMap = new WeakMap();
}
}

Expand Down
131 changes: 0 additions & 131 deletions packages/ember-metal/lib/weak_map.js

This file was deleted.

Loading

0 comments on commit dfbdde1

Please sign in to comment.