Skip to content

Commit 4f36340

Browse files
vicbmatsko
vicb
authored and
matsko
committed
feat(ivy): add support for short-circuiting (angular#24039)
Short-circuitable expressions (using ternary & binary operators) could not use the regular binding mechanism as it relies on the bindings being checked every single time - the index is incremented as part of checking the bindings. Then for pure function kind of bindings we use a different mechanism with a fixed index. As such short circuiting a binding check does not mess with the expected binding index. Note that all pure function bindings are handled the same wether or not they actually are short-circuitable. This allows to keep the compiler and compiled code simple - and there is no runtime perf cost anyway. PR Close angular#24039
1 parent 83bb5d1 commit 4f36340

13 files changed

+362
-116
lines changed

‎packages/compiler/src/render3/r3_identifiers.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,4 +121,7 @@ export class Identifiers {
121121
static NgOnChangesFeature: o.ExternalReference = {name: 'ɵNgOnChangesFeature', moduleName: CORE};
122122

123123
static listener: o.ExternalReference = {name: 'ɵL', moduleName: CORE};
124+
125+
// Reserve slots for pure functions
126+
static reserveSlots: o.ExternalReference = {name: 'ɵrS', moduleName: CORE};
124127
}

‎packages/compiler/src/render3/view/template.ts

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
5757
// Maps of placeholder to node indexes for each of the i18n section
5858
private _phToNodeIdxes: {[phName: string]: number[]}[] = [{}];
5959

60+
// Number of slots to reserve for pureFunctions
61+
private _pureFunctionSlots = 0;
62+
6063
constructor(
6164
private constantPool: ConstantPool, private contextParameter: string,
6265
parentBindingScope: BindingScope, private level = 0, private contextName: string|null,
@@ -70,6 +73,7 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
7073
});
7174
this._valueConverter = new ValueConverter(
7275
constantPool, () => this.allocateDataSlot(),
76+
(numSlots: number): number => this._pureFunctionSlots += numSlots,
7377
(name, localName, slot, value: o.ReadVarExpr) => {
7478
const pipeType = pipeTypeByName.get(name);
7579
if (pipeType) {
@@ -139,6 +143,11 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
139143

140144
t.visitAll(this, nodes);
141145

146+
if (this._pureFunctionSlots > 0) {
147+
this.instruction(
148+
this._creationCode, null, R3.reserveSlots, o.literal(this._pureFunctionSlots));
149+
}
150+
142151
const creationCode = this._creationCode.length > 0 ?
143152
[o.ifStmt(
144153
o.variable(RENDER_FLAGS).bitwiseAnd(o.literal(core.RenderFlags.Create), null, false),
@@ -501,6 +510,7 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
501510
class ValueConverter extends AstMemoryEfficientTransformer {
502511
constructor(
503512
private constantPool: ConstantPool, private allocateSlot: () => number,
513+
private allocatePureFunctionSlots: (numSlots: number) => number,
504514
private definePipe:
505515
(name: string, localName: string, slot: number, value: o.Expression) => void) {
506516
super();
@@ -511,14 +521,20 @@ class ValueConverter extends AstMemoryEfficientTransformer {
511521
// Allocate a slot to create the pipe
512522
const slot = this.allocateSlot();
513523
const slotPseudoLocal = `PIPE:${slot}`;
524+
// Allocate one slot for the result plus one slot per pipe argument
525+
const pureFunctionSlot = this.allocatePureFunctionSlots(2 + pipe.args.length);
514526
const target = new PropertyRead(pipe.span, new ImplicitReceiver(pipe.span), slotPseudoLocal);
515527
const bindingId = pipeBinding(pipe.args);
516528
this.definePipe(pipe.name, slotPseudoLocal, slot, o.importExpr(bindingId));
517529
const value = pipe.exp.visit(this);
518530
const args = this.visitAll(pipe.args);
519531

520-
return new FunctionCall(
521-
pipe.span, target, [new LiteralPrimitive(pipe.span, slot), value, ...args]);
532+
return new FunctionCall(pipe.span, target, [
533+
new LiteralPrimitive(pipe.span, slot),
534+
new LiteralPrimitive(pipe.span, pureFunctionSlot),
535+
value,
536+
...args,
537+
]);
522538
}
523539

524540
visitLiteralArray(array: LiteralArray, context: any): AST {
@@ -527,8 +543,9 @@ class ValueConverter extends AstMemoryEfficientTransformer {
527543
// calls to literal factories that compose the literal and will cache intermediate
528544
// values. Otherwise, just return an literal array that contains the values.
529545
const literal = o.literalArr(values);
530-
return values.every(a => a.isConstant()) ? this.constantPool.getConstLiteral(literal, true) :
531-
getLiteralFactory(this.constantPool, literal);
546+
return values.every(a => a.isConstant()) ?
547+
this.constantPool.getConstLiteral(literal, true) :
548+
getLiteralFactory(this.constantPool, literal, this.allocatePureFunctionSlots);
532549
});
533550
}
534551

@@ -539,14 +556,13 @@ class ValueConverter extends AstMemoryEfficientTransformer {
539556
// values. Otherwise, just return an literal array that contains the values.
540557
const literal = o.literalMap(values.map(
541558
(value, index) => ({key: map.keys[index].key, value, quoted: map.keys[index].quoted})));
542-
return values.every(a => a.isConstant()) ? this.constantPool.getConstLiteral(literal, true) :
543-
getLiteralFactory(this.constantPool, literal);
559+
return values.every(a => a.isConstant()) ?
560+
this.constantPool.getConstLiteral(literal, true) :
561+
getLiteralFactory(this.constantPool, literal, this.allocatePureFunctionSlots);
544562
});
545563
}
546564
}
547565

548-
549-
550566
// Pipes always have at least one parameter, the value they operate on
551567
const pipeBindingIdentifiers = [R3.pipeBind1, R3.pipeBind2, R3.pipeBind3, R3.pipeBind4];
552568

@@ -559,15 +575,20 @@ const pureFunctionIdentifiers = [
559575
R3.pureFunction5, R3.pureFunction6, R3.pureFunction7, R3.pureFunction8
560576
];
561577
function getLiteralFactory(
562-
constantPool: ConstantPool, literal: o.LiteralArrayExpr | o.LiteralMapExpr): o.Expression {
578+
constantPool: ConstantPool, literal: o.LiteralArrayExpr | o.LiteralMapExpr,
579+
allocateSlots: (numSlots: number) => number): o.Expression {
563580
const {literalFactory, literalFactoryArguments} = constantPool.getLiteralFactory(literal);
581+
// Allocate 1 slot for the result plus 1 per argument
582+
const startSlot = allocateSlots(1 + literalFactoryArguments.length);
564583
literalFactoryArguments.length > 0 || error(`Expected arguments to a literal factory function`);
565584
let pureFunctionIdent =
566585
pureFunctionIdentifiers[literalFactoryArguments.length] || R3.pureFunctionV;
567586

568587
// Literal factories are pure functions that only need to be re-invoked when the parameters
569588
// change.
570-
return o.importExpr(pureFunctionIdent).callFn([literalFactory, ...literalFactoryArguments]);
589+
return o.importExpr(pureFunctionIdent).callFn([
590+
o.literal(startSlot), literalFactory, ...literalFactoryArguments
591+
]);
571592
}
572593

573594
/**

‎packages/compiler/test/render3/r3_compiler_compliance_spec.ts

Lines changed: 62 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,57 @@ describe('compiler compliance', () => {
107107
expectEmit(result.source, template, 'Incorrect template');
108108
});
109109

110+
it('should reserve slots for pure functions', () => {
111+
const files = {
112+
app: {
113+
'spec.ts': `
114+
import {Component, NgModule} from '@angular/core';
115+
116+
@Component({
117+
selector: 'my-component',
118+
template: \`<div
119+
[ternary]="cond ? [a] : [0]"
120+
[pipe]="value | pipe:1:2"
121+
[and]="cond && [b]"
122+
[or]="cond || [c]"
123+
></div>\`
124+
})
125+
export class MyComponent {
126+
id = 'one';
127+
}
128+
129+
@NgModule({declarations: [MyComponent]})
130+
export class MyModule {}
131+
`
132+
}
133+
};
134+
135+
const factory = 'factory: function MyComponent_Factory() { return new MyComponent(); }';
136+
const template = `
137+
…
138+
template: function MyComponent_Template(rf: IDENT, ctx: IDENT) {
139+
if (rf & 1) {
140+
$r3$.ɵE(0, 'div');
141+
$r3$.ɵPp(1,'pipe');
142+
$r3$.ɵe();
143+
$r3$.ɵrS(10);
144+
}
145+
if (rf & 2) {
146+
$r3$.ɵp(0, 'ternary', $r3$.ɵb((ctx.cond ? $r3$.ɵf1(2, _c0, ctx.a): _c1)));
147+
$r3$.ɵp(0, 'pipe', $r3$.ɵb($r3$.ɵpb3(6, 1, ctx.value, 1, 2)));
148+
$r3$.ɵp(0, 'and', $r3$.ɵb((ctx.cond && $r3$.ɵf1(4, _c0, ctx.b))));
149+
$r3$.ɵp(0, 'or', $r3$.ɵb((ctx.cond || $r3$.ɵf1(6, _c0, ctx.c))));
150+
}
151+
}
152+
`;
153+
154+
155+
const result = compile(files, angularFiles);
156+
157+
expectEmit(result.source, factory, 'Incorrect factory');
158+
expectEmit(result.source, template, 'Incorrect template');
159+
});
160+
110161
it('should bind to class and style names', () => {
111162
const files = {
112163
app: {
@@ -415,9 +466,10 @@ describe('compiler compliance', () => {
415466
if (rf & 1) {
416467
$r3$.ɵE(0, 'my-comp');
417468
$r3$.ɵe();
469+
$r3$.ɵrS(2);
418470
}
419471
if (rf & 2) {
420-
$r3$.ɵp(0, 'names', $r3$.ɵb($r3$.ɵf1($e0_ff$, ctx.customName)));
472+
$r3$.ɵp(0, 'names', $r3$.ɵb($r3$.ɵf1(2, $e0_ff$, ctx.customName)));
421473
}
422474
},
423475
directives: [MyComp]
@@ -494,11 +546,12 @@ describe('compiler compliance', () => {
494546
if (rf & 1) {
495547
$r3$.ɵE(0, 'my-comp');
496548
$r3$.ɵe();
549+
$r3$.ɵrS(10);
497550
}
498551
if (rf & 2) {
499552
$r3$.ɵp(
500553
0, 'names',
501-
$r3$.ɵb($r3$.ɵfV($e0_ff$, ctx.n0, ctx.n1, ctx.n2, ctx.n3, ctx.n4, ctx.n5, ctx.n6, ctx.n7, ctx.n8)));
554+
$r3$.ɵb($r3$.ɵfV(10, $e0_ff$, ctx.n0, ctx.n1, ctx.n2, ctx.n3, ctx.n4, ctx.n5, ctx.n6, ctx.n7, ctx.n8)));
502555
}
503556
},
504557
directives: [MyComp]
@@ -555,9 +608,10 @@ describe('compiler compliance', () => {
555608
if (rf & 1) {
556609
$r3$.ɵE(0, 'object-comp');
557610
$r3$.ɵe();
611+
$r3$.ɵrS(2);
558612
}
559613
if (rf & 2) {
560-
$r3$.ɵp(0, 'config', $r3$.ɵb($r3$.ɵf1($e0_ff$, ctx.name)));
614+
$r3$.ɵp(0, 'config', $r3$.ɵb($r3$.ɵf1(2, $e0_ff$, ctx.name)));
561615
}
562616
},
563617
directives: [ObjectComp]
@@ -620,12 +674,12 @@ describe('compiler compliance', () => {
620674
if (rf & 1) {
621675
$r3$.ɵE(0, 'nested-comp');
622676
$r3$.ɵe();
677+
$r3$.ɵrS(7);
623678
}
624679
if (rf & 2) {
625680
$r3$.ɵp(
626681
0, 'config',
627-
$r3$.ɵb($r3$.ɵf2(
628-
$e0_ff_2$, ctx.name, $r3$.ɵf1($e0_ff_1$, $r3$.ɵf1($e0_ff$, ctx.duration)))));
682+
$r3$.ɵb($r3$.ɵf2(7, $e0_ff_2$, ctx.name, $r3$.ɵf1(4, $e0_ff_1$, $r3$.ɵf1(2, $e0_ff$, ctx.duration)))));
629683
}
630684
},
631685
directives: [NestedComp]
@@ -912,10 +966,11 @@ describe('compiler compliance', () => {
912966
$r3$.ɵT(4);
913967
$r3$.ɵPp(5, 'myPurePipe');
914968
$r3$.ɵe();
969+
$r3$.ɵrS(9);
915970
}
916971
if (rf & 2) {
917-
$r3$.ɵt(0, $r3$.ɵi1('', $r3$.ɵpb2(1, $r3$.ɵpb2(2,ctx.name, ctx.size), ctx.size), ''));
918-
$r3$.ɵt(4, $r3$.ɵi1('', $r3$.ɵpb2(5, ctx.name, ctx.size), ''));
972+
$r3$.ɵt(0, $r3$.ɵi1('', $r3$.ɵpb2(1, 3, $r3$.ɵpb2(2, 6, ctx.name, ctx.size), ctx.size), ''));
973+
$r3$.ɵt(4, $r3$.ɵi1('', $r3$.ɵpb2(5, 9, ctx.name, ctx.size), ''));
919974
}
920975
},
921976
pipes: [MyPurePipe, MyPipe]

‎packages/core/src/core_render3_private_export.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ export {
6565
e as ɵe,
6666
p as ɵp,
6767
pD as ɵpD,
68+
rS as ɵrS,
6869
a as ɵa,
6970
s as ɵs,
7071
sn as ɵsn,

‎packages/core/src/render3/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,8 @@ export {
6464
text as T,
6565
textBinding as t,
6666

67+
reserveSlots as rS,
68+
6769
embeddedViewStart as V,
6870
embeddedViewEnd as v,
6971
detectChanges,

‎packages/core/src/render3/instructions.ts

Lines changed: 67 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1446,12 +1446,10 @@ function generateInitialInputs(
14461446
return initialInputData;
14471447
}
14481448

1449-
14501449
//////////////////////////
14511450
//// ViewContainer & View
14521451
//////////////////////////
14531452

1454-
14551453
export function createLContainer(
14561454
parentLNode: LNode, currentView: LView, template?: ComponentTemplate<any>): LContainer {
14571455
ngDevMode && assertNotNull(parentLNode, 'containers should have a parent');
@@ -2146,6 +2144,57 @@ export function bind<T>(value: T | NO_CHANGE): T|NO_CHANGE {
21462144
return changed ? value : NO_CHANGE;
21472145
}
21482146

2147+
/**
2148+
* Reserves slots for pure functions (`pureFunctionX` instructions)
2149+
*
2150+
* Binding for pure functions are store after the LNodes in the data array but before the binding.
2151+
*
2152+
* ----------------------------------------------------------------------------
2153+
* | LNodes ... | pure function bindings | regular bindings / interpolations |
2154+
* ----------------------------------------------------------------------------
2155+
* ^
2156+
* LView.bindingStartIndex
2157+
*
2158+
* Pure function instructions are given an offset from LView.bindingStartIndex.
2159+
* Subtracting the offset from LView.bindingStartIndex gives the first index where the bindings
2160+
* are stored.
2161+
*
2162+
* NOTE: reserveSlots instructions are only ever allowed at the very end of the creation block
2163+
*/
2164+
export function reserveSlots(numSlots: number) {
2165+
// Init the slots with a unique `NO_CHANGE` value so that the first change is always detected
2166+
// whether is happens or not during the first change detection pass - pure functions checks
2167+
// might be skipped when short-circuited.
2168+
data.length += numSlots;
2169+
data.fill(NO_CHANGE, -numSlots);
2170+
// We need to initialize the binding in case a `pureFunctionX` kind of binding instruction is
2171+
// called first in the update section.
2172+
initBindings();
2173+
}
2174+
2175+
/**
2176+
* Sets up the binding index before execute any `pureFunctionX` instructions.
2177+
*
2178+
* The index must be restored after the pure function is executed
2179+
*
2180+
* {@link reserveSlots}
2181+
*/
2182+
export function moveBindingIndexToReservedSlot(offset: number): number {
2183+
const currentSlot = currentView.bindingIndex;
2184+
currentView.bindingIndex = currentView.bindingStartIndex - offset;
2185+
return currentSlot;
2186+
}
2187+
2188+
/**
2189+
* Restores the binding index to the given value.
2190+
*
2191+
* This function is typically used to restore the index after a `pureFunctionX` has
2192+
* been executed.
2193+
*/
2194+
export function restoreBindingIndex(index: number): void {
2195+
currentView.bindingIndex = index;
2196+
}
2197+
21492198
/**
21502199
* Create interpolation bindings with a variable number of expressions.
21512200
*
@@ -2378,6 +2427,22 @@ function assertDataNext(index: number, arr?: any[]) {
23782427
arr.length, index, `index ${index} expected to be at the end of arr (length ${arr.length})`);
23792428
}
23802429

2430+
/**
2431+
* On the first template pass the reserved slots should be set `NO_CHANGE`.
2432+
*
2433+
* If not they might not have been actually reserved.
2434+
*/
2435+
export function assertReservedSlotInitialized(slotOffset: number, numSlots: number) {
2436+
if (firstTemplatePass) {
2437+
const startIndex = currentView.bindingStartIndex - slotOffset;
2438+
for (let i = 0; i < numSlots; i++) {
2439+
assertEqual(
2440+
data[startIndex + i], NO_CHANGE,
2441+
'The reserved slots should be set to `NO_CHANGE` on first template pass');
2442+
}
2443+
}
2444+
}
2445+
23812446
export function _getComponentHostLElementNode<T>(component: T): LElementNode {
23822447
ngDevMode && assertNotNull(component, 'expecting component got null');
23832448
const lElementNode = (component as any)[NG_HOST_SYMBOL] as LElementNode;

0 commit comments

Comments
 (0)