From b76f5a6a7df92c9eb142b7ffe0ecc931f9b4e68e Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Sat, 14 Apr 2018 11:52:53 -0700 Subject: [PATCH] perf(ivy): add performance counters in ngDevMode (#23385) PR Close #23385 --- packages/core/src/render3/instructions.ts | 42 +++++++-- packages/core/src/render3/ng_dev_mode.ts | 54 ++++++++++-- .../core/test/render3/instructions_spec.ts | 88 ++++++++++++++++++- .../core/test/render3/integration_spec.ts | 6 ++ .../core/test/render3/perfCounter_spec.ts | 46 ++++++++++ packages/types.d.ts | 8 +- 6 files changed, 226 insertions(+), 18 deletions(-) create mode 100644 packages/core/test/render3/perfCounter_spec.ts diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index dcddb6588c..62d5a3fbfa 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -566,6 +566,7 @@ export function elementStart( assertEqual( currentView.bindingStartIndex, -1, 'elements should be created before any bindings'); + ngDevMode && ngDevMode.rendererCreateElement++; const native: RElement = renderer.createElement(name); const node: LElementNode = createLNode(index, LNodeType.Element, native !, null); @@ -580,6 +581,7 @@ function createDirectivesAndLocals( localRefs: string[] | null | undefined, containerData: TView[] | null) { const node = previousOrParentNode; if (firstTemplatePass) { + ngDevMode && ngDevMode.firstTemplatePass++; ngDevMode && assertDataInRange(index - 1); node.tNode = tData[index] = createTNode(name, attrs || null, containerData); cacheMatchingDirectivesForNode(node.tNode, currentView.tView, localRefs || null); @@ -756,6 +758,7 @@ function getOrCreateTView( /** Creates a TView instance */ export function createTView( defs: DirectiveDefListOrFactory | null, pipes: PipeDefListOrFactory | null): TView { + ngDevMode && ngDevMode.tView++; return { data: [], directives: null, @@ -784,6 +787,7 @@ function setUpAttributes(native: RElement, attrs: string[]): void { const attrName = attrs[i]; if (attrName !== NG_PROJECT_AS_ATTR_NAME) { const attrVal = attrs[i + 1]; + ngDevMode && ngDevMode.rendererSetAttribute++; isProc ? (renderer as ProceduralRenderer3).setAttribute(native, attrName, attrVal) : native.setAttribute(attrName, attrVal); } @@ -867,6 +871,7 @@ export function listener( // In order to match current behavior, native DOM event listeners must be added for all // events (including outputs). const cleanupFns = cleanup || (cleanup = currentView.cleanup = []); + ngDevMode && ngDevMode.rendererAddEventListener++; if (isProceduralRenderer(renderer)) { const wrappedListener = wrapListenerWithDirtyLogic(currentView, listenerFn); const cleanupFn = renderer.listen(native, eventName, wrappedListener); @@ -931,9 +936,11 @@ export function elementAttribute( if (value !== NO_CHANGE) { const element: LElementNode = data[index]; if (value == null) { + ngDevMode && ngDevMode.rendererRemoveAttribute++; isProceduralRenderer(renderer) ? renderer.removeAttribute(element.native, name) : element.native.removeAttribute(name); } else { + ngDevMode && ngDevMode.rendererSetAttribute++; const strValue = sanitizer == null ? stringify(value) : sanitizer(value); isProceduralRenderer(renderer) ? renderer.setAttribute(element.native, name, strValue) : element.native.setAttribute(name, strValue); @@ -977,6 +984,7 @@ export function elementProperty( // is risky, so sanitization can be done without further checks. value = sanitizer != null ? (sanitizer(value) as any) : value; const native = node.native; + ngDevMode && ngDevMode.rendererSetProperty++; isProceduralRenderer(renderer) ? renderer.setProperty(native, propName, value) : (native.setProperty ? native.setProperty(propName, value) : (native as any)[propName] = value); @@ -994,6 +1002,7 @@ export function elementProperty( */ function createTNode( tagName: string | null, attrs: string[] | null, data: TContainer | null): TNode { + ngDevMode && ngDevMode.tNode++; return { flags: 0, tagName: tagName, @@ -1067,10 +1076,12 @@ export function elementClassNamed(index: number, className: string, value: T if (value !== NO_CHANGE) { const lElement = data[index] as LElementNode; if (value) { + ngDevMode && ngDevMode.rendererAddClass++; isProceduralRenderer(renderer) ? renderer.addClass(lElement.native, className) : lElement.native.classList.add(className); } else { + ngDevMode && ngDevMode.rendererRemoveClass++; isProceduralRenderer(renderer) ? renderer.removeClass(lElement.native, className) : lElement.native.classList.remove(className); } @@ -1095,6 +1106,7 @@ export function elementClass(index: number, value: T | NO_CHANGE): void { // future // we will add logic here which would work with the animation code. const lElement: LElementNode = data[index]; + ngDevMode && ngDevMode.rendererSetClassName++; isProceduralRenderer(renderer) ? renderer.setProperty(lElement.native, 'className', value) : lElement.native['className'] = stringify(value); } @@ -1121,6 +1133,7 @@ export function elementStyleNamed( if (value !== NO_CHANGE) { const lElement: LElementNode = data[index]; if (value == null) { + ngDevMode && ngDevMode.rendererRemoveStyle++; isProceduralRenderer(renderer) ? renderer.removeStyle(lElement.native, styleName, RendererStyleFlags3.DashCase) : lElement.native['style'].removeProperty(styleName); @@ -1128,6 +1141,7 @@ export function elementStyleNamed( let strValue = typeof suffixOrSanitizer == 'function' ? suffixOrSanitizer(value) : stringify(value); if (typeof suffixOrSanitizer == 'string') strValue = strValue + suffixOrSanitizer; + ngDevMode && ngDevMode.rendererSetStyle++; isProceduralRenderer(renderer) ? renderer.setStyle(lElement.native, styleName, strValue, RendererStyleFlags3.DashCase) : lElement.native['style'].setProperty(styleName, strValue); @@ -1155,14 +1169,20 @@ export function elementStyle( // we will add logic here which would work with the animation code. const lElement = data[index] as LElementNode; if (isProceduralRenderer(renderer)) { + ngDevMode && ngDevMode.rendererSetStyle++; renderer.setProperty(lElement.native, 'style', value); } else { const style = lElement.native['style']; for (let i = 0, keys = Object.keys(value); i < keys.length; i++) { const styleName: string = keys[i]; const styleValue: any = (value as any)[styleName]; - styleValue == null ? style.removeProperty(styleName) : - style.setProperty(styleName, styleValue); + if (styleValue == null) { + ngDevMode && ngDevMode.rendererRemoveStyle++; + style.removeProperty(styleName); + } else { + ngDevMode && ngDevMode.rendererSetStyle++; + style.setProperty(styleName, styleValue); + } } } } @@ -1184,6 +1204,7 @@ export function text(index: number, value?: any): void { ngDevMode && assertEqual( currentView.bindingStartIndex, -1, 'text nodes should be created before bindings'); + ngDevMode && ngDevMode.rendererCreateTextNode++; const textNode = createTextNode(value, renderer); const node = createLNode(index, LNodeType.Element, textNode); // Text nodes are self closing. @@ -1203,9 +1224,18 @@ export function textBinding(index: number, value: T | NO_CHANGE): void { let existingNode = data[index] as LTextNode; ngDevMode && assertNotNull(existingNode, 'LNode should exist'); ngDevMode && assertNotNull(existingNode.native, 'native element should exist'); - value !== NO_CHANGE && - (isProceduralRenderer(renderer) ? renderer.setValue(existingNode.native, stringify(value)) : - existingNode.native.textContent = stringify(value)); + if (existingNode.native) { + // If DOM node exists and value changed, update textContent + ngDevMode && ngDevMode.rendererSetText++; + value !== NO_CHANGE && + (isProceduralRenderer(renderer) ? renderer.setValue(existingNode.native, stringify(value)) : + existingNode.native.textContent = stringify(value)); + } else { + // Node was created but DOM node creation was delayed. Create and append now. + ngDevMode && ngDevMode.rendererCreateTextNode++; + existingNode.native = createTextNode(value, renderer); + insertChild(existingNode, currentView); + } } ////////////////////////// @@ -1407,7 +1437,7 @@ export function createLContainer( * @param localRefs A set of local reference bindings on the element. */ export function container( - index: number, template?: ComponentTemplate, tagName?: string, attrs?: string[], + index: number, template?: ComponentTemplate, tagName?: string | null, attrs?: string[], localRefs?: string[] | null): void { ngDevMode && assertEqual( currentView.bindingStartIndex, -1, diff --git a/packages/core/src/render3/ng_dev_mode.ts b/packages/core/src/render3/ng_dev_mode.ts index 00fcad5c5d..3580605386 100644 --- a/packages/core/src/render3/ng_dev_mode.ts +++ b/packages/core/src/render3/ng_dev_mode.ts @@ -8,15 +8,51 @@ declare global { - const ngDevMode: boolean; + const ngDevMode: null|NgDevModePerfCounters; + interface NgDevModePerfCounters { + firstTemplatePass: number; + tNode: number; + tView: number; + rendererCreateTextNode: number; + rendererSetText: number; + rendererCreateElement: number; + rendererAddEventListener: number; + rendererSetAttribute: number; + rendererRemoveAttribute: number; + rendererSetProperty: number; + rendererSetClassName: number; + rendererAddClass: number; + rendererRemoveClass: number; + rendererSetStyle: number; + rendererRemoveStyle: number; + } } + + declare let global: any; - -if (typeof ngDevMode == 'undefined') { - if (typeof window != 'undefined') (window as any).ngDevMode = true; - if (typeof self != 'undefined') (self as any).ngDevMode = true; - if (typeof global != 'undefined') (global as any).ngDevMode = true; -} - -export const _ngDevMode = true; +export const ngDevModeResetPerfCounters: () => void = + (typeof ngDevMode == 'undefined' && (function(global: {ngDevMode: NgDevModePerfCounters}) { + function ngDevModeResetPerfCounters() { + global['ngDevMode'] = { + firstTemplatePass: 0, + tNode: 0, + tView: 0, + rendererCreateTextNode: 0, + rendererSetText: 0, + rendererCreateElement: 0, + rendererAddEventListener: 0, + rendererSetAttribute: 0, + rendererRemoveAttribute: 0, + rendererSetProperty: 0, + rendererSetClassName: 0, + rendererAddClass: 0, + rendererRemoveClass: 0, + rendererSetStyle: 0, + rendererRemoveStyle: 0, + }; + } + ngDevModeResetPerfCounters(); + return ngDevModeResetPerfCounters; + })(typeof window != 'undefined' && window || typeof self != 'undefined' && self || + typeof global != 'undefined' && global)) as() => void; diff --git a/packages/core/test/render3/instructions_spec.ts b/packages/core/test/render3/instructions_spec.ts index c654954a7f..1bba3b2cc5 100644 --- a/packages/core/test/render3/instructions_spec.ts +++ b/packages/core/test/render3/instructions_spec.ts @@ -6,12 +6,17 @@ * found in the LICENSE file at https://angular.io/license */ -import {elementAttribute, elementClass, elementEnd, elementProperty, elementStart, elementStyle, elementStyleNamed, renderTemplate} from '../../src/render3/instructions'; +import {NgForOfContext} from '@angular/common'; + +import {RenderFlags, directiveInject} from '../../src/render3'; +import {defineComponent} from '../../src/render3/definition'; +import {bind, container, elementAttribute, elementClass, elementEnd, elementProperty, elementStart, elementStyle, elementStyleNamed, interpolation1, renderTemplate, text, textBinding} from '../../src/render3/instructions'; import {LElementNode, LNode} from '../../src/render3/interfaces/node'; import {RElement, domRendererFactory3} from '../../src/render3/interfaces/renderer'; import {bypassSanitizationTrustStyle, bypassSanitizationTrustUrl, sanitizeStyle, sanitizeUrl} from '../../src/sanitization/sanitization'; -import {TemplateFixture} from './render_util'; +import {NgForOf} from './common_with_def'; +import {ComponentFixture, TemplateFixture} from './render_util'; describe('instructions', () => { function createDiv() { @@ -30,6 +35,13 @@ describe('instructions', () => { () => elementAttribute( 0, 'title', bypassSanitizationTrustUrl('javascript:true'), sanitizeUrl)); expect(t.html).toEqual('
'); + expect(ngDevMode).toHaveProperties({ + firstTemplatePass: 1, + tNode: 1, + tView: 1, + rendererCreateElement: 1, + rendererSetAttribute: 2 + }); }); }); @@ -44,6 +56,12 @@ describe('instructions', () => { () => elementProperty( 0, 'title', bypassSanitizationTrustUrl('javascript:false'), sanitizeUrl)); expect(t.html).toEqual('
'); + expect(ngDevMode).toHaveProperties({ + firstTemplatePass: 1, + tNode: 1, + tView: 1, + rendererCreateElement: 1, + }); }); it('should not stringify non string values', () => { @@ -52,6 +70,13 @@ describe('instructions', () => { t.update(() => elementProperty(0, 'hidden', false)); // The hidden property would be true if `false` was stringified into `"false"`. expect((t.hostNode.native as HTMLElement).querySelector('div') !.hidden).toEqual(false); + expect(ngDevMode).toHaveProperties({ + firstTemplatePass: 1, + tNode: 1, + tView: 1, + rendererCreateElement: 1, + rendererSetProperty: 1 + }); }); }); @@ -93,4 +118,63 @@ describe('instructions', () => { expect(fixture.html).toEqual('
'); }); }); + + describe('performance counters', () => { + it('should create tViews only once for each nested level', () => { + const _c0 = ['ngFor', '', 'ngForOf', '']; + /** + *
    + *
  • {{col}}
    • + *
+ */ + class NestedLoops { + rows = [['a', 'b'], ['A', 'B'], ['a', 'b'], ['A', 'B']]; + + static ngComponentDef = defineComponent({ + type: NestedLoops, + selectors: [['todo-app']], + factory: function ToDoAppComponent_Factory() { return new NestedLoops(); }, + template: function ToDoAppComponent_Template(rf: RenderFlags, ctx: NestedLoops) { + if (rf & 1) { + container(0, ToDoAppComponent_NgForOf_Template_0, null, _c0); + } + if (rf & 2) { + elementProperty(0, 'ngForOf', bind(ctx.rows)); + } + function ToDoAppComponent_NgForOf_Template_0( + rf: RenderFlags, ctx0: NgForOfContext) { + if (rf & 1) { + elementStart(0, 'ul'); + container(1, ToDoAppComponent_NgForOf_NgForOf_Template_1, null, _c0); + elementEnd(); + } + if (rf & 2) { + const row_r2 = ctx0.$implicit; + elementProperty(1, 'ngForOf', bind(row_r2)); + } + function ToDoAppComponent_NgForOf_NgForOf_Template_1( + rf: RenderFlags, ctx1: NgForOfContext) { + if (rf & 1) { + elementStart(0, 'li'); + text(1); + elementEnd(); + } + if (rf & 2) { + const col_r3 = ctx1.$implicit; + textBinding(1, interpolation1('', col_r3, '')); + } + } + } + }, + directives: [NgForOf] + }); + } + const fixture = new ComponentFixture(NestedLoops); + expect(ngDevMode).toHaveProperties({ + // Expect: host view + component + *ngForRow + *ngForCol + tView: 7, // should be: 4, + }); + + }); + }); }); diff --git a/packages/core/test/render3/integration_spec.ts b/packages/core/test/render3/integration_spec.ts index deb02afc3e..f95229b3c1 100644 --- a/packages/core/test/render3/integration_spec.ts +++ b/packages/core/test/render3/integration_spec.ts @@ -28,6 +28,12 @@ describe('render3 integration test', () => { elementEnd(); } } + expect(ngDevMode).toHaveProperties({ + firstTemplatePass: 1, + tNode: 1, + tView: 1, + rendererCreateElement: 1, + }); }); it('should render and update basic "Hello, World" template', () => { diff --git a/packages/core/test/render3/perfCounter_spec.ts b/packages/core/test/render3/perfCounter_spec.ts new file mode 100644 index 0000000000..b98b54ff3b --- /dev/null +++ b/packages/core/test/render3/perfCounter_spec.ts @@ -0,0 +1,46 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {ngDevModeResetPerfCounters} from '../../src/render3/ng_dev_mode'; + +beforeEach(ngDevModeResetPerfCounters); +beforeEach(() => { + jasmine.addMatchers({ + toHaveProperties: function(util, customEqualityTesters) { + return {compare: toHavePropertiesCompare}; + } + }); +}); +function toHavePropertiesCompare(actual: any, expected: any) { + let pass = true; + let errors = []; + for (let key of Object.keys(actual)) { + if (expected.hasOwnProperty(key)) { + if (actual[key] !== expected[key]) { + pass = false; + errors.push(`Expected '${key}' to be '${expected[key]}' but was '${actual[key]}'.`); + } + } + } + return {pass: pass, message: errors.join('\n')}; +} + +describe('toHaveProperties', () => { + it('should pass', () => { + expect({tNode: 1}).toHaveProperties({}); + expect({tNode: 2}).toHaveProperties({tNode: 2}); + }); + + it('should fail', () => { + expect(toHavePropertiesCompare({tNode: 2, tView: 4}, {tNode: 3, tView: 5})).toEqual({ + pass: false, + message: + 'Expected \'tNode\' to be \'3\' but was \'2\'.\nExpected \'tView\' to be \'5\' but was \'4\'.' + }); + }); +}); diff --git a/packages/types.d.ts b/packages/types.d.ts index 1ec1d9f1e9..ad0b87d802 100644 --- a/packages/types.d.ts +++ b/packages/types.d.ts @@ -18,4 +18,10 @@ /// declare let isNode: boolean; -declare let isBrowser: boolean; \ No newline at end of file +declare let isBrowser: boolean; + +declare namespace jasmine { + interface Matchers { + toHaveProperties(obj: any): boolean; + } +}