Skip to content

Commit

Permalink
[PERF beta] @each should remain a stable node for chains.
Browse files Browse the repository at this point in the history
`@each` was designed for chaining, the special cased eager behavior + it invalidating for array changes meant that the EachProxy and chains were rebuilt every array change, instead of the leaves just changing.
  • Loading branch information
krisselden committed Aug 5, 2015
1 parent 8ec08a7 commit 0a1f241
Show file tree
Hide file tree
Showing 18 changed files with 51 additions and 100 deletions.
15 changes: 0 additions & 15 deletions packages/ember-metal/lib/chains.js
Original file line number Diff line number Diff line change
Expand Up @@ -192,15 +192,6 @@ function ChainNode(parent, key, value) {
addChainWatcher(this._object, this._key, this);
}
}

// Special-case: the EachProxy relies on immediate evaluation to
// establish its observers.
//
// TODO: Replace this with an efficient callback that the EachProxy
// can implement.
if (this._parent && this._parent._key === '@each') {
this.value();
}
}

function lazyGet(obj, key) {
Expand Down Expand Up @@ -374,12 +365,6 @@ ChainNode.prototype = {
addChainWatcher(obj, this._key, this);
}
this._value = undefined;

// Special-case: the EachProxy relies on immediate evaluation to
// establish its observers.
if (this._parent && this._parent._key === '@each') {
this.value();
}
}

// then notify chains...
Expand Down
2 changes: 1 addition & 1 deletion packages/ember-metal/lib/expand_properties.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ var SPLIT_REGEX = /\{|\}/;
Ember.expandProperties('{foo,bar}', echo); //=> 'foo', 'bar'
Ember.expandProperties('foo.{bar,baz}', echo); //=> 'foo.bar', 'foo.baz'
Ember.expandProperties('{foo,bar}.baz', echo); //=> 'foo.baz', 'bar.baz'
Ember.expandProperties('foo.{bar,baz}.@each', echo) //=> 'foo.bar.@each', 'foo.baz.@each'
Ember.expandProperties('foo.{bar,baz}.[]', echo) //=> 'foo.bar.[]', 'foo.baz.[]'
Ember.expandProperties('{foo,bar}.{spam,eggs}', echo) //=> 'foo.spam', 'foo.eggs', 'bar.spam', 'bar.eggs'
Ember.expandProperties('{foo}.bar.{baz}') //=> 'foo.bar.baz'
```
Expand Down
5 changes: 3 additions & 2 deletions packages/ember-metal/tests/expand_properties_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ QUnit.test('Properties without expansions are unaffected', function() {

expandProperties('a', addProperty);
expandProperties('a.b', addProperty);
expandProperties('a.b.@each', addProperty);
expandProperties('a.b.[]', addProperty);
expandProperties('[email protected]', addProperty);

deepEqual(['a', 'a.b', 'a.b.@each'].sort(), foundProperties.sort());
deepEqual(['a', 'a.b', 'a.b.[]', 'a.b.@each.c'].sort(), foundProperties.sort());
});

QUnit.test('A single expansion at the end expands properly', function() {
Expand Down
29 changes: 7 additions & 22 deletions packages/ember-runtime/lib/system/each_proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import {
_removeBeforeObserver,
removeObserver
} from 'ember-metal/observer';
import { watchedEvents } from 'ember-metal/events';
import { defineProperty } from 'ember-metal/properties';
import {
beginPropertyChanges,
Expand All @@ -28,7 +27,6 @@ import {
} from 'ember-metal/property_events';

var EachArray = EmberObject.extend(EmberArray, {

init(attr) {
this._super(...arguments);
this._keyName = attr.keyName;
Expand All @@ -44,12 +42,9 @@ var EachArray = EmberObject.extend(EmberArray, {
length: computed(function() {
var content = this._content;
return content ? get(content, 'length') : 0;
})

}).volatile()
});

var IS_OBSERVER = /^.+:(before|change)$/;

function addObserverForContentKey(content, keyName, proxy, idx, loc) {
var objects = proxy._objects;
var guid;
Expand Down Expand Up @@ -113,9 +108,6 @@ var EachProxy = EmberObject.extend({

// in case someone is already observing some keys make sure they are
// added
watchedEvents(this).forEach((eventName) => {
this.didAddListener(eventName);
});
},

/**
Expand All @@ -133,8 +125,8 @@ var EachProxy = EmberObject.extend({
keyName: keyName,
owner: this
});
defineProperty(this, keyName, null, ret);
this.beginObservingContentKey(keyName);
defineProperty(this, keyName, null, undefined);
this.set(keyName, ret);
return ret;
},

Expand All @@ -157,7 +149,6 @@ var EachProxy = EmberObject.extend({
propertyWillChange(this, key);
}

propertyWillChange(this._content, '@each');
endPropertyChanges(this);
},

Expand All @@ -174,25 +165,19 @@ var EachProxy = EmberObject.extend({

propertyDidChange(this, key);
}

propertyDidChange(this._content, '@each');
}, this);
},

// ..........................................................
// LISTEN FOR NEW OBSERVERS AND OTHER EVENT LISTENERS
// Start monitoring keys based on who is listening...

didAddListener(eventName) {
if (IS_OBSERVER.test(eventName)) {
this.beginObservingContentKey(eventName.slice(0, -7));
}
willWatchProperty(property) {
this.beginObservingContentKey(property);
},

didRemoveListener(eventName) {
if (IS_OBSERVER.test(eventName)) {
this.stopObservingContentKey(eventName.slice(0, -7));
}
didUnwatchProperty(property) {
this.stopObservingContentKey(property);
},

// ..........................................................
Expand Down
20 changes: 0 additions & 20 deletions packages/ember-runtime/tests/mixins/array_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -327,26 +327,6 @@ QUnit.module('EmberArray.@each support', {
}
});

QUnit.test('adding an object should notify (@each)', function() {
var called = 0;

var observerObject = EmberObject.create({
wasCalled() {
called++;
}
});

// get(ary, '@each');
addObserver(ary, '@each', observerObject, 'wasCalled');

ary.addObject(EmberObject.create({
desc: 'foo',
isDone: false
}));

equal(called, 1, 'calls observer when object is pushed');
});

QUnit.test('adding an object should notify (@each.isDone)', function() {
var called = 0;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ suite.test('[A,B].addObject(C) => [A,B,C] + notify', function() {

if (observer.isEnabled) {
equal(observer.timesCalled('[]'), 1, 'should have notified [] once');
equal(observer.timesCalled('@each'), 1, 'should have notified @each once');
equal(observer.timesCalled('@each'), 0, 'should not have notified @each once');
equal(observer.timesCalled('length'), 1, 'should have notified length once');
equal(observer.timesCalled('lastObject'), 1, 'should have notified lastObject once');

Expand Down
2 changes: 1 addition & 1 deletion packages/ember-runtime/tests/suites/mutable_array/clear.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ suite.test('[X].clear() => [] + notify', function () {
equal(Ember.get(obj, 'length'), after.length, 'length');

equal(observer.timesCalled('[]'), 1, 'should have notified [] once');
equal(observer.timesCalled('@each'), 1, 'should have notified @each once');
equal(observer.timesCalled('@each'), 0, 'should not have notified @each once');
equal(observer.timesCalled('length'), 1, 'should have notified length once');
equal(observer.timesCalled('firstObject'), 1, 'should have notified firstObject once');
equal(observer.timesCalled('lastObject'), 1, 'should have notified lastObject once');
Expand Down
24 changes: 12 additions & 12 deletions packages/ember-runtime/tests/suites/mutable_array/insertAt.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ suite.test('[].insertAt(0, X) => [X] + notify', function() {


equal(observer.timesCalledBefore('[]'), 1, 'should have notified [] will change once');
equal(observer.timesCalledBefore('@each'), 1, 'should have notified @each will change once');
equal(observer.timesCalledBefore('@each'), 0, 'should not have notified @each will change once');
equal(observer.timesCalledBefore('length'), 1, 'should have notified length will change once');
equal(observer.timesCalledBefore('firstObject'), 1, 'should have notified firstObject will change once');
equal(observer.timesCalledBefore('lastObject'), 1, 'should have notified lastObject will change once');

equal(observer.timesCalled('[]'), 1, 'should have notified [] did change once');
equal(observer.timesCalled('@each'), 1, 'should have notified @each did change once');
equal(observer.timesCalled('@each'), 0, 'should not have notified @each did change once');
equal(observer.timesCalled('length'), 1, 'should have notified length did change once');
equal(observer.timesCalled('firstObject'), 1, 'should have notified firstObject did change once');
equal(observer.timesCalled('lastObject'), 1, 'should have notified lastObject did change once');
Expand Down Expand Up @@ -59,13 +59,13 @@ suite.test('[A].insertAt(0, X) => [X,A] + notify', function() {
equal(get(obj, 'length'), after.length, 'length');

equal(observer.timesCalledBefore('[]'), 1, 'should have notified [] will change once');
equal(observer.timesCalledBefore('@each'), 1, 'should have notified @each will change once');
equal(observer.timesCalledBefore('@each'), 0, 'should not have notified @each will change once');
equal(observer.timesCalledBefore('length'), 1, 'should have notified length will change once');
equal(observer.timesCalledBefore('firstObject'), 1, 'should have notified firstObject will change once');
equal(observer.timesCalledBefore('lastObject'), 0, 'should NOT have notified lastObject will change once');

equal(observer.timesCalled('[]'), 1, 'should have notified [] once');
equal(observer.timesCalled('@each'), 1, 'should have notified @each once');
equal(observer.timesCalled('@each'), 0, 'should not have notified @each once');
equal(observer.timesCalled('length'), 1, 'should have notified length once');
equal(observer.timesCalled('firstObject'), 1, 'should have notified firstObject once');

Expand All @@ -89,13 +89,13 @@ suite.test('[A].insertAt(1, X) => [A,X] + notify', function() {
equal(get(obj, 'length'), after.length, 'length');

equal(observer.timesCalledBefore('[]'), 1, 'should have notified [] will change once');
equal(observer.timesCalledBefore('@each'), 1, 'should have notified @each will change once');
equal(observer.timesCalledBefore('@each'), 0, 'should not have notified @each will change once');
equal(observer.timesCalledBefore('length'), 1, 'should have notified length will change once');
equal(observer.timesCalledBefore('firstObject'), 0, 'should NOT have notified firstObject will change once');
equal(observer.timesCalledBefore('lastObject'), 1, 'should have notified lastObject will change once');

equal(observer.timesCalled('[]'), 1, 'should have notified [] once');
equal(observer.timesCalled('@each'), 1, 'should have notified @each once');
equal(observer.timesCalled('@each'), 0, 'should not have notified @each once');
equal(observer.timesCalled('length'), 1, 'should have notified length once');
equal(observer.timesCalled('lastObject'), 1, 'should have notified lastObject once');

Expand Down Expand Up @@ -128,13 +128,13 @@ suite.test('[A,B,C].insertAt(0,X) => [X,A,B,C] + notify', function() {
equal(get(obj, 'length'), after.length, 'length');

equal(observer.timesCalledBefore('[]'), 1, 'should have notified [] will change once');
equal(observer.timesCalledBefore('@each'), 1, 'should have notified @each will change once');
equal(observer.timesCalledBefore('@each'), 0, 'should not have notified @each will change once');
equal(observer.timesCalledBefore('length'), 1, 'should have notified length will change once');
equal(observer.timesCalledBefore('firstObject'), 1, 'should have notified firstObject will change once');
equal(observer.timesCalledBefore('lastObject'), 0, 'should NOT have notified lastObject will change once');

equal(observer.timesCalled('[]'), 1, 'should have notified [] once');
equal(observer.timesCalled('@each'), 1, 'should have notified @each once');
equal(observer.timesCalled('@each'), 0, 'should not have notified @each once');
equal(observer.timesCalled('length'), 1, 'should have notified length once');
equal(observer.timesCalled('firstObject'), 1, 'should have notified firstObject once');

Expand All @@ -158,13 +158,13 @@ suite.test('[A,B,C].insertAt(1,X) => [A,X,B,C] + notify', function() {
equal(get(obj, 'length'), after.length, 'length');

equal(observer.timesCalledBefore('[]'), 1, 'should have notified [] will change once');
equal(observer.timesCalledBefore('@each'), 1, 'should have notified @each will change once');
equal(observer.timesCalledBefore('@each'), 0, 'should not have notified @each will change once');
equal(observer.timesCalledBefore('length'), 1, 'should have notified length will change once');
equal(observer.timesCalledBefore('firstObject'), 0, 'should NOT have notified firstObject will change once');
equal(observer.timesCalledBefore('lastObject'), 0, 'should NOT have notified lastObject will change once');

equal(observer.timesCalled('[]'), 1, 'should have notified [] once');
equal(observer.timesCalled('@each'), 1, 'should have notified @each once');
equal(observer.timesCalled('@each'), 0, 'should not have notified @each once');
equal(observer.timesCalled('length'), 1, 'should have notified length once');

equal(observer.validate('firstObject'), false, 'should NOT have notified firstObject');
Expand All @@ -188,13 +188,13 @@ suite.test('[A,B,C].insertAt(3,X) => [A,B,C,X] + notify', function() {
equal(get(obj, 'length'), after.length, 'length');

equal(observer.timesCalledBefore('[]'), 1, 'should have notified [] will change once');
equal(observer.timesCalledBefore('@each'), 1, 'should have notified @each will change once');
equal(observer.timesCalledBefore('@each'), 0, 'should not have notified @each will change once');
equal(observer.timesCalledBefore('length'), 1, 'should have notified length will change once');
equal(observer.timesCalledBefore('firstObject'), 0, 'should NOT have notified firstObject will change once');
equal(observer.timesCalledBefore('lastObject'), 1, 'should have notified lastObject will change once');

equal(observer.timesCalled('[]'), 1, 'should have notified [] once');
equal(observer.timesCalled('@each'), 1, 'should have notified @each once');
equal(observer.timesCalled('@each'), 0, 'should not have notified @each once');
equal(observer.timesCalled('length'), 1, 'should have notified length once');
equal(observer.timesCalled('lastObject'), 1, 'should have notified lastObject once');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ suite.test('[X].popObject() => [] + notify', function() {
equal(get(obj, 'length'), after.length, 'length');

equal(observer.timesCalled('[]'), 1, 'should have notified [] once');
equal(observer.timesCalled('@each'), 1, 'should have notified @each once');
equal(observer.timesCalled('@each'), 0, 'should not have notified @each once');
equal(observer.timesCalled('length'), 1, 'should have notified length once');
equal(observer.timesCalled('firstObject'), 1, 'should have notified firstObject once');
equal(observer.timesCalled('lastObject'), 1, 'should have notified lastObject once');
Expand All @@ -61,7 +61,7 @@ suite.test('[A,B,C].popObject() => [A,B] + notify', function() {
equal(get(obj, 'length'), after.length, 'length');

equal(observer.timesCalled('[]'), 1, 'should have notified [] once');
equal(observer.timesCalled('@each'), 1, 'should have notified @each once');
equal(observer.timesCalled('@each'), 0, 'should not have notified @each once');
equal(observer.timesCalled('length'), 1, 'should have notified length once');
equal(observer.timesCalled('lastObject'), 1, 'should have notified lastObject once');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ suite.test('[].pushObject(X) => [X] + notify', function() {
equal(get(obj, 'length'), after.length, 'length');

equal(observer.timesCalled('[]'), 1, 'should have notified [] once');
equal(observer.timesCalled('@each'), 1, 'should have notified @each once');
equal(observer.timesCalled('@each'), 0, 'should not have notified @each once');
equal(observer.timesCalled('length'), 1, 'should have notified length once');
equal(observer.timesCalled('firstObject'), 1, 'should have notified firstObject once');
equal(observer.timesCalled('lastObject'), 1, 'should have notified lastObject once');
Expand All @@ -48,7 +48,7 @@ suite.test('[A,B,C].pushObject(X) => [A,B,C,X] + notify', function() {
equal(get(obj, 'length'), after.length, 'length');

equal(observer.timesCalled('[]'), 1, 'should have notified [] once');
equal(observer.timesCalled('@each'), 1, 'should have notified @each once');
equal(observer.timesCalled('@each'), 0, 'should not have notified @each once');
equal(observer.timesCalled('length'), 1, 'should have notified length once');
equal(observer.timesCalled('lastObject'), 1, 'should have notified lastObject once');

Expand Down
10 changes: 5 additions & 5 deletions packages/ember-runtime/tests/suites/mutable_array/removeAt.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ suite.test('[X].removeAt(0) => [] + notify', function() {
equal(get(obj, 'length'), after.length, 'length');

equal(observer.timesCalled('[]'), 1, 'should have notified [] once');
equal(observer.timesCalled('@each'), 1, 'should have notified @each once');
equal(observer.timesCalled('@each'), 0, 'should not have notified @each once');
equal(observer.timesCalled('length'), 1, 'should have notified length once');
equal(observer.timesCalled('firstObject'), 1, 'should have notified firstObject once');
equal(observer.timesCalled('lastObject'), 1, 'should have notified lastObject once');
Expand Down Expand Up @@ -48,7 +48,7 @@ suite.test('[A,B].removeAt(0) => [B] + notify', function() {
equal(get(obj, 'length'), after.length, 'length');

equal(observer.timesCalled('[]'), 1, 'should have notified [] once');
equal(observer.timesCalled('@each'), 1, 'should have notified @each once');
equal(observer.timesCalled('@each'), 0, 'should not have notified @each once');
equal(observer.timesCalled('length'), 1, 'should have notified length once');
equal(observer.timesCalled('firstObject'), 1, 'should have notified firstObject once');

Expand All @@ -70,7 +70,7 @@ suite.test('[A,B].removeAt(1) => [A] + notify', function() {
equal(get(obj, 'length'), after.length, 'length');

equal(observer.timesCalled('[]'), 1, 'should have notified [] once');
equal(observer.timesCalled('@each'), 1, 'should have notified @each once');
equal(observer.timesCalled('@each'), 0, 'should not have notified @each once');
equal(observer.timesCalled('length'), 1, 'should have notified length once');
equal(observer.timesCalled('lastObject'), 1, 'should have notified lastObject once');

Expand All @@ -92,7 +92,7 @@ suite.test('[A,B,C].removeAt(1) => [A,C] + notify', function() {
equal(get(obj, 'length'), after.length, 'length');

equal(observer.timesCalled('[]'), 1, 'should have notified [] once');
equal(observer.timesCalled('@each'), 1, 'should have notified @each once');
equal(observer.timesCalled('@each'), 0, 'should not have notified @each once');
equal(observer.timesCalled('length'), 1, 'should have notified length once');

equal(observer.validate('firstObject'), false, 'should NOT have notified firstObject once');
Expand All @@ -114,7 +114,7 @@ suite.test('[A,B,C,D].removeAt(1,2) => [A,D] + notify', function() {
equal(get(obj, 'length'), after.length, 'length');

equal(observer.timesCalled('[]'), 1, 'should have notified [] once');
equal(observer.timesCalled('@each'), 1, 'should have notified @each once');
equal(observer.timesCalled('@each'), 0, 'should not have notified @each once');
equal(observer.timesCalled('length'), 1, 'should have notified length once');

equal(observer.validate('firstObject'), false, 'should NOT have notified firstObject once');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ suite.test('[A,B,C].removeObject(B) => [A,C] + notify', function() {

if (observer.isEnabled) {
equal(observer.timesCalled('[]'), 1, 'should have notified [] once');
equal(observer.timesCalled('@each'), 1, 'should have notified @each once');
equal(observer.timesCalled('@each'), 0, 'should not have notified @each once');
equal(observer.timesCalled('length'), 1, 'should have notified length once');

equal(observer.validate('firstObject'), false, 'should NOT have notified firstObject once');
Expand Down
Loading

0 comments on commit 0a1f241

Please sign in to comment.