From 69c3bff08624211504c0ee8981d28f84f15eb52a Mon Sep 17 00:00:00 2001 From: vsavkin Date: Wed, 1 Apr 2015 15:49:14 -0700 Subject: [PATCH] feat(change_detection): updated change detection to update directive directly, without the dispatcher --- .../change_detection_jit_generator.dart | 2 +- .../change_detection_jit_generator.es6 | 83 ++++-- .../dynamic_change_detector.js | 46 ++-- .../src/change_detection/interfaces.js | 9 +- modules/angular2/src/core/compiler/view.js | 37 ++- .../change_detection/change_detection_spec.js | 243 +++++++++++------- modules/benchmarks/e2e_test/tree_perf.es6 | 22 ++ .../change_detection_benchmark.js | 57 ++-- 8 files changed, 322 insertions(+), 177 deletions(-) diff --git a/modules/angular2/src/change_detection/change_detection_jit_generator.dart b/modules/angular2/src/change_detection/change_detection_jit_generator.dart index 31db7530ab..6dbc791f0f 100644 --- a/modules/angular2/src/change_detection/change_detection_jit_generator.dart +++ b/modules/angular2/src/change_detection/change_detection_jit_generator.dart @@ -5,6 +5,6 @@ class ChangeDetectorJITGenerator { } generate() { - throw "Not supported in Dart"; + throw "Jit Change Detection is not supported in Dart"; } } \ No newline at end of file diff --git a/modules/angular2/src/change_detection/change_detection_jit_generator.es6 b/modules/angular2/src/change_detection/change_detection_jit_generator.es6 index 6b3601b89f..80bb346443 100644 --- a/modules/angular2/src/change_detection/change_detection_jit_generator.es6 +++ b/modules/angular2/src/change_detection/change_detection_jit_generator.es6 @@ -76,16 +76,23 @@ function pipeOnDestroyTemplate(pipeNames:List) { return pipeNames.map((p) => `${p}.onDestroy()`).join("\n"); } -function hydrateTemplate(type:string, mode:string, fieldsDefinitions:string, pipeOnDestroy:string):string { +function hydrateTemplate(type:string, mode:string, fieldDefinitions:string, pipeOnDestroy:string, + directiveFieldNames:List):string { + var directiveInit = ""; + for(var i = 0; i < directiveFieldNames.length; ++i) { + directiveInit += `${directiveFieldNames[i]} = this.directiveMementos[${i}].directive(directives);\n`; + } + return ` -${type}.prototype.hydrate = function(context, locals) { +${type}.prototype.hydrate = function(context, locals, directives) { ${MODE_ACCESSOR} = "${mode}"; ${CONTEXT_ACCESSOR} = context; ${LOCALS_ACCESSOR} = locals; + ${directiveInit} } ${type}.prototype.dehydrate = function() { ${pipeOnDestroy} - ${fieldsDefinitions} + ${fieldDefinitions} ${LOCALS_ACCESSOR} = null; } ${type}.prototype.hydrated = function() { @@ -110,8 +117,8 @@ ${type}.prototype.callOnAllChangesDone = function() { `; } -function onAllChangesDoneTemplate(index:number):string { - return `${DISPATCHER_ACCESSOR}.onAllChangesDone(${MEMENTOS_ACCESSOR}[${index}]);`; +function onAllChangesDoneTemplate(directive:string):string { + return `${directive}.onAllChangesDone();`; } @@ -130,7 +137,7 @@ ${records} } function pipeCheckTemplate(protoIndex:number, context:string, bindingPropagationConfig:string, pipe:string, pipeType:string, - oldValue:string, newValue:string, change:string, invokeMementoAndAddChange:string, + oldValue:string, newValue:string, change:string, invokeMemento:string, addToChanges, lastInDirective:string):string{ return ` ${CURRENT_PROTO} = ${PROTOS_ACCESSOR}[${protoIndex}]; @@ -144,7 +151,7 @@ if (${pipe} === ${UTIL}.unitialized()) { ${newValue} = ${pipe}.transform(${context}); if (! ${UTIL}.noChangeMarker(${newValue})) { ${change} = true; - ${invokeMementoAndAddChange} + ${invokeMemento} ${addToChanges} ${oldValue} = ${newValue}; } @@ -153,13 +160,13 @@ ${lastInDirective} } function referenceCheckTemplate(protoIndex:number, assignment:string, oldValue:string, newValue:string, change:string, - invokeMementoAndAddChange:string, addToChanges:string, lastInDirective:string):string { + invokeMemento:string, addToChanges:string, lastInDirective:string):string { return ` ${CURRENT_PROTO} = ${PROTOS_ACCESSOR}[${protoIndex}]; ${assignment} if (${newValue} !== ${oldValue} || (${newValue} !== ${newValue}) && (${oldValue} !== ${oldValue})) { ${change} = true; - ${invokeMementoAndAddChange} + ${invokeMemento} ${addToChanges} ${oldValue} = ${newValue}; } @@ -196,20 +203,26 @@ function addToChangesTemplate(oldValue:string, newValue:string):string { return `${CHANGES_LOCAL} = ${UTIL}.addChange(${CHANGES_LOCAL}, ${CURRENT_PROTO}.bindingMemento, ${UTIL}.simpleChange(${oldValue}, ${newValue}));`; } -function invokeBindingMemento(oldValue:string, newValue:string):string { +function updateDirectiveTemplate(oldValue:string, newValue:string, directiveProperty:string):string { return ` if(throwOnChange) ${UTIL}.throwOnChange(${CURRENT_PROTO}, ${UTIL}.simpleChange(${oldValue}, ${newValue})); +${directiveProperty} = ${newValue}; + `; +} +function updateElementTemplate(oldValue:string, newValue:string):string { + return ` +if(throwOnChange) ${UTIL}.throwOnChange(${CURRENT_PROTO}, ${UTIL}.simpleChange(${oldValue}, ${newValue})); ${DISPATCHER_ACCESSOR}.invokeMementoFor(${CURRENT_PROTO}.bindingMemento, ${newValue}); `; } -function lastInDirectiveTemplate(protoIndex:number):string{ +function notifyOnChangesTemplate(directive:string):string{ return ` -if (${CHANGES_LOCAL}) { - ${DISPATCHER_ACCESSOR}.onChange(${PROTOS_ACCESSOR}[${protoIndex}].directiveMemento, ${CHANGES_LOCAL}); +if(${CHANGES_LOCAL}) { + ${directive}.onChange(${CHANGES_LOCAL}); + ${CHANGES_LOCAL} = null; } -${CHANGES_LOCAL} = null; `; } @@ -271,13 +284,22 @@ export class ChangeDetectorJITGenerator { genHydrate():string { var mode = ChangeDetectionUtil.changeDetectionMode(this.changeDetectionStrategy); return hydrateTemplate(this.typeName, mode, this.genFieldDefinitions(), - pipeOnDestroyTemplate(this.getNonNullPipeNames())); + pipeOnDestroyTemplate(this.getNonNullPipeNames()), this.getDirectiveFieldNames()); + } + + getDirectiveFieldNames():List { + return this.directiveMementos.map((d) => this.getDirective(d)); + } + + getDirective(memento) { + return `this.directive_${memento.name}`; } genFieldDefinitions() { var fields = []; fields = fields.concat(this.fieldNames); fields = fields.concat(this.getNonNullPipeNames()); + fields = fields.concat(this.getDirectiveFieldNames()); return fieldDefinitionsTemplate(fields); } @@ -303,7 +325,8 @@ export class ChangeDetectorJITGenerator { for (var i = mementos.length - 1; i >= 0; --i) { var memento = mementos[i]; if (memento.callOnAllChangesDone) { - notifications.push(onAllChangesDoneTemplate(i)); + var directive = `this.directive_${memento.name}`; + notifications.push(onAllChangesDoneTemplate(directive)); } } @@ -340,9 +363,9 @@ export class ChangeDetectorJITGenerator { var pipe = this.pipeNames[r.selfIndex]; var bpc = r.mode === RECORD_TYPE_BINDING_PIPE ? "this.bindingPropagationConfig" : "null"; - var invokeMemento = this.getInvokeMementoAndAddChangeTemplate(r); + var invokeMemento = this.genUpdateDirectiveOrElement(r); var addToChanges = this.genAddToChanges(r); - var lastInDirective = this.genLastInDirective(r); + var lastInDirective = this.genNotifyOnChanges(r); return pipeCheckTemplate(r.selfIndex - 1, context, bpc, pipe, r.name, oldValue, newValue, change, invokeMemento, addToChanges, lastInDirective); @@ -354,9 +377,9 @@ export class ChangeDetectorJITGenerator { var change = this.changeNames[r.selfIndex]; var assignment = this.genUpdateCurrentValue(r); - var invokeMemento = this.getInvokeMementoAndAddChangeTemplate(r); + var invokeMemento = this.genUpdateDirectiveOrElement(r); var addToChanges = this.genAddToChanges(r); - var lastInDirective = this.genLastInDirective(r); + var lastInDirective = this.genNotifyOnChanges(r); var check = referenceCheckTemplate(r.selfIndex - 1, assignment, oldValue, newValue, change, invokeMemento, addToChanges, lastInDirective); @@ -426,10 +449,18 @@ export class ChangeDetectorJITGenerator { return JSON.stringify(value); } - getInvokeMementoAndAddChangeTemplate(r:ProtoRecord):string { + genUpdateDirectiveOrElement(r:ProtoRecord):string { + if (! r.lastInBinding) return ""; + var newValue = this.localNames[r.selfIndex]; var oldValue = this.fieldNames[r.selfIndex]; - return r.lastInBinding ? invokeBindingMemento(oldValue, newValue) : ""; + + if (isPresent(r.directiveMemento)) { + var directiveProperty = `${this.getDirective(r.directiveMemento)}.${r.bindingMemento.propertyName}`; + return updateDirectiveTemplate(oldValue, newValue, directiveProperty); + } else { + return updateElementTemplate(oldValue, newValue); + } } genAddToChanges(r:ProtoRecord):string { @@ -439,9 +470,13 @@ export class ChangeDetectorJITGenerator { return callOnChange ? addToChangesTemplate(oldValue, newValue) : ""; } - genLastInDirective(r:ProtoRecord):string{ + genNotifyOnChanges(r:ProtoRecord):string{ var callOnChange = r.directiveMemento && r.directiveMemento.callOnChange; - return r.lastInDirective && callOnChange ? lastInDirectiveTemplate(r.selfIndex - 1) : ''; + if (r.lastInDirective && callOnChange) { + return notifyOnChangesTemplate(this.getDirective(r.directiveMemento)); + } else { + return ""; + } } genArgs(r:ProtoRecord):string { diff --git a/modules/angular2/src/change_detection/dynamic_change_detector.js b/modules/angular2/src/change_detection/dynamic_change_detector.js index 1ec9ac69f7..365534d143 100644 --- a/modules/angular2/src/change_detection/dynamic_change_detector.js +++ b/modules/angular2/src/change_detection/dynamic_change_detector.js @@ -34,6 +34,7 @@ export class DynamicChangeDetector extends AbstractChangeDetector { prevContexts:List; protos:List; + directives:any; directiveMementos:List; changeControlStrategy:string; @@ -53,16 +54,18 @@ export class DynamicChangeDetector extends AbstractChangeDetector { ListWrapper.fill(this.prevContexts, uninitialized); ListWrapper.fill(this.changes, false); this.locals = null; + this.directives = null; this.protos = protoRecords; this.directiveMementos = directiveMementos; this.changeControlStrategy = changeControlStrategy; } - hydrate(context:any, locals:any) { + hydrate(context:any, locals:any, directives:any) { this.mode = ChangeDetectionUtil.changeDetectionMode(this.changeControlStrategy); this.values[0] = context; this.locals = locals; + this.directives = directives; } dehydrate() { @@ -90,29 +93,18 @@ export class DynamicChangeDetector extends AbstractChangeDetector { var protos:List = this.protos; var changes = null; - var currentDirectiveMemento = null; - for (var i = 0; i < protos.length; ++i) { var proto:ProtoRecord = protos[i]; - if (isBlank(currentDirectiveMemento)) { - currentDirectiveMemento = proto.directiveMemento; - } var change = this._check(proto); if (isPresent(change)) { if (throwOnChange) ChangeDetectionUtil.throwOnChange(proto, change); - this.dispatcher.invokeMementoFor(proto.bindingMemento, change.currentValue); - - if (isPresent(currentDirectiveMemento) && currentDirectiveMemento.callOnChange) { - changes = ChangeDetectionUtil.addChange(changes, proto.bindingMemento, change); - } + this._updateDirectiveOrElement(change, proto.directiveMemento, proto.bindingMemento); + changes = this._addChange(proto.directiveMemento, proto.bindingMemento, change, changes); } - if (proto.lastInDirective) { - if (isPresent(changes)) { - this.dispatcher.onChange(currentDirectiveMemento, changes); - } - currentDirectiveMemento = null; + if (proto.lastInDirective && isPresent(changes)) { + this._directive(proto.directiveMemento).onChange(changes); changes = null; } } @@ -123,11 +115,31 @@ export class DynamicChangeDetector extends AbstractChangeDetector { for (var i = mementos.length - 1; i >= 0; --i) { var memento = mementos[i]; if (memento.callOnAllChangesDone) { - this.dispatcher.onAllChangesDone(memento); + this._directive(memento).onAllChangesDone(); } } } + _updateDirectiveOrElement(change, directiveMemento, bindingMemento) { + if (isBlank(directiveMemento)) { + this.dispatcher.invokeMementoFor(bindingMemento, change.currentValue); + } else { + bindingMemento.setter(this._directive(directiveMemento), change.currentValue); + } + } + + _addChange(directiveMemento, bindingMemento, change, changes) { + if (isPresent(directiveMemento) && directiveMemento.callOnChange) { + return ChangeDetectionUtil.addChange(changes, bindingMemento, change); + } else { + return changes; + } + } + + _directive(memento) { + return memento.directive(this.directives); + } + _check(proto:ProtoRecord) { try { if (proto.mode === RECORD_TYPE_PIPE || proto.mode === RECORD_TYPE_BINDING_PIPE) { diff --git a/modules/angular2/src/change_detection/interfaces.js b/modules/angular2/src/change_detection/interfaces.js index 68417c4d4c..d5d7d269a8 100644 --- a/modules/angular2/src/change_detection/interfaces.js +++ b/modules/angular2/src/change_detection/interfaces.js @@ -1,16 +1,17 @@ import {List} from 'angular2/src/facade/collection'; import {Locals} from './parser/locals'; import {AST} from './parser/ast'; +import {DEFAULT} from './constants'; export class ProtoChangeDetector { addAst(ast:AST, bindingMemento:any, directiveMemento:any = null){} - instantiate(dispatcher:any, bindingRecords:List, variableBindings:List, directiveMemento:List):ChangeDetector{ + instantiate(dispatcher:any, bindingRecords:List, variableBindings:List, directiveMementos:List):ChangeDetector{ return null; } } export class ChangeDetection { - createProtoChangeDetector(name:string, changeControlStrategy:string):ProtoChangeDetector{ + createProtoChangeDetector(name:string, changeControlStrategy:string=DEFAULT):ProtoChangeDetector{ return null; } } @@ -35,7 +36,7 @@ export class ChangeRecord { } export class ChangeDispatcher { - onRecordChange(directiveMemento, records:List) {} + invokeMementoFor(memento:any, value) {} } export class ChangeDetector { @@ -45,7 +46,7 @@ export class ChangeDetector { addChild(cd:ChangeDetector) {} removeChild(cd:ChangeDetector) {} remove() {} - hydrate(context:any, locals:Locals) {} + hydrate(context:any, locals:Locals, directives:any) {} dehydrate() {} markPathToRootAsCheckOnce() {} diff --git a/modules/angular2/src/core/compiler/view.js b/modules/angular2/src/core/compiler/view.js index daefe68cee..3160532041 100644 --- a/modules/angular2/src/core/compiler/view.js +++ b/modules/angular2/src/core/compiler/view.js @@ -67,10 +67,14 @@ export class View { return isPresent(this.context); } - _hydrateContext(newContext, locals) { + _setContextAndLocals(newContext, locals) { this.context = newContext; this.locals.parent = locals; - this.changeDetector.hydrate(this.context, this.locals); + this.changeDetector.hydrate(this.context, this.locals, this.elementInjectors); + } + + _hydrateChangeDetector() { + this.changeDetector.hydrate(this.context, this.locals, this.elementInjectors); } _dehydrateContext() { @@ -117,7 +121,8 @@ export class View { if (this.hydrated()) throw new BaseException('The view is already hydrated.'); this.render = renderComponentViewRefs[renderComponentIndex++]; - this._hydrateContext(context, locals); + + this._setContextAndLocals(context, locals); // viewContainers for (var i = 0; i < this.viewContainers.length; i++) { @@ -173,6 +178,7 @@ export class View { componentChildViewIndex++; } } + this._hydrateChangeDetector(); this.proto.renderer.setEventDispatcher(this.render, this); return renderComponentIndex; } @@ -222,22 +228,9 @@ export class View { this.dispatchEvent(binderIndex, eventName, locals); } - onAllChangesDone(directiveMemento:DirectiveMemento) { - var dir = directiveMemento.directive(this.elementInjectors); - dir.onAllChangesDone(); - } - - onChange(directiveMemento:DirectiveMemento, changes) { - var dir = directiveMemento.directive(this.elementInjectors); - dir.onChange(changes); - } - // dispatch to element injector or text nodes based on context invokeMementoFor(memento:any, currentValue:any) { - if (memento instanceof DirectiveBindingMemento) { - var directiveMemento:DirectiveBindingMemento = memento; - directiveMemento.invoke(currentValue, this.elementInjectors); - } else if (memento instanceof ElementBindingMemento) { + if (memento instanceof ElementBindingMemento) { var elementMemento:ElementBindingMemento = memento; this.proto.renderer.setElementProperty( this.render, elementMemento.elementIndex, elementMemento.propertyName, currentValue @@ -461,7 +454,7 @@ export class DirectiveBindingMemento { _elementInjectorIndex:int; _directiveIndex:int; propertyName:string; - _setter:SetterFn; + setter:SetterFn; constructor( elementInjectorIndex:number, directiveIndex:number, @@ -470,13 +463,13 @@ export class DirectiveBindingMemento { this._elementInjectorIndex = elementInjectorIndex; this._directiveIndex = directiveIndex; this.propertyName = propertyName; - this._setter = setter; + this.setter = setter; } invoke(currentValue:any, elementInjectors:List) { var elementInjector:ElementInjector = elementInjectors[this._elementInjectorIndex]; var directive = elementInjector.getDirectiveAtIndex(this._directiveIndex); - this._setter(directive, currentValue); + this.setter(directive, currentValue); } } @@ -486,6 +479,10 @@ class DirectiveMemento { callOnAllChangesDone:boolean; callOnChange:boolean; + get name() { + return `${this._elementInjectorIndex}_${this._directiveIndex}`; + } + constructor(elementInjectorIndex:number, directiveIndex:number, callOnAllChangesDone:boolean, callOnChange:boolean) { this._elementInjectorIndex = elementInjectorIndex; diff --git a/modules/angular2/test/change_detection/change_detection_spec.js b/modules/angular2/test/change_detection/change_detection_spec.js index de21d77461..8b6dfc7d91 100644 --- a/modules/angular2/test/change_detection/change_detection_spec.js +++ b/modules/angular2/test/change_detection/change_detection_spec.js @@ -42,9 +42,10 @@ export function main() { var dispatcher = new TestDispatcher(); var variableBindings = convertLocalsToVariableBindings(locals); - var records = [new BindingRecord(ast(exp), memo, new FakeDirectiveMemento(memo, false))]; + + var records = [new BindingRecord(ast(exp), memo, null)]; var cd = pcd.instantiate(dispatcher, records, variableBindings, []); - cd.hydrate(context, locals); + cd.hydrate(context, locals, null); return {"changeDetector" : cd, "dispatcher" : dispatcher}; } @@ -55,8 +56,9 @@ export function main() { return res["dispatcher"].log; } - function instantiate(protoChangeDetector, dispatcher, bindings) { - return protoChangeDetector.instantiate(dispatcher, bindings, null, []); + function instantiate(protoChangeDetector, dispatcher, bindings, directiveMementos = null) { + if (isBlank(directiveMementos)) directiveMementos = []; + return protoChangeDetector.instantiate(dispatcher, bindings, null, directiveMementos); } describe(`${name} change detection`, () => { @@ -68,6 +70,7 @@ export function main() { it('should do simple watching', () => { var person = new Person("misko"); + var c = createChangeDetector('name', 'name', person); var cd = c["changeDetector"]; var dispatcher = c["dispatcher"]; @@ -202,7 +205,7 @@ export function main() { var ast = parser.parseInterpolation("B{{a}}A", "location"); var cd = instantiate(pcd, dispatcher, [new BindingRecord(ast, "memo", null)]); - cd.hydrate(new TestData("value"), null); + cd.hydrate(new TestData("value"), null, null); cd.detectChanges(); @@ -238,98 +241,133 @@ export function main() { }); }); - describe("onChange", () => { - var dirMemento1 = new FakeDirectiveMemento(1, false, true); - var dirMemento2 = new FakeDirectiveMemento(2, false, true); - var dirMementoNoOnChange = new FakeDirectiveMemento(3, false, false); - var memo1 = new FakeBindingMemento("memo1"); - var memo2 = new FakeBindingMemento("memo2"); + describe("updatingDirectives", () => { + var dirMemento1 = new FakeDirectiveMemento(0, true, true); + var dirMemento2 = new FakeDirectiveMemento(1, true, true); + var dirMementoNoCallbacks = new FakeDirectiveMemento(0, false, false); - it("should notify the dispatcher when a group of records changes", () => { + var updateA = new FakeBindingMemento((o, v) => o.a = v, "a"); + var updateB = new FakeBindingMemento((o, v) => o.b = v, "b"); + + var directive1; + var directive2; + + beforeEach(() => { + directive1 = new TestDirective(); + directive2 = new TestDirective(); + }); + + it("should happen directly, without invoking the dispatcher", () => { var pcd = createProtoChangeDetector(); - var cd = instantiate(pcd, dispatcher, [ - new BindingRecord(ast("1 + 2"), memo1, dirMemento1), - new BindingRecord(ast("10 + 20"), memo2, dirMemento1), - new BindingRecord(ast("100 + 200"), memo1, dirMemento2) - ]); + var cd = instantiate(pcd, dispatcher, [new BindingRecord(ast("42"), updateA, dirMemento1)], + [dirMemento1]); + + cd.hydrate(null, null, [directive1]) cd.detectChanges(); - expect(dispatcher.loggedOnChange).toEqual([{'memo1': 3, 'memo2': 30}, {'memo1': 300}]); + expect(dispatcher.loggedValues).toEqual([]); + expect(directive1.a).toEqual(42); }); - it("should not notify the dispatcher when callOnChange is false", () => { - var pcd = createProtoChangeDetector(); + describe("onChange", () => { + it("should notify the directive when a group of records changes", () => { + var pcd = createProtoChangeDetector(); - var cd = instantiate(pcd, dispatcher, [ - new BindingRecord(ast("1"), memo1, dirMemento1), - new BindingRecord(ast("2"), memo1, dirMementoNoOnChange), - new BindingRecord(ast("3"), memo1, dirMemento2) - ]); + var cd = instantiate(pcd, dispatcher, [ + new BindingRecord(ast("1"), updateA, dirMemento1), + new BindingRecord(ast("2"), updateB, dirMemento1), + new BindingRecord(ast("3"), updateA, dirMemento2) + ], [dirMemento1, dirMemento2]); - cd.detectChanges(); + cd.hydrate(null, null, [directive1, directive2]) - expect(dispatcher.loggedOnChange).toEqual([{'memo1': 1}, {'memo1': 3}]); - }); - }); + cd.detectChanges(); - describe("onAllChangesDone", () => { - it("should notify the dispatcher about processing all the children", () => { - var memento1 = new FakeDirectiveMemento(1, false); - var memento2 = new FakeDirectiveMemento(2, true); + expect(directive1.changes).toEqual({'a': 1, 'b': 2}); + expect(directive2.changes).toEqual({'a': 3}); + }); - var pcd = createProtoChangeDetector(); - var cd = pcd.instantiate(dispatcher, [], null, [memento1, memento2]); + it("should not call onChange when callOnChange is false", () => { + var pcd = createProtoChangeDetector(); - cd.hydrate(null, null); + var cd = instantiate(pcd, dispatcher, [ + new BindingRecord(ast("1"), updateA, dirMementoNoCallbacks) + ], [dirMementoNoCallbacks]); - cd.detectChanges(); + cd.hydrate(null, null, [directive1]) - expect(dispatcher.loggedValues).toEqual([ - ["onAllChangesDone", memento2] - ]); + cd.detectChanges(); + + expect(directive1.changes).toEqual(null); + }); }); - it("should notify in reverse order so the child is always notified before the parent", () => { - var memento1 = new FakeDirectiveMemento(1, true); - var memento2 = new FakeDirectiveMemento(2, true); + describe("onAllChangesDone", () => { + it("should be called after processing all the children", () => { + var pcd = createProtoChangeDetector(); - var pcd = createProtoChangeDetector(); - var cd = pcd.instantiate(dispatcher, [], null, [memento1, memento2]); + var cd = instantiate(pcd, dispatcher, [], [dirMemento1, dirMemento2]); + cd.hydrate(null, null, [directive1, directive2]); - cd.hydrate(null, null); + cd.detectChanges(); - cd.detectChanges(); + expect(directive1.onChangesDoneCalled).toBe(true); + expect(directive2.onChangesDoneCalled).toBe(true); + }); - expect(dispatcher.loggedValues).toEqual([ - ["onAllChangesDone", memento2], - ["onAllChangesDone", memento1] - ]); - }); - it("should notify the dispatcher before processing shadow dom children", () => { - var memento = new FakeDirectiveMemento(1, true); + it("should not be called when onAllChangesDone is false", () => { + var pcd = createProtoChangeDetector(); - var pcd = createProtoChangeDetector(); - var shadowDomChildPCD = createProtoChangeDetector(); + var cd = instantiate(pcd, dispatcher, [ + new BindingRecord(ast("1"), updateA, dirMementoNoCallbacks) + ], [dirMementoNoCallbacks]); - var parent = pcd.instantiate(dispatcher, [], null, [memento]); - var child = shadowDomChildPCD.instantiate(dispatcher, [ - new BindingRecord(ast("1"), "a", memento)], null, []); - parent.addShadowDomChild(child); + cd.hydrate(null, null, [directive1]) - parent.hydrate(null, null); - child.hydrate(null, null); + cd.detectChanges(); - parent.detectChanges(); + expect(directive1.onChangesDoneCalled).toEqual(false); + }); - // loggedValues contains everything that the dispatcher received - // the first value is the directive memento passed into onAllChangesDone - expect(dispatcher.loggedValues).toEqual([ - ["onAllChangesDone", memento], - 1 - ]); + it("should be called in reverse order so the child is always notified before the parent", () => { + var pcd = createProtoChangeDetector(); + var cd = instantiate(pcd, dispatcher, [], [dirMemento1, dirMemento2]); + + var onChangesDoneCalls = []; + var td1; + td1 = new TestDirective(() => ListWrapper.push(onChangesDoneCalls, td1)); + var td2; + td2 = new TestDirective(() => ListWrapper.push(onChangesDoneCalls, td2)); + cd.hydrate(null, null, [td1, td2]); + + cd.detectChanges(); + + expect(onChangesDoneCalls).toEqual([td2, td1]); + }); + + it("should be called before processing shadow dom children", () => { + var pcd = createProtoChangeDetector(); + var shadowDomChildPCD = createProtoChangeDetector(); + + var parent = pcd.instantiate(dispatcher, [], null, [dirMemento1]); + + var child = shadowDomChildPCD.instantiate(dispatcher, [ + new BindingRecord(ast("1"), updateA, dirMemento1)], null, [dirMemento1]); + parent.addShadowDomChild(child); + + var directiveInShadowDOm = new TestDirective(); + var parentDirective = new TestDirective(() => { + expect(directiveInShadowDOm.a).toBe(null); + }); + + parent.hydrate(null, null, [parentDirective]); + child.hydrate(null, null, [directiveInShadowDOm]); + + parent.detectChanges(); + }); }); }); }); @@ -343,7 +381,7 @@ export function main() { var cd = instantiate(pcd, dispatcher, [ new BindingRecord(ast("a"), "a", 1) ]); - cd.hydrate(new TestData('value'), null); + cd.hydrate(new TestData('value'), null, null); expect(() => { cd.checkNoChanges(); @@ -448,7 +486,7 @@ export function main() { expect(cd.mode).toEqual(null); - cd.hydrate(null, null); + cd.hydrate(null, null, null); expect(cd.mode).toEqual(CHECK_ALWAYS); }); @@ -456,7 +494,7 @@ export function main() { it("should set the mode to CHECK_ONCE when the push change detection is used", () => { var proto = createProtoChangeDetector(null, ON_PUSH); var cd = proto.instantiate(null, [], [], []); - cd.hydrate(null, null); + cd.hydrate(null, null, null); expect(cd.mode).toEqual(CHECK_ONCE); }); @@ -536,13 +574,13 @@ export function main() { var c = createChangeDetector("memo", "name"); var cd = c["changeDetector"]; - cd.hydrate("some context", null); + cd.hydrate("some context", null, null); expect(cd.hydrated()).toBe(true); cd.dehydrate(); expect(cd.hydrated()).toBe(false); - cd.hydrate("other context", null); + cd.hydrate("other context", null, null); expect(cd.hydrated()).toBe(true); }); @@ -726,10 +764,33 @@ class FakePipeRegistry extends PipeRegistry { } } -class TestRecord { +class TestDirective { a; b; - c; + changes; + onChangesDoneCalled; + onChangesDoneSpy; + + constructor(onChangesDoneSpy = null) { + this.onChangesDoneCalled = false; + this.onChangesDoneSpy = onChangesDoneSpy; + this.a = null; + this.b = null; + this.changes = null; + } + + onChange(changes) { + var r = {}; + StringMapWrapper.forEach(changes, (c, key) => r[key] = c.currentValue); + this.changes = r; + } + + onAllChangesDone() { + this.onChangesDoneCalled = true; + if(isPresent(this.onChangesDoneSpy)) { + this.onChangesDoneSpy(); + } + } } class Person { @@ -776,21 +837,31 @@ class TestData { } class FakeDirectiveMemento { - value:any; callOnAllChangesDone:boolean; callOnChange:boolean; + directiveIndex:number; - constructor(value, callOnAllChangesDone:boolean = false, callOnChange:boolean = false) { - this.value = value; + constructor(directiveIndex:number = 0, callOnAllChangesDone:boolean = false, callOnChange:boolean = false) { + this.directiveIndex = directiveIndex; this.callOnAllChangesDone = callOnAllChangesDone; this.callOnChange = callOnChange; } + + get name() { + return this.directiveIndex; + } + + directive(directives) { + return directives[this.directiveIndex]; + } } class FakeBindingMemento { + setter:Function; propertyName:string; - constructor(propertyName:string) { + constructor(setter:Function, propertyName:string) { + this.setter = setter; this.propertyName = propertyName; } } @@ -798,7 +869,6 @@ class FakeBindingMemento { class TestDispatcher extends ChangeDispatcher { log:List; loggedValues:List; - loggedOnChange:List; constructor() { super(); @@ -808,17 +878,6 @@ class TestDispatcher extends ChangeDispatcher { clear() { this.log = ListWrapper.create(); this.loggedValues = ListWrapper.create(); - this.loggedOnChange = ListWrapper.create(); - } - - onChange(directiveMemento, changes) { - var r = {}; - StringMapWrapper.forEach(changes, (c, key) => r[key] = c.currentValue); - ListWrapper.push(this.loggedOnChange, r); - } - - onAllChangesDone(directiveMemento) { - ListWrapper.push(this.loggedValues, ["onAllChangesDone", directiveMemento]); } invokeMementoFor(memento, value) { diff --git a/modules/benchmarks/e2e_test/tree_perf.es6 b/modules/benchmarks/e2e_test/tree_perf.es6 index f510c901bd..ed91a5ce7b 100644 --- a/modules/benchmarks/e2e_test/tree_perf.es6 +++ b/modules/benchmarks/e2e_test/tree_perf.es6 @@ -17,6 +17,17 @@ describe('ng2 tree benchmark', function () { }).then(done, done.fail); }); + it('should log the ng stats (update)', function(done) { + perfUtil.runClickBenchmark({ + url: URL, + buttons: ['#ng2CreateDom'], + id: 'ng2.tree.update', + params: [{ + name: 'depth', value: 9, scale: 'log2' + }] + }).then(done, done.fail); + }); + it('should log the baseline stats', function(done) { perfUtil.runClickBenchmark({ url: URL, @@ -28,4 +39,15 @@ describe('ng2 tree benchmark', function () { }).then(done, done.fail); }); + it('should log the baseline stats (update)', function(done) { + perfUtil.runClickBenchmark({ + url: URL, + buttons: ['#baselineCreateDom'], + id: 'baseline.tree', + params: [{ + name: 'depth', value: 9, scale: 'log2' + }] + }).then(done, done.fail); + }); + }); diff --git a/modules/benchmarks/src/change_detection/change_detection_benchmark.js b/modules/benchmarks/src/change_detection/change_detection_benchmark.js index ad37f23afb..76a907605e 100644 --- a/modules/benchmarks/src/change_detection/change_detection_benchmark.js +++ b/modules/benchmarks/src/change_detection/change_detection_benchmark.js @@ -185,27 +185,29 @@ function setUpChangeDetection(changeDetection:ChangeDetection, iterations, objec var dispatcher = new DummyDispatcher(); var parser = new Parser(new Lexer()); - var parentProto = changeDetection.createProtoChangeDetector('parent', null); + var parentProto = changeDetection.createProtoChangeDetector('parent'); var parentCd = parentProto.instantiate(dispatcher, [], [], []); var targetObj = new Obj(); - var proto = changeDetection.createProtoChangeDetector("proto", null); + var proto = changeDetection.createProtoChangeDetector("proto"); + + var directiveMemento = new FakeDirectiveMemento("target", targetObj); var bindingRecords = [ - new BindingRecord(parser.parseBinding('field0', null), new FakeBindingMemento(targetObj, reflector.setter("field0")), null), - new BindingRecord(parser.parseBinding('field1', null), new FakeBindingMemento(targetObj, reflector.setter("field1")), null), - new BindingRecord(parser.parseBinding('field2', null), new FakeBindingMemento(targetObj, reflector.setter("field2")), null), - new BindingRecord(parser.parseBinding('field3', null), new FakeBindingMemento(targetObj, reflector.setter("field3")), null), - new BindingRecord(parser.parseBinding('field4', null), new FakeBindingMemento(targetObj, reflector.setter("field4")), null), - new BindingRecord(parser.parseBinding('field5', null), new FakeBindingMemento(targetObj, reflector.setter("field5")), null), - new BindingRecord(parser.parseBinding('field6', null), new FakeBindingMemento(targetObj, reflector.setter("field6")), null), - new BindingRecord(parser.parseBinding('field7', null), new FakeBindingMemento(targetObj, reflector.setter("field7")), null), - new BindingRecord(parser.parseBinding('field8', null), new FakeBindingMemento(targetObj, reflector.setter("field8")), null), - new BindingRecord(parser.parseBinding('field9', null), new FakeBindingMemento(targetObj, reflector.setter("field9")), null) + new BindingRecord(parser.parseBinding('field0', null), new FakeBindingMemento(reflector.setter("field0"), "field0"), directiveMemento), + new BindingRecord(parser.parseBinding('field1', null), new FakeBindingMemento(reflector.setter("field1"), "field1"), directiveMemento), + new BindingRecord(parser.parseBinding('field2', null), new FakeBindingMemento(reflector.setter("field2"), "field2"), directiveMemento), + new BindingRecord(parser.parseBinding('field3', null), new FakeBindingMemento(reflector.setter("field3"), "field3"), directiveMemento), + new BindingRecord(parser.parseBinding('field4', null), new FakeBindingMemento(reflector.setter("field4"), "field4"), directiveMemento), + new BindingRecord(parser.parseBinding('field5', null), new FakeBindingMemento(reflector.setter("field5"), "field5"), directiveMemento), + new BindingRecord(parser.parseBinding('field6', null), new FakeBindingMemento(reflector.setter("field6"), "field6"), directiveMemento), + new BindingRecord(parser.parseBinding('field7', null), new FakeBindingMemento(reflector.setter("field7"), "field7"), directiveMemento), + new BindingRecord(parser.parseBinding('field8', null), new FakeBindingMemento(reflector.setter("field8"), "field8"), directiveMemento), + new BindingRecord(parser.parseBinding('field9', null), new FakeBindingMemento(reflector.setter("field9"), "field9"), directiveMemento) ]; for (var i = 0; i < iterations; ++i) { - var cd = proto.instantiate(dispatcher, bindingRecords, [], []); - cd.hydrate(object, null); + var cd = proto.instantiate(dispatcher, bindingRecords, [], [directiveMemento]); + cd.hydrate(object, null, null); parentCd.addChild(cd); } return parentCd; @@ -298,17 +300,34 @@ export function main () { class FakeBindingMemento { setter:Function; - targetObj:Obj; + propertyName:string; - constructor(targetObj:Obj, setter:Function) { - this.targetObj = targetObj; + constructor(setter:Function, propertyName:string) { this.setter = setter; + this.propertyName = propertyName; + } +} + +class FakeDirectiveMemento { + targetObj:Obj; + name:string; + callOnChange:boolean; + callOnAllChangesDone:boolean; + + constructor(name, targetObj) { + this.targetObj = targetObj; + this.name = name; + this.callOnChange = false; + this.callOnAllChangesDone = false; + } + + directive(dirs) { + return this.targetObj; } } class DummyDispatcher extends ChangeDispatcher { invokeMementoFor(bindingMemento, newValue) { - var obj = bindingMemento.targetObj; - bindingMemento.setter(obj, newValue); + throw "Should not be used"; } }