Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX canary] Meta refactor #11966

Merged
merged 40 commits into from
Aug 5, 2015
Merged
Show file tree
Hide file tree
Changes from 36 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
a5dcce8
DRY up meta()
ef4 Aug 2, 2015
8133291
moving meta into its own module
ef4 Aug 2, 2015
3044547
make exports consistent
ef4 Aug 2, 2015
ab25153
notes-to-self on each meta property's access patterns
ef4 Aug 2, 2015
807b963
encapsulated meta.cache
ef4 Aug 2, 2015
31c16f0
generalize upgrade support
ef4 Aug 2, 2015
4bb9009
more progress on generic meta accessors
ef4 Aug 2, 2015
d885f23
encapsulated meta.watching
ef4 Aug 2, 2015
542765b
align semantics
ef4 Aug 2, 2015
a4c9dd3
lazy watchers
ef4 Aug 2, 2015
68895ec
encapsulated meta.mixins
ef4 Aug 2, 2015
aab8f9a
encapsulate meta.bindings
ef4 Aug 2, 2015
0b96990
encapsulated meta.values
ef4 Aug 2, 2015
bbfb0e7
typos
ef4 Aug 2, 2015
c40512a
extra reusable bits
ef4 Aug 2, 2015
bab0092
encapsulate meta.listeners
ef4 Aug 2, 2015
9b51c9e
encapsulate meta.deps
ef4 Aug 2, 2015
1fd3eb5
encapsulate meta.chainWatchers
ef4 Aug 3, 2015
a3f86cc
encapsulate meta.chains
ef4 Aug 3, 2015
93743c1
break the meta prototype chain!
ef4 Aug 3, 2015
d51a492
better naming
ef4 Aug 3, 2015
f216afc
better naming for inheritedMap accessors
ef4 Aug 3, 2015
a4a4854
better naming for inheritMapOfLists accessors
ef4 Aug 3, 2015
2d70d56
better naming for inheritedMapOfMaps accessors
ef4 Aug 3, 2015
d748879
better naming for ownCustomObject accessors
ef4 Aug 3, 2015
dbc712c
better naming for inheritedCustomObject accessors
ef4 Aug 3, 2015
8886eb7
commenting
ef4 Aug 3, 2015
45d73c1
spelling, how does it work?
ef4 Aug 3, 2015
26008e0
starting some new meta tests
ef4 Aug 3, 2015
c4b3bce
point everything at the new meta module
ef4 Aug 3, 2015
71ebf9c
fancy new listener data structures
ef4 Aug 3, 2015
84a0a50
remove outdated comment
ef4 Aug 3, 2015
7b5fcc0
micro-optimize every Object.create(null) to our own EmptyObject
ef4 Aug 3, 2015
c64b316
fix silly unnecessary defProp
ef4 Aug 3, 2015
09218cf
incorporating stef feedback
ef4 Aug 4, 2015
714f0b8
use empty object
ef4 Aug 4, 2015
580a40a
speed up Meta instantiation
stefanpenner Aug 4, 2015
a64e211
don’t event try the for loop if there are no actions
stefanpenner Aug 4, 2015
b6b0739
remove get for an internal property that will never be a CP
stefanpenner Aug 4, 2015
584fc73
Merge pull request #2 from stefanpenner/eds
ef4 Aug 5, 2015
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions packages/ember-application/lib/system/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import Ember from 'ember-metal'; // Ember.deprecate, Ember.assert, Ember.librari
import isEnabled from 'ember-metal/features';
import { get } from 'ember-metal/property_get';
import { set } from 'ember-metal/property_set';
import EmptyObject from 'ember-metal/empty_object';
import { runLoadHooks } from 'ember-runtime/system/lazy_load';
import Namespace from 'ember-runtime/system/namespace';
import DefaultResolver from 'ember-application/system/resolver';
Expand Down Expand Up @@ -722,8 +723,8 @@ if (isEnabled('ember-application-visit')) {
}

Application.reopenClass({
initializers: Object.create(null),
instanceInitializers: Object.create(null),
initializers: new EmptyObject(),
instanceInitializers: new EmptyObject(),

/**
Initializer receives an object which has the following attributes:
Expand Down
3 changes: 2 additions & 1 deletion packages/ember-htmlbars/lib/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@
@property helpers
*/
import Ember from 'ember-metal/core';
import EmptyObject from 'ember-metal/empty_object';

var helpers = Object.create(null);
var helpers = new EmptyObject();

/**
@module ember
Expand Down
10 changes: 4 additions & 6 deletions packages/ember-metal/lib/alias.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@ import {
defineProperty
} from 'ember-metal/properties';
import { ComputedProperty } from 'ember-metal/computed';
import {
meta,
inspect
} from 'ember-metal/utils';
import { inspect } from 'ember-metal/utils';
import { meta } from 'ember-metal/meta';
import {
addDependentKeys,
removeDependentKeys
Expand Down Expand Up @@ -47,14 +45,14 @@ AliasedProperty.prototype.didUnwatch = function(obj, keyName) {
AliasedProperty.prototype.setup = function(obj, keyName) {
Ember.assert(`Setting alias '${keyName}' on self`, this.altKey !== keyName);
var m = meta(obj);
if (m.watching[keyName]) {
if (m.peekWatching(keyName)) {
addDependentKeys(this, obj, keyName, m);
}
};

AliasedProperty.prototype.teardown = function(obj, keyName) {
var m = meta(obj);
if (m.watching[keyName]) {
if (m.peekWatching(keyName)) {
removeDependentKeys(this, obj, keyName, m);
}
};
Expand Down
40 changes: 20 additions & 20 deletions packages/ember-metal/lib/chains.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import Ember from 'ember-metal/core'; // warn, assert, etc;
import { get, normalizeTuple } from 'ember-metal/property_get';
import { meta as metaFor } from 'ember-metal/utils';
import { meta as metaFor } from 'ember-metal/meta';
import { watchKey, unwatchKey } from 'ember-metal/watch_key';
import EmptyObject from 'ember-metal/empty_object';

var FIRST_KEY = /^([^\.]+)/;

Expand All @@ -19,7 +20,7 @@ function isVolatile(obj) {

function Chains() { }

Chains.prototype = Object.create(null);
Chains.prototype = new EmptyObject();

function ChainWatchers(obj) {
// this obj would be the referencing chain node's parent node's value
Expand Down Expand Up @@ -131,19 +132,17 @@ export function flushPendingChains() {
);
}

function makeChainWatcher(obj) {
return new ChainWatchers(obj);
}

function addChainWatcher(obj, keyName, node) {
if (!isObject(obj)) {
return;
}

let m = metaFor(obj);

if (m.chainWatchers === undefined || m.chainWatchers.obj !== obj) {
m.chainWatchers = new ChainWatchers(obj);
}

m.chainWatchers.add(keyName, node);

m.writableChainWatchers(makeChainWatcher).add(keyName, node);
watchKey(obj, keyName, m);
}

Expand All @@ -154,15 +153,14 @@ function removeChainWatcher(obj, keyName, node) {

let m = obj.__ember_meta__;

if (!m ||
m.chainWatchers === undefined || m.chainWatchers.obj !== obj) {
if (!m || !m.readableChainWatchers()) {
return;
}

// make meta writable
m = metaFor(obj);

m.chainWatchers.remove(keyName, node);
m.readableChainWatchers().remove(keyName, node);

unwatchKey(obj, keyName, m);
}
Expand Down Expand Up @@ -222,8 +220,9 @@ function lazyGet(obj, key) {
return get(obj, key);
// Otherwise attempt to get the cached value of the computed property
} else {
if (meta.cache && key in meta.cache) {
return meta.cache[key];
let cache = meta.readableCache();
if (cache && key in cache) {
return cache[key];
}
}
}
Expand Down Expand Up @@ -423,16 +422,17 @@ export function finishChains(obj) {
// We only create meta if we really have to
let m = obj.__ember_meta__;
if (m) {
m = metaFor(obj);

// finish any current chains node watchers that reference obj
let chainWatchers = m.chainWatchers;
let chainWatchers = m.readableChainWatchers();
if (chainWatchers) {
chainWatchers.revalidateAll();
}
// copy chains from prototype
let chains = m.chains;
if (chains && chains.value() !== obj) {
// need to check if meta is writable
metaFor(obj).chains = chains.copy(obj);
// ensure that if we have inherited any chains they have been
// copied onto our own meta.
if (m.readableChains()) {
m.writableChains();
}
}
}
Expand Down
43 changes: 20 additions & 23 deletions packages/ember-metal/lib/computed.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import Ember from 'ember-metal/core';
import { set } from 'ember-metal/property_set';
import {
meta,
inspect
} from 'ember-metal/utils';
import { inspect } from 'ember-metal/utils';
import { meta } from 'ember-metal/meta';
import expandProperties from 'ember-metal/expand_properties';
import EmberError from 'ember-metal/error';
import {
Expand Down Expand Up @@ -266,9 +264,10 @@ ComputedPropertyPrototype.didChange = function(obj, keyName) {
// _suspended is set via a CP.set to ensure we don't clear
// the cached value set by the setter
if (this._cacheable && this._suspended !== obj) {
var meta = metaFor(obj);
if (meta.cache && meta.cache[keyName] !== undefined) {
meta.cache[keyName] = undefined;
let meta = metaFor(obj);
let cache = meta.readableCache();
if (cache && cache[keyName] !== undefined) {
cache[keyName] = undefined;
removeDependentKeys(this, obj, keyName, meta);
}
}
Expand Down Expand Up @@ -305,9 +304,9 @@ ComputedPropertyPrototype.get = function(obj, keyName) {
var ret, cache, meta;
if (this._cacheable) {
meta = metaFor(obj);
cache = meta.cache;
cache = meta.writableCache();

var result = cache && cache[keyName];
var result = cache[keyName];

if (result === UNDEFINED) {
return undefined;
Expand All @@ -316,18 +315,16 @@ ComputedPropertyPrototype.get = function(obj, keyName) {
}

ret = this._getter.call(obj, keyName);
cache = meta.cache;
if (!cache) {
cache = meta.cache = {};
}

if (ret === undefined) {
cache[keyName] = UNDEFINED;
} else {
cache[keyName] = ret;
}

if (meta.chainWatchers) {
meta.chainWatchers.revalidate(keyName);
let chainWatchers = meta.readableChainWatchers();
if (chainWatchers) {
chainWatchers.revalidate(keyName);
}
addDependentKeys(this, obj, keyName, meta);
} else {
Expand Down Expand Up @@ -401,7 +398,7 @@ ComputedPropertyPrototype._set = function computedPropertySet(obj, keyName, valu
var cacheable = this._cacheable;
var setter = this._setter;
var meta = metaFor(obj, cacheable);
var cache = meta.cache;
var cache = meta.readableCache();
var hadCachedValue = false;

var cachedValue, ret;
Expand All @@ -427,7 +424,7 @@ ComputedPropertyPrototype._set = function computedPropertySet(obj, keyName, valu

if (hadCachedValue && cachedValue === ret) { return; }

var watched = meta.watching[keyName];
var watched = meta.peekWatching(keyName);
if (watched) {
propertyWillChange(obj, keyName);
}
Expand All @@ -441,7 +438,7 @@ ComputedPropertyPrototype._set = function computedPropertySet(obj, keyName, valu
addDependentKeys(this, obj, keyName, meta);
}
if (!cache) {
cache = meta.cache = {};
cache = meta.writableCache();
}
if (ret === undefined) {
cache[keyName] = UNDEFINED;
Expand All @@ -460,13 +457,13 @@ ComputedPropertyPrototype._set = function computedPropertySet(obj, keyName, valu
/* called before property is overridden */
ComputedPropertyPrototype.teardown = function(obj, keyName) {
var meta = metaFor(obj);

if (meta.cache) {
if (keyName in meta.cache) {
let cache = meta.readableCache();
if (cache) {
if (keyName in cache) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if I recall, the cache will never have an undefined member representing the value of undefined, rather it will use the UNDEFINED sentinel value. This means, we can likely just do (the unfortunately faster) if (cache[keyName] !== undefined)

removeDependentKeys(this, obj, keyName, meta);
}

if (this._cacheable) { delete meta.cache[keyName]; }
if (this._cacheable) { delete cache[keyName]; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

given the above it my be worth investigating cache[keyName] = undefined We will leak keys, but they should be bounded.

}

return null; // no value to restore
Expand Down Expand Up @@ -560,7 +557,7 @@ export default function computed(func) {
*/
function cacheFor(obj, key) {
var meta = obj['__ember_meta__'];
var cache = meta && meta.cache;
var cache = meta && meta.readableCache();
var ret = cache && cache[key];

if (ret === UNDEFINED) {
Expand Down
41 changes: 4 additions & 37 deletions packages/ember-metal/lib/dependent_keys.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,50 +17,19 @@ import {
// DEPENDENT KEYS
//

// data structure:
// meta.deps = {
// 'depKey': {
// 'keyName': count,
// }
// }

/*
This function returns a map of unique dependencies for a
given object and key.
*/
function keysForDep(depsMeta, depKey) {
var keys = depsMeta[depKey];
if (!keys) {
// if there are no dependencies yet for a the given key
// create a new empty list of dependencies for the key
keys = depsMeta[depKey] = {};
} else if (!depsMeta.hasOwnProperty(depKey)) {
// otherwise if the dependency list is inherited from
// a superclass, clone the hash
keys = depsMeta[depKey] = Object.create(keys);
}
return keys;
}

function metaForDeps(meta) {
return keysForDep(meta, 'deps');
}

export function addDependentKeys(desc, obj, keyName, meta) {
// the descriptor has a list of dependent keys, so
// add all of its dependent keys.
var depsMeta, idx, len, depKey, keys;
var idx, len, depKey, keys;
var depKeys = desc._dependentKeys;
if (!depKeys) {
return;
}

depsMeta = metaForDeps(meta);

for (idx = 0, len = depKeys.length; idx < len; idx++) {
depKey = depKeys[idx];
// Lookup keys meta for depKey
keys = keysForDep(depsMeta, depKey);
keys = meta.writableDeps(depKey);
// Increment the number of times depKey depends on keyName.
keys[keyName] = (keys[keyName] || 0) + 1;
// Watch the depKey
Expand All @@ -72,17 +41,15 @@ export function removeDependentKeys(desc, obj, keyName, meta) {
// the descriptor has a list of dependent keys, so
// remove all of its dependent keys.
var depKeys = desc._dependentKeys;
var depsMeta, idx, len, depKey, keys;
var idx, len, depKey, keys;
if (!depKeys) {
return;
}

depsMeta = metaForDeps(meta);

for (idx = 0, len = depKeys.length; idx < len; idx++) {
depKey = depKeys[idx];
// Lookup keys meta for depKey
keys = keysForDep(depsMeta, depKey);
keys = meta.writableDeps(depKey);
// Decrement the number of times depKey depends on keyName.
keys[keyName] = (keys[keyName] || 0) - 1;
// Unwatch the depKey
Expand Down
19 changes: 19 additions & 0 deletions packages/ember-metal/lib/empty_object.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// This exists because `Object.create(null)` is absurdly slow compared
// to `new EmptyObject()`. In either case, you want a null prototype
// when you're treating the object instances as arbitrary dictionaries
// and don't want your keys colliding with build-in methods on the
// default object prototype.

var proto = Object.create(null, {
// without this, we will always still end up with (new
// EmptyObject()).constructor === Object
constructor: {
value: undefined,
enumerable: false,
writable: true
}
});

function EmptyObject() {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

slightly more concise version is possible

export default function EmptyObject() {

}

EmptyObject.prototype = null

EmptyObject.prototype = proto;
export default EmptyObject;
Loading