From b2437c45007073314e1c07734ecc32f196e27801 Mon Sep 17 00:00:00 2001 From: Pawel Kozlowski Date: Fri, 3 May 2019 14:40:00 +0200 Subject: [PATCH] perf(ivy): avoid unnecessary function calls when bound value doesn't change (#30255) In the existing implementation the `elementPropertyInternal` function (meant to set element properties) was executed even if a bound value didn't change. The `elementPropertyInternal` was inspecting the incoming value and after comparing it to `NO_CHANGE` - exiting early. All in all it meant that we were unnecessarily invoking the `elementPropertyInternal` function for cases where bound value didn't change. Based on my bencharks (running change detection without any model update in a tight loop) this unnecessary function call was causing ~5% slowdown in the change detection process. PR Close #30255 --- .../core/src/render3/instructions/property.ts | 12 +++- .../instructions/property_interpolation.ts | 60 ++++++++++++------- .../core/src/render3/instructions/shared.ts | 7 +-- 3 files changed, 51 insertions(+), 28 deletions(-) diff --git a/packages/core/src/render3/instructions/property.ts b/packages/core/src/render3/instructions/property.ts index acb170f78d..4a4b14ad91 100644 --- a/packages/core/src/render3/instructions/property.ts +++ b/packages/core/src/render3/instructions/property.ts @@ -41,7 +41,9 @@ export function ɵɵproperty( const index = getSelectedIndex(); ngDevMode && assertNotEqual(index, -1, 'selected index cannot be -1'); const bindReconciledValue = ɵɵbind(value); - elementPropertyInternal(index, propName, bindReconciledValue, sanitizer, nativeOnly); + if (bindReconciledValue !== NO_CHANGE) { + elementPropertyInternal(index, propName, bindReconciledValue, sanitizer, nativeOnly); + } return ɵɵproperty; } @@ -80,7 +82,9 @@ export function ɵɵbind(value: T): T|NO_CHANGE { export function ɵɵelementProperty( index: number, propName: string, value: T | NO_CHANGE, sanitizer?: SanitizerFn | null, nativeOnly?: boolean): void { - elementPropertyInternal(index, propName, value, sanitizer, nativeOnly); + if (value !== NO_CHANGE) { + elementPropertyInternal(index, propName, value, sanitizer, nativeOnly); + } } /** @@ -109,5 +113,7 @@ export function ɵɵelementProperty( export function ɵɵcomponentHostSyntheticProperty( index: number, propName: string, value: T | NO_CHANGE, sanitizer?: SanitizerFn | null, nativeOnly?: boolean) { - elementPropertyInternal(index, propName, value, sanitizer, nativeOnly, loadComponentRenderer); + if (value !== NO_CHANGE) { + elementPropertyInternal(index, propName, value, sanitizer, nativeOnly, loadComponentRenderer); + } } diff --git a/packages/core/src/render3/instructions/property_interpolation.ts b/packages/core/src/render3/instructions/property_interpolation.ts index 67984b7c8e..044d7d2b18 100644 --- a/packages/core/src/render3/instructions/property_interpolation.ts +++ b/packages/core/src/render3/instructions/property_interpolation.ts @@ -360,7 +360,10 @@ export function ɵɵpropertyInterpolate1( propName: string, prefix: string, v0: any, suffix: string, sanitizer?: SanitizerFn): TsickleIssue1009 { const index = getSelectedIndex(); - elementPropertyInternal(index, propName, ɵɵinterpolation1(prefix, v0, suffix), sanitizer); + const interpolatedValue = ɵɵinterpolation1(prefix, v0, suffix); + if (interpolatedValue !== NO_CHANGE) { + elementPropertyInternal(index, propName, interpolatedValue, sanitizer); + } return ɵɵpropertyInterpolate1; } @@ -398,7 +401,10 @@ export function ɵɵpropertyInterpolate2( propName: string, prefix: string, v0: any, i0: string, v1: any, suffix: string, sanitizer?: SanitizerFn): TsickleIssue1009 { const index = getSelectedIndex(); - elementPropertyInternal(index, propName, ɵɵinterpolation2(prefix, v0, i0, v1, suffix), sanitizer); + const interpolatedValue = ɵɵinterpolation2(prefix, v0, i0, v1, suffix); + if (interpolatedValue !== NO_CHANGE) { + elementPropertyInternal(index, propName, interpolatedValue, sanitizer); + } return ɵɵpropertyInterpolate2; } @@ -439,8 +445,10 @@ export function ɵɵpropertyInterpolate3( propName: string, prefix: string, v0: any, i0: string, v1: any, i1: string, v2: any, suffix: string, sanitizer?: SanitizerFn): TsickleIssue1009 { const index = getSelectedIndex(); - elementPropertyInternal( - index, propName, ɵɵinterpolation3(prefix, v0, i0, v1, i1, v2, suffix), sanitizer); + const interpolatedValue = ɵɵinterpolation3(prefix, v0, i0, v1, i1, v2, suffix); + if (interpolatedValue !== NO_CHANGE) { + elementPropertyInternal(index, propName, interpolatedValue, sanitizer); + } return ɵɵpropertyInterpolate3; } @@ -483,8 +491,10 @@ export function ɵɵpropertyInterpolate4( propName: string, prefix: string, v0: any, i0: string, v1: any, i1: string, v2: any, i2: string, v3: any, suffix: string, sanitizer?: SanitizerFn): TsickleIssue1009 { const index = getSelectedIndex(); - elementPropertyInternal( - index, propName, ɵɵinterpolation4(prefix, v0, i0, v1, i1, v2, i2, v3, suffix), sanitizer); + const interpolatedValue = ɵɵinterpolation4(prefix, v0, i0, v1, i1, v2, i2, v3, suffix); + if (interpolatedValue !== NO_CHANGE) { + elementPropertyInternal(index, propName, interpolatedValue, sanitizer); + } return ɵɵpropertyInterpolate4; } @@ -529,9 +539,10 @@ export function ɵɵpropertyInterpolate5( propName: string, prefix: string, v0: any, i0: string, v1: any, i1: string, v2: any, i2: string, v3: any, i3: string, v4: any, suffix: string, sanitizer?: SanitizerFn): TsickleIssue1009 { const index = getSelectedIndex(); - elementPropertyInternal( - index, propName, ɵɵinterpolation5(prefix, v0, i0, v1, i1, v2, i2, v3, i3, v4, suffix), - sanitizer); + const interpolatedValue = ɵɵinterpolation5(prefix, v0, i0, v1, i1, v2, i2, v3, i3, v4, suffix); + if (interpolatedValue !== NO_CHANGE) { + elementPropertyInternal(index, propName, interpolatedValue, sanitizer); + } return ɵɵpropertyInterpolate5; } @@ -579,9 +590,11 @@ export function ɵɵpropertyInterpolate6( v3: any, i3: string, v4: any, i4: string, v5: any, suffix: string, sanitizer?: SanitizerFn): TsickleIssue1009 { const index = getSelectedIndex(); - elementPropertyInternal( - index, propName, ɵɵinterpolation6(prefix, v0, i0, v1, i1, v2, i2, v3, i3, v4, i4, v5, suffix), - sanitizer); + const interpolatedValue = + ɵɵinterpolation6(prefix, v0, i0, v1, i1, v2, i2, v3, i3, v4, i4, v5, suffix); + if (interpolatedValue !== NO_CHANGE) { + elementPropertyInternal(index, propName, interpolatedValue, sanitizer); + } return ɵɵpropertyInterpolate6; } @@ -631,10 +644,11 @@ export function ɵɵpropertyInterpolate7( v3: any, i3: string, v4: any, i4: string, v5: any, i5: string, v6: any, suffix: string, sanitizer?: SanitizerFn): TsickleIssue1009 { const index = getSelectedIndex(); - elementPropertyInternal( - index, propName, - ɵɵinterpolation7(prefix, v0, i0, v1, i1, v2, i2, v3, i3, v4, i4, v5, i5, v6, suffix), - sanitizer); + const interpolatedValue = + ɵɵinterpolation7(prefix, v0, i0, v1, i1, v2, i2, v3, i3, v4, i4, v5, i5, v6, suffix); + if (interpolatedValue !== NO_CHANGE) { + elementPropertyInternal(index, propName, interpolatedValue, sanitizer); + } return ɵɵpropertyInterpolate7; } @@ -686,10 +700,11 @@ export function ɵɵpropertyInterpolate8( v3: any, i3: string, v4: any, i4: string, v5: any, i5: string, v6: any, i6: string, v7: any, suffix: string, sanitizer?: SanitizerFn): TsickleIssue1009 { const index = getSelectedIndex(); - elementPropertyInternal( - index, propName, - ɵɵinterpolation8(prefix, v0, i0, v1, i1, v2, i2, v3, i3, v4, i4, v5, i5, v6, i6, v7, suffix), - sanitizer); + const interpolatedValue = + ɵɵinterpolation8(prefix, v0, i0, v1, i1, v2, i2, v3, i3, v4, i4, v5, i5, v6, i6, v7, suffix); + if (interpolatedValue !== NO_CHANGE) { + elementPropertyInternal(index, propName, interpolatedValue, sanitizer); + } return ɵɵpropertyInterpolate8; } @@ -727,6 +742,9 @@ export function ɵɵpropertyInterpolateV( propName: string, values: any[], sanitizer?: SanitizerFn): TsickleIssue1009 { const index = getSelectedIndex(); - elementPropertyInternal(index, propName, ɵɵinterpolationV(values), sanitizer); + const interpolatedValue = ɵɵinterpolationV(values); + if (interpolatedValue !== NO_CHANGE) { + elementPropertyInternal(index, propName, interpolatedValue, sanitizer); + } return ɵɵpropertyInterpolateV; } diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index f7d513d886..1a28666b42 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -11,7 +11,7 @@ import {Type} from '../../interface/type'; import {CUSTOM_ELEMENTS_SCHEMA, NO_ERRORS_SCHEMA, SchemaMetadata} from '../../metadata/schema'; import {validateAgainstEventProperties} from '../../sanitization/sanitization'; import {Sanitizer} from '../../sanitization/security'; -import {assertDataInRange, assertDefined, assertDomNode, assertEqual, assertLessThan, assertNotEqual} from '../../util/assert'; +import {assertDataInRange, assertDefined, assertDomNode, assertEqual, assertLessThan, assertNotEqual, assertNotSame} from '../../util/assert'; import {normalizeDebugBindingName, normalizeDebugBindingValue} from '../../util/ng_reflect'; import {assertLView, assertPreviousIsParent} from '../assert'; import {attachPatchData, getComponentViewByInstance} from '../context_discovery'; @@ -831,10 +831,9 @@ const ATTR_TO_PROP: {[name: string]: string} = { }; export function elementPropertyInternal( - index: number, propName: string, value: T | NO_CHANGE, sanitizer?: SanitizerFn | null, - nativeOnly?: boolean, + index: number, propName: string, value: T, sanitizer?: SanitizerFn | null, nativeOnly?: boolean, loadRendererFn?: ((tNode: TNode, lView: LView) => Renderer3) | null): void { - if (value === NO_CHANGE) return; + ngDevMode && assertNotSame(value, NO_CHANGE as any, 'Incoming value should never be NO_CHANGE.'); const lView = getLView(); const element = getNativeByIndex(index, lView) as RElement | RComment; const tNode = getTNode(index, lView);