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
This commit is contained in:
Pawel Kozlowski 2019-05-03 14:40:00 +02:00 committed by Kara Erickson
parent d6538eb2fd
commit b2437c4500
3 changed files with 51 additions and 28 deletions

View File

@ -41,7 +41,9 @@ export function ɵɵproperty<T>(
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<T>(value: T): T|NO_CHANGE {
export function ɵɵelementProperty<T>(
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<T>(
export function ɵɵcomponentHostSyntheticProperty<T>(
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);
}
}

View File

@ -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;
}

View File

@ -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<T>(
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);