fix(compiler): only call pure pipes if their input changed.

This commit is contained in:
Tobias Bosch 2016-04-21 15:27:51 -07:00
parent bab81a9831
commit 8db62151d2
4 changed files with 78 additions and 9 deletions

View File

@ -124,7 +124,7 @@ export class CompileView implements NameResolver {
} }
} }
createPipe(name: string): o.Expression { callPipe(name: string, input: o.Expression, args: o.Expression[]): o.Expression {
var pipeMeta: CompilePipeMetadata = null; var pipeMeta: CompilePipeMetadata = null;
for (var i = this.pipeMetas.length - 1; i >= 0; i--) { for (var i = this.pipeMetas.length - 1; i >= 0; i--) {
var localPipeMeta = this.pipeMetas[i]; var localPipeMeta = this.pipeMetas[i];
@ -139,6 +139,7 @@ export class CompileView implements NameResolver {
} }
var pipeFieldName = pipeMeta.pure ? `_pipe_${name}` : `_pipe_${name}_${this.pipes.size}`; var pipeFieldName = pipeMeta.pure ? `_pipe_${name}` : `_pipe_${name}_${this.pipes.size}`;
var pipeExpr = this.pipes.get(pipeFieldName); var pipeExpr = this.pipes.get(pipeFieldName);
var pipeFieldCacheProp = o.THIS_EXPR.prop(`${pipeFieldName}_cache`);
if (isBlank(pipeExpr)) { if (isBlank(pipeExpr)) {
var deps = pipeMeta.type.diDeps.map((diDep) => { var deps = pipeMeta.type.diDeps.map((diDep) => {
if (diDep.token.equalsTo(identifierToken(Identifiers.ChangeDetectorRef))) { if (diDep.token.equalsTo(identifierToken(Identifiers.ChangeDetectorRef))) {
@ -148,6 +149,12 @@ export class CompileView implements NameResolver {
}); });
this.fields.push( this.fields.push(
new o.ClassField(pipeFieldName, o.importType(pipeMeta.type), [o.StmtModifier.Private])); new o.ClassField(pipeFieldName, o.importType(pipeMeta.type), [o.StmtModifier.Private]));
if (pipeMeta.pure) {
this.fields.push(new o.ClassField(pipeFieldCacheProp.name, null, [o.StmtModifier.Private]));
this.createMethod.addStmt(o.THIS_EXPR.prop(pipeFieldCacheProp.name)
.set(o.importExpr(Identifiers.uninitialized))
.toStmt());
}
this.createMethod.resetDebugInfo(null, null); this.createMethod.resetDebugInfo(null, null);
this.createMethod.addStmt(o.THIS_EXPR.prop(pipeFieldName) this.createMethod.addStmt(o.THIS_EXPR.prop(pipeFieldName)
.set(o.importExpr(pipeMeta.type).instantiate(deps)) .set(o.importExpr(pipeMeta.type).instantiate(deps))
@ -156,7 +163,15 @@ export class CompileView implements NameResolver {
this.pipes.set(pipeFieldName, pipeExpr); this.pipes.set(pipeFieldName, pipeExpr);
bindPipeDestroyLifecycleCallbacks(pipeMeta, pipeExpr, this); bindPipeDestroyLifecycleCallbacks(pipeMeta, pipeExpr, this);
} }
return pipeExpr; var callPipeExpr: o.Expression = pipeExpr.callMethod('transform', [input, o.literalArr(args)]);
if (pipeMeta.pure) {
callPipeExpr =
o.THIS_EXPR.callMethod(
'checkPurePipe',
[o.literal(this.literalArrayCount++), o.literalArr([input].concat(args))])
.conditional(pipeFieldCacheProp.set(callPipeExpr), pipeFieldCacheProp);
}
return callPipeExpr;
} }
getVariable(name: string): o.Expression { getVariable(name: string): o.Expression {

View File

@ -8,7 +8,7 @@ import {isBlank, isPresent, isArray, CONST_EXPR} from 'angular2/src/facade/lang'
var IMPLICIT_RECEIVER = o.variable('#implicit'); var IMPLICIT_RECEIVER = o.variable('#implicit');
export interface NameResolver { export interface NameResolver {
createPipe(name: string): o.Expression; callPipe(name: string, input: o.Expression, args: o.Expression[]): o.Expression;
getVariable(name: string): o.Expression; getVariable(name: string): o.Expression;
createLiteralArray(values: o.Expression[]): o.Expression; createLiteralArray(values: o.Expression[]): o.Expression;
createLiteralMap(values: Array<Array<string | o.Expression>>): o.Expression; createLiteralMap(values: Array<Array<string | o.Expression>>): o.Expression;
@ -132,13 +132,12 @@ class _AstToIrVisitor implements cdAst.AstVisitor {
ast.falseExp.visit(this, _Mode.Expression))); ast.falseExp.visit(this, _Mode.Expression)));
} }
visitPipe(ast: cdAst.BindingPipe, mode: _Mode): any { visitPipe(ast: cdAst.BindingPipe, mode: _Mode): any {
var pipeInstance = this._nameResolver.createPipe(ast.name);
var input = ast.exp.visit(this, _Mode.Expression); var input = ast.exp.visit(this, _Mode.Expression);
var args = this.visitAll(ast.args, _Mode.Expression); var args = this.visitAll(ast.args, _Mode.Expression);
var pipeResult = this._nameResolver.callPipe(ast.name, input, args);
this.needsValueUnwrapper = true; this.needsValueUnwrapper = true;
return convertToStatementIfNeeded( return convertToStatementIfNeeded(mode,
mode, this._valueUnwrapper.callMethod( this._valueUnwrapper.callMethod('unwrap', [pipeResult]));
'unwrap', [pipeInstance.callMethod('transform', [input, o.literalArr(args)])]));
} }
visitFunctionCall(ast: cdAst.FunctionCall, mode: _Mode): any { visitFunctionCall(ast: cdAst.FunctionCall, mode: _Mode): any {
return convertToStatementIfNeeded(mode, ast.target.visit(this, _Mode.Expression) return convertToStatementIfNeeded(mode, ast.target.visit(this, _Mode.Expression)

View File

@ -360,11 +360,23 @@ export abstract class AppView<T> {
this.viewContainerElement = null; this.viewContainerElement = null;
} }
checkPurePipe(id: number, newArgs: any[]): boolean {
var prevArgs = this._literalArrayCache[id];
var newPresent = isPresent(newArgs);
var prevPresent = isPresent(prevArgs);
if (newPresent !== prevPresent || (newPresent && !arrayLooseIdentical(prevArgs, newArgs))) {
this._literalArrayCache[id] = newArgs;
return true;
} else {
return false;
}
}
literalArray(id: number, value: any[]): any[] { literalArray(id: number, value: any[]): any[] {
var prevValue = this._literalArrayCache[id];
if (isBlank(value)) { if (isBlank(value)) {
return value; return value;
} }
var prevValue = this._literalArrayCache[id];
if (isBlank(prevValue) || !arrayLooseIdentical(prevValue, value)) { if (isBlank(prevValue) || !arrayLooseIdentical(prevValue, value)) {
prevValue = this._literalArrayCache[id] = value; prevValue = this._literalArrayCache[id] = value;
} }
@ -372,10 +384,10 @@ export abstract class AppView<T> {
} }
literalMap(id: number, value: {[key: string]: any}): {[key: string]: any} { literalMap(id: number, value: {[key: string]: any}): {[key: string]: any} {
var prevValue = this._literalMapCache[id];
if (isBlank(value)) { if (isBlank(value)) {
return value; return value;
} }
var prevValue = this._literalMapCache[id];
if (isBlank(prevValue) || !mapLooseIdentical(prevValue, value)) { if (isBlank(prevValue) || !mapLooseIdentical(prevValue, value)) {
prevValue = this._literalMapCache[id] = value; prevValue = this._literalMapCache[id] = value;
} }

View File

@ -491,6 +491,42 @@ export function main() {
expect(renderLog.log).toEqual(['someProp=Megatron']); expect(renderLog.log).toEqual(['someProp=Megatron']);
})); }));
it('should call pure pipes only if the arguments change', fakeAsync(() => {
var ctx = _bindSimpleValue('name | countingPipe', Person);
// change from undefined -> null
ctx.componentInstance.name = null;
ctx.detectChanges(false);
expect(renderLog.loggedValues).toEqual(['null state:0']);
ctx.detectChanges(false);
expect(renderLog.loggedValues).toEqual(['null state:0']);
// change from null -> some value
ctx.componentInstance.name = 'bob';
ctx.detectChanges(false);
expect(renderLog.loggedValues).toEqual(['null state:0', 'bob state:1']);
ctx.detectChanges(false);
expect(renderLog.loggedValues).toEqual(['null state:0', 'bob state:1']);
// change from some value -> some other value
ctx.componentInstance.name = 'bart';
ctx.detectChanges(false);
expect(renderLog.loggedValues)
.toEqual(['null state:0', 'bob state:1', 'bart state:2']);
ctx.detectChanges(false);
expect(renderLog.loggedValues)
.toEqual(['null state:0', 'bob state:1', 'bart state:2']);
}));
it('should call impure pipes on each change detection run', fakeAsync(() => {
var ctx = _bindSimpleValue('name | countingImpurePipe', Person);
ctx.componentInstance.name = 'bob';
ctx.detectChanges(false);
expect(renderLog.loggedValues).toEqual(['bob state:0']);
ctx.detectChanges(false);
expect(renderLog.loggedValues).toEqual(['bob state:0', 'bob state:1']);
}));
}); });
describe('event expressions', () => { describe('event expressions', () => {
@ -1014,6 +1050,7 @@ const ALL_DIRECTIVES = CONST_EXPR([
const ALL_PIPES = CONST_EXPR([ const ALL_PIPES = CONST_EXPR([
forwardRef(() => CountingPipe), forwardRef(() => CountingPipe),
forwardRef(() => CountingImpurePipe),
forwardRef(() => MultiArgPipe), forwardRef(() => MultiArgPipe),
forwardRef(() => PipeWithOnDestroy), forwardRef(() => PipeWithOnDestroy),
forwardRef(() => IdentityPipe), forwardRef(() => IdentityPipe),
@ -1089,6 +1126,12 @@ class CountingPipe implements PipeTransform {
transform(value, args = null) { return `${value} state:${this.state ++}`; } transform(value, args = null) { return `${value} state:${this.state ++}`; }
} }
@Pipe({name: 'countingImpurePipe', pure: false})
class CountingImpurePipe implements PipeTransform {
state: number = 0;
transform(value, args = null) { return `${value} state:${this.state ++}`; }
}
@Pipe({name: 'pipeWithOnDestroy'}) @Pipe({name: 'pipeWithOnDestroy'})
class PipeWithOnDestroy implements PipeTransform, OnDestroy { class PipeWithOnDestroy implements PipeTransform, OnDestroy {
constructor(private directiveLog: DirectiveLog) {} constructor(private directiveLog: DirectiveLog) {}