perf(ivy): chain multiple property instructions (#31078)

Currently each property binding generates an instruction like this:

```
property('prop1', ctx.value1);
property('prop2', ctx.value2);
```

The problem is that we're repeating the call to `property` for each of the properties. Since the `property` instruction returns itself, we can chain all of the calls which is more compact and it looks like this:

```
property('prop1', ctx.value1)('prop2', ctx.value2);
```

These changes implement the chaining behavior for regular property bindings and for synthetic ones, however interpolated ones are still handled like before, because they use a separate instruction.

This PR resolves FW-1389.

PR Close #31078
This commit is contained in:
crisbeto 2019-06-19 19:50:42 +02:00 committed by Kara Erickson
parent ab86a58f27
commit 4b16d98955
4 changed files with 305 additions and 25 deletions

View File

@ -361,7 +361,6 @@ describe('compiler compliance', () => {
// `$r3$.ɵɵproperty("ternary", (ctx.cond ? $r3$.ɵɵpureFunction1(8, $c0$, ctx.a): $c1$));` // `$r3$.ɵɵproperty("ternary", (ctx.cond ? $r3$.ɵɵpureFunction1(8, $c0$, ctx.a): $c1$));`
/////////////// ///////////////
const $e0_attrs$ = [];
const factory = const factory =
'factory: function MyComponent_Factory(t) { return new (t || MyComponent)(); }'; 'factory: function MyComponent_Factory(t) { return new (t || MyComponent)(); }';
const template = ` const template = `
@ -371,10 +370,7 @@ describe('compiler compliance', () => {
$r3$.ɵɵpipe(1,"pipe"); $r3$.ɵɵpipe(1,"pipe");
} }
if (rf & 2) { if (rf & 2) {
$r3$.ɵɵproperty("ternary", ctx.cond ? $r3$.ɵɵpureFunction1(8, $c0$, ctx.a): $c1$); $r3$.ɵɵproperty("ternary", ctx.cond ? $r3$.ɵɵpureFunction1(8, $c0$, ctx.a): $c1$)("pipe", $r3$.ɵɵpipeBind3(1, 4, ctx.value, 1, 2))("and", ctx.cond && $r3$.ɵɵpureFunction1(10, $c0$, ctx.b))("or", ctx.cond || $r3$.ɵɵpureFunction1(12, $c0$, ctx.c));
$r3$.ɵɵproperty("pipe", $r3$.ɵɵpipeBind3(1, 4, ctx.value, 1, 2));
$r3$.ɵɵproperty("and", ctx.cond && $r3$.ɵɵpureFunction1(10, $c0$, ctx.b));
$r3$.ɵɵproperty("or", ctx.cond || $r3$.ɵɵpureFunction1(12, $c0$, ctx.c));
} }
} }
`; `;

View File

@ -180,6 +180,232 @@ describe('compiler compliance: bindings', () => {
expectEmit(result.source, template, 'Incorrect template'); expectEmit(result.source, template, 'Incorrect template');
}); });
it('should chain multiple property bindings into a single instruction', () => {
const files = {
app: {
'example.ts': `
import {Component} from '@angular/core';
@Component({
template: '<button [title]="myTitle" [id]="buttonId" [tabindex]="1"></button>'
})
export class MyComponent {
myTitle = 'hello';
buttonId = 'special-button';
}`
}
};
const result = compile(files, angularFiles);
const template = `
template: function MyComponent_Template(rf, ctx) {
if (rf & 2) {
$r3$.ɵɵproperty("title", ctx.myTitle)("id", ctx.buttonId)("tabindex", 1);
}
}
`;
expectEmit(result.source, template, 'Incorrect template');
});
it('should chain property bindings in the presence of other instructions', () => {
const files = {
app: {
'example.ts': `
import {Component} from '@angular/core';
@Component({
template: '<button [title]="1" [attr.id]="2" [tabindex]="3" aria-label="{{1 + 3}}"></button>'
})
export class MyComponent {}`
}
};
const result = compile(files, angularFiles);
const template = `
template: function MyComponent_Template(rf, ctx) {
if (rf & 2) {
$r3$.ɵɵattribute("id", 2);
$r3$.ɵɵpropertyInterpolate("aria-label", 1 + 3);
$r3$.ɵɵproperty("title", 1)("tabindex", 3);
}
}
`;
expectEmit(result.source, template, 'Incorrect template');
});
it('should not add interpolated properties to the property instruction chain', () => {
const files = {
app: {
'example.ts': `
import {Component} from '@angular/core';
@Component({
template: '<button [title]="1" [id]="2" tabindex="{{0 + 3}}" aria-label="hello-{{1 + 3}}-{{2 + 3}}"></button>'
})
export class MyComponent {}`
}
};
const result = compile(files, angularFiles);
const template = `
template: function MyComponent_Template(rf, ctx) {
if (rf & 2) {
$r3$.ɵɵpropertyInterpolate("tabindex", 0 + 3);
$r3$.ɵɵpropertyInterpolate2("aria-label", "hello-", 1 + 3, "-", 2 + 3, "");
$r3$.ɵɵproperty("title", 1)("id", 2);
}
}
`;
expectEmit(result.source, template, 'Incorrect template');
});
it('should chain synthetic property bindings together with regular property bindings', () => {
const files = {
app: {
'example.ts': `
import {Component} from '@angular/core';
@Component({
template: \`
<button
[title]="myTitle"
[@expand]="expansionState"
[tabindex]="1"
[@fade]="'out'"></button>
\`
})
export class MyComponent {
expansionState = 'expanded';
}`
}
};
const result = compile(files, angularFiles);
const template = `
template: function MyComponent_Template(rf, ctx) {
if (rf & 2) {
$r3$.ɵɵproperty("title", ctx.myTitle)("@expand", ctx.expansionState)("tabindex", 1)("@fade", "out");
}
}
`;
expectEmit(result.source, template, 'Incorrect template');
});
it('should chain multiple property bindings on an ng-template', () => {
const files = {
app: {
'example.ts': `
import {Component} from '@angular/core';
@Component({
template: '<ng-template [title]="myTitle" [id]="buttonId" [tabindex]="1"></ng-template>'
})
export class MyComponent {
myTitle = 'hello';
buttonId = 'custom-id';
}`
}
};
const result = compile(files, angularFiles);
const template = `
template: function MyComponent_Template(rf, ctx) {
if (rf & 2) {
$r3$.ɵɵproperty("title", ctx.myTitle)("id", ctx.buttonId)("tabindex", 1);
}
}
`;
expectEmit(result.source, template, 'Incorrect template');
});
it('should chain multiple property bindings when there are multiple elements', () => {
const files = {
app: {
'example.ts': `
import {Component} from '@angular/core';
@Component({
template: \`
<button [title]="myTitle" [id]="buttonId" [tabindex]="1"></button>
<span [id]="1" [title]="'hello'" [someProp]="1 + 2"></span>
<custom-element [prop]="'one'" [otherProp]="2"></custom-element>
\`
})
export class MyComponent {
myTitle = 'hello';
buttonId = 'special-button';
}`
}
};
const result = compile(files, angularFiles);
const template = `
template: function MyComponent_Template(rf, ctx) {
if (rf & 2) {
$r3$.ɵɵproperty("title", ctx.myTitle)("id", ctx.buttonId)("tabindex", 1);
$r3$.ɵɵselect(1);
$r3$.ɵɵproperty("id", 1)("title", "hello")("someProp", 1 + 2);
$r3$.ɵɵselect(2);
$r3$.ɵɵproperty("prop", "one")("otherProp", 2);
}
}
`;
expectEmit(result.source, template, 'Incorrect template');
});
it('should chain multiple property bindings when there are child elements', () => {
const files = {
app: {
'example.ts': `
import {Component} from '@angular/core';
@Component({
template: \`
<button [title]="myTitle" [id]="buttonId" [tabindex]="1">
<span [id]="1" [title]="'hello'" [someProp]="1 + 2"></span>
</button>\`
})
export class MyComponent {
myTitle = 'hello';
buttonId = 'special-button';
}`
}
};
const result = compile(files, angularFiles);
const template = `
template: function MyComponent_Template(rf, ctx) {
if (rf & 2) {
$r3$.ɵɵproperty("title", ctx.myTitle)("id", ctx.buttonId)("tabindex", 1);
$r3$.ɵɵselect(1);
$r3$.ɵɵproperty("id", 1)("title", "hello")("someProp", 1 + 2);
}
}
`;
expectEmit(result.source, template, 'Incorrect template');
});
}); });
describe('host bindings', () => { describe('host bindings', () => {

View File

@ -154,8 +154,7 @@ describe('r3_view_compiler', () => {
$i0$.ɵɵelement(0, "div"); $i0$.ɵɵelement(0, "div");
} }
if (rf & 2) { if (rf & 2) {
$i0$.ɵɵproperty("@attr", ); $i0$.ɵɵproperty("@attr", )("@binding", );
$i0$.ɵɵproperty("@binding", );
} }
}`; }`;
const result = compile(files, angularFiles); const result = compile(files, angularFiles);

View File

@ -712,6 +712,7 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
// special value to symbolize that there is no RHS to this binding // special value to symbolize that there is no RHS to this binding
// TODO (matsko): revisit this once FW-959 is approached // TODO (matsko): revisit this once FW-959 is approached
const emptyValueBindInstruction = o.literal(undefined); const emptyValueBindInstruction = o.literal(undefined);
const propertyBindings: ChainablePropertyBinding[] = [];
// Generate element input bindings // Generate element input bindings
allOtherInputs.forEach((input: t.BoundAttribute) => { allOtherInputs.forEach((input: t.BoundAttribute) => {
@ -729,14 +730,12 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
// defined in... // defined in...
const hasValue = value instanceof LiteralPrimitive ? !!value.value : true; const hasValue = value instanceof LiteralPrimitive ? !!value.value : true;
this.allocateBindingSlots(value); this.allocateBindingSlots(value);
const bindingName = prepareSyntheticPropertyName(input.name);
this.updateInstruction(elementIndex, input.sourceSpan, R3.property, () => { propertyBindings.push({
return [ name: prepareSyntheticPropertyName(input.name),
o.literal(bindingName), input,
(hasValue ? this.convertPropertyBinding(value, /* skipBindFn */ true) : value: () => hasValue ? this.convertPropertyBinding(value, /* skipBindFn */ true) :
emptyValueBindInstruction), emptyValueBindInstruction
];
}); });
} else { } else {
// we must skip attributes with associated i18n context, since these attributes are handled // we must skip attributes with associated i18n context, since these attributes are handled
@ -771,8 +770,12 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
params); params);
} else { } else {
// [prop]="value" // [prop]="value"
this.boundUpdateInstruction( // Collect all the properties so that we can chain into a single function at the end.
R3.property, elementIndex, attrName, input, value, params); propertyBindings.push({
name: attrName,
input,
value: () => this.convertPropertyBinding(value, true), params
});
} }
} else if (inputType === BindingType.Attribute) { } else if (inputType === BindingType.Attribute) {
if (value instanceof Interpolation && getInterpolationArgsLength(value) > 1) { if (value instanceof Interpolation && getInterpolationArgsLength(value) > 1) {
@ -799,6 +802,10 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
} }
}); });
if (propertyBindings.length > 0) {
this.propertyInstructionChain(elementIndex, propertyBindings);
}
// Traverse element child nodes // Traverse element child nodes
t.visitAll(this, element.children); t.visitAll(this, element.children);
@ -912,12 +919,12 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
}); });
// handle property bindings e.g. ɵɵproperty('ngForOf', ctx.items), et al; // handle property bindings e.g. ɵɵproperty('ngForOf', ctx.items), et al;
this.templatePropertyBindings(template, templateIndex, template.templateAttrs); this.templatePropertyBindings(templateIndex, template.templateAttrs);
// Only add normal input/output binding instructions on explicit ng-template elements. // Only add normal input/output binding instructions on explicit ng-template elements.
if (template.tagName === NG_TEMPLATE_TAG_NAME) { if (template.tagName === NG_TEMPLATE_TAG_NAME) {
// Add the input bindings // Add the input bindings
this.templatePropertyBindings(template, templateIndex, template.inputs); this.templatePropertyBindings(templateIndex, template.inputs);
// Generate listeners for directive output // Generate listeners for directive output
template.outputs.forEach((outputAst: t.BoundEvent) => { template.outputs.forEach((outputAst: t.BoundEvent) => {
this.creationInstruction( this.creationInstruction(
@ -1024,21 +1031,26 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
private bindingContext() { return `${this._bindingContext++}`; } private bindingContext() { return `${this._bindingContext++}`; }
private templatePropertyBindings( private templatePropertyBindings(
template: t.Template, templateIndex: number, attrs: (t.BoundAttribute|t.TextAttribute)[]) { templateIndex: number, attrs: (t.BoundAttribute|t.TextAttribute)[]) {
const propertyBindings: ChainablePropertyBinding[] = [];
attrs.forEach(input => { attrs.forEach(input => {
if (input instanceof t.BoundAttribute) { if (input instanceof t.BoundAttribute) {
const value = input.value.visit(this._valueConverter); const value = input.value.visit(this._valueConverter);
if (value !== undefined) { if (value !== undefined) {
this.allocateBindingSlots(value); this.allocateBindingSlots(value);
this.updateInstruction( propertyBindings.push({
templateIndex, template.sourceSpan, R3.property, name: input.name,
() => input,
[o.literal(input.name), value: () => this.convertPropertyBinding(value, /* skipBindFn */ true)
this.convertPropertyBinding(value, /* skipBindFn */ true)]); });
} }
} }
}); });
if (propertyBindings.length > 0) {
this.propertyInstructionChain(templateIndex, propertyBindings);
}
} }
// Bindings must only be resolved after all local refs have been visited, so all // Bindings must only be resolved after all local refs have been visited, so all
@ -1077,13 +1089,37 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
private updateInstruction( private updateInstruction(
nodeIndex: number, span: ParseSourceSpan|null, reference: o.ExternalReference, nodeIndex: number, span: ParseSourceSpan|null, reference: o.ExternalReference,
paramsOrFn?: o.Expression[]|(() => o.Expression[])) { paramsOrFn?: o.Expression[]|(() => o.Expression[])) {
this.addSelectInstructionIfNecessary(nodeIndex, span);
this.instructionFn(this._updateCodeFns, span, reference, paramsOrFn || []);
}
private updateInstructionChain(
nodeIndex: number, span: ParseSourceSpan|null, reference: o.ExternalReference,
callsOrFn?: o.Expression[][]|(() => o.Expression[][])) {
this.addSelectInstructionIfNecessary(nodeIndex, span);
this._updateCodeFns.push(() => {
const calls = typeof callsOrFn === 'function' ? callsOrFn() : callsOrFn;
return chainedInstruction(span, reference, calls || []).toStmt();
});
}
private propertyInstructionChain(
nodeIndex: number, propertyBindings: ChainablePropertyBinding[]) {
this.updateInstructionChain(
nodeIndex, propertyBindings.length ? propertyBindings[0].input.sourceSpan : null,
R3.property, () => {
return propertyBindings.map(
property => [o.literal(property.name), property.value(), ...(property.params || [])]);
});
}
private addSelectInstructionIfNecessary(nodeIndex: number, span: ParseSourceSpan|null) {
if (this._lastNodeIndexWithFlush < nodeIndex) { if (this._lastNodeIndexWithFlush < nodeIndex) {
if (nodeIndex > 0) { if (nodeIndex > 0) {
this.instructionFn(this._updateCodeFns, span, R3.select, [o.literal(nodeIndex)]); this.instructionFn(this._updateCodeFns, span, R3.select, [o.literal(nodeIndex)]);
} }
this._lastNodeIndexWithFlush = nodeIndex; this._lastNodeIndexWithFlush = nodeIndex;
} }
this.instructionFn(this._updateCodeFns, span, reference, paramsOrFn || []);
} }
private allocatePureFunctionSlots(numSlots: number): number { private allocatePureFunctionSlots(numSlots: number): number {
@ -1388,6 +1424,22 @@ function instruction(
return o.importExpr(reference, null, span).callFn(params, span); return o.importExpr(reference, null, span).callFn(params, span);
} }
function chainedInstruction(
span: ParseSourceSpan | null, reference: o.ExternalReference, calls: o.Expression[][]) {
let expression = o.importExpr(reference, null, span) as o.Expression;
if (calls.length > 0) {
for (let i = 0; i < calls.length; i++) {
expression = expression.callFn(calls[i], span);
}
} else {
// Add a blank invocation, in case the `calls` array is empty.
expression = expression.callFn([], span);
}
return expression;
}
// e.g. x(2); // e.g. x(2);
function generateNextContextExpr(relativeLevelDiff: number): o.Expression { function generateNextContextExpr(relativeLevelDiff: number): o.Expression {
return o.importExpr(R3.nextContext) return o.importExpr(R3.nextContext)
@ -1979,3 +2031,10 @@ function isTextNode(node: t.Node): boolean {
function hasTextChildrenOnly(children: t.Node[]): boolean { function hasTextChildrenOnly(children: t.Node[]): boolean {
return children.every(isTextNode); return children.every(isTextNode);
} }
interface ChainablePropertyBinding {
name: string;
input: t.BoundAttribute;
value: () => o.Expression;
params?: any[];
}