From b1a9e445b3ee4fa1cf9a7d65c920158a447256f1 Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Fri, 29 Apr 2016 09:11:57 -0700 Subject: [PATCH] =?UTF-8?q?fix(perf):=20don=E2=80=99t=20use=20`try/catch`?= =?UTF-8?q?=20in=20production=20mode?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous code that had `try/catch` statements in methods could not be optimized by Chrome. This change separates `AppView` (no `try/catch`) form `DebugAppView` (always `try/catch`). Our codegen will use `AppView` in production mode and `DebugAppView` in debug mode. Closes #8338 --- modules/angular2/src/compiler/identifiers.ts | 5 +- .../src/compiler/output/interpretive_view.ts | 10 +- .../compiler/view_compiler/view_builder.ts | 34 ++-- .../angular2/src/core/linker/debug_context.ts | 6 +- modules/angular2/src/core/linker/view.ts | 150 ++++++++++-------- 5 files changed, 113 insertions(+), 92 deletions(-) diff --git a/modules/angular2/src/compiler/identifiers.ts b/modules/angular2/src/compiler/identifiers.ts index c07d96b5af..3938001b22 100644 --- a/modules/angular2/src/compiler/identifiers.ts +++ b/modules/angular2/src/compiler/identifiers.ts @@ -1,5 +1,5 @@ import {CompileIdentifierMetadata, CompileTokenMetadata} from './compile_metadata'; -import {AppView} from 'angular2/src/core/linker/view'; +import {AppView, DebugAppView} from 'angular2/src/core/linker/view'; import {StaticNodeDebugInfo, DebugContext} from 'angular2/src/core/linker/debug_context'; import { ViewUtils, @@ -47,6 +47,7 @@ var CD_MODULE_URL = 'asset:angular2/lib/src/core/change_detection/change_detecti // (only needed for Dart). var impViewUtils = ViewUtils; var impAppView = AppView; +var impDebugAppView = DebugAppView; var impDebugContext = DebugContext; var impAppElement = AppElement; var impElementRef = ElementRef; @@ -80,6 +81,8 @@ export class Identifiers { }); static AppView = new CompileIdentifierMetadata( {name: 'AppView', moduleUrl: APP_VIEW_MODULE_URL, runtime: impAppView}); + static DebugAppView = new CompileIdentifierMetadata( + {name: 'DebugAppView', moduleUrl: APP_VIEW_MODULE_URL, runtime: impDebugAppView}); static AppElement = new CompileIdentifierMetadata({ name: 'AppElement', moduleUrl: 'asset:angular2/lib/src/core/linker/element' + MODULE_SUFFIX, diff --git a/modules/angular2/src/compiler/output/interpretive_view.ts b/modules/angular2/src/compiler/output/interpretive_view.ts index 84f6b451ed..1bccafa6ae 100644 --- a/modules/angular2/src/compiler/output/interpretive_view.ts +++ b/modules/angular2/src/compiler/output/interpretive_view.ts @@ -1,5 +1,5 @@ import {isPresent} from 'angular2/src/facade/lang'; -import {AppView} from 'angular2/src/core/linker/view'; +import {AppView, DebugAppView} from 'angular2/src/core/linker/view'; import {AppElement} from 'angular2/src/core/linker/element'; import {BaseException} from 'angular2/src/facade/exceptions'; import {InstanceFactory, DynamicInstance} from './output_interpreter'; @@ -8,13 +8,19 @@ export class InterpretiveAppViewInstanceFactory implements InstanceFactory { createInstance(superClass: any, clazz: any, args: any[], props: Map, getters: Map, methods: Map): any { if (superClass === AppView) { + // We are always using DebugAppView as parent. + // However, in prod mode we generate a constructor call that does + // not have the argument for the debugNodeInfos. + args = args.concat([null]); + return new _InterpretiveAppView(args, props, getters, methods); + } else if (superClass === DebugAppView) { return new _InterpretiveAppView(args, props, getters, methods); } throw new BaseException(`Can't instantiate class ${superClass} in interpretative mode`); } } -class _InterpretiveAppView extends AppView implements DynamicInstance { +class _InterpretiveAppView extends DebugAppView implements DynamicInstance { constructor(args: any[], public props: Map, public getters: Map, public methods: Map) { super(args[0], args[1], args[2], args[3], args[4], args[5], args[6], args[7]); diff --git a/modules/angular2/src/compiler/view_compiler/view_builder.ts b/modules/angular2/src/compiler/view_compiler/view_builder.ts index 63d505ea95..1b6ca5a1e0 100644 --- a/modules/angular2/src/compiler/view_compiler/view_builder.ts +++ b/modules/angular2/src/compiler/view_compiler/view_builder.ts @@ -398,19 +398,20 @@ function createViewClass(view: CompileView, renderCompTypeVar: o.ReadVarExpr, new o.FnParam(ViewConstructorVars.parentInjector.name, o.importType(Identifiers.Injector)), new o.FnParam(ViewConstructorVars.declarationEl.name, o.importType(Identifiers.AppElement)) ]; - var viewConstructor = new o.ClassMethod(null, viewConstructorArgs, [ - o.SUPER_EXPR.callFn([ - o.variable(view.className), - renderCompTypeVar, - ViewTypeEnum.fromValue(view.viewType), - ViewConstructorVars.viewUtils, - ViewConstructorVars.parentInjector, - ViewConstructorVars.declarationEl, - ChangeDetectionStrategyEnum.fromValue(getChangeDetectionMode(view)), - nodeDebugInfosVar - ]) - .toStmt() - ]); + var superConstructorArgs = [ + o.variable(view.className), + renderCompTypeVar, + ViewTypeEnum.fromValue(view.viewType), + ViewConstructorVars.viewUtils, + ViewConstructorVars.parentInjector, + ViewConstructorVars.declarationEl, + ChangeDetectionStrategyEnum.fromValue(getChangeDetectionMode(view)) + ]; + if (view.genConfig.genDebugInfo) { + superConstructorArgs.push(nodeDebugInfosVar); + } + var viewConstructor = new o.ClassMethod(null, viewConstructorArgs, + [o.SUPER_EXPR.callFn(superConstructorArgs).toStmt()]); var viewMethods = [ new o.ClassMethod('createInternal', [new o.FnParam(rootSelectorVar.name, o.STRING_TYPE)], @@ -431,9 +432,10 @@ function createViewClass(view: CompileView, renderCompTypeVar: o.ReadVarExpr, new o.ClassMethod('dirtyParentQueriesInternal', [], view.dirtyParentQueriesMethod.finish()), new o.ClassMethod('destroyInternal', [], view.destroyMethod.finish()) ].concat(view.eventHandlerMethods); - var viewClass = new o.ClassStmt( - view.className, o.importExpr(Identifiers.AppView, [getContextType(view)]), view.fields, - view.getters, viewConstructor, viewMethods.filter((method) => method.body.length > 0)); + var superClass = view.genConfig.genDebugInfo ? Identifiers.DebugAppView : Identifiers.AppView; + var viewClass = new o.ClassStmt(view.className, o.importExpr(superClass, [getContextType(view)]), + view.fields, view.getters, viewConstructor, + viewMethods.filter((method) => method.body.length > 0)); return viewClass; } diff --git a/modules/angular2/src/core/linker/debug_context.ts b/modules/angular2/src/core/linker/debug_context.ts index fb52eab8ba..d5e1b134fa 100644 --- a/modules/angular2/src/core/linker/debug_context.ts +++ b/modules/angular2/src/core/linker/debug_context.ts @@ -2,7 +2,7 @@ import {isPresent, isBlank, CONST} from 'angular2/src/facade/lang'; import {ListWrapper, StringMapWrapper} from 'angular2/src/facade/collection'; import {Injector} from 'angular2/src/core/di'; import {RenderDebugInfo} from 'angular2/src/core/render/api'; -import {AppView} from './view'; +import {DebugAppView} from './view'; import {ViewType} from './view_type'; @CONST() @@ -12,7 +12,7 @@ export class StaticNodeDebugInfo { } export class DebugContext implements RenderDebugInfo { - constructor(private _view: AppView, private _nodeIndex: number, private _tplRow: number, + constructor(private _view: DebugAppView, private _nodeIndex: number, private _tplRow: number, private _tplCol: number) {} private get _staticNodeInfo(): StaticNodeDebugInfo { @@ -31,7 +31,7 @@ export class DebugContext implements RenderDebugInfo { var componentView = this._view; while (isPresent(componentView.declarationAppElement) && componentView.type !== ViewType.COMPONENT) { - componentView = componentView.declarationAppElement.parentView; + componentView = >componentView.declarationAppElement.parentView; } return isPresent(componentView.declarationAppElement) ? componentView.declarationAppElement.nativeElement : diff --git a/modules/angular2/src/core/linker/view.ts b/modules/angular2/src/core/linker/view.ts index 1f842ec322..e82e8fd5e0 100644 --- a/modules/angular2/src/core/linker/view.ts +++ b/modules/angular2/src/core/linker/view.ts @@ -24,7 +24,12 @@ import { } from 'angular2/src/facade/lang'; import {ObservableWrapper} from 'angular2/src/facade/async'; -import {Renderer, RootRenderer, RenderComponentType} from 'angular2/src/core/render/api'; +import { + Renderer, + RootRenderer, + RenderComponentType, + RenderDebugInfo +} from 'angular2/src/core/render/api'; import {ViewRef_} from './view_ref'; import {ViewType} from './view_type'; @@ -78,16 +83,13 @@ export abstract class AppView { renderer: Renderer; - private _currentDebugContext: DebugContext = null; - private _hasExternalHostElement: boolean; public context: T; constructor(public clazz: any, public componentType: RenderComponentType, public type: ViewType, public viewUtils: ViewUtils, public parentInjector: Injector, - public declarationAppElement: AppElement, public cdMode: ChangeDetectionStrategy, - public staticNodeDebugInfos: StaticNodeDebugInfo[]) { + public declarationAppElement: AppElement, public cdMode: ChangeDetectionStrategy) { this.ref = new ViewRef_(this); if (type === ViewType.COMPONENT || type === ViewType.HOST) { this.renderer = viewUtils.renderComponent(componentType); @@ -115,17 +117,7 @@ export abstract class AppView { } this._hasExternalHostElement = isPresent(rootSelectorOrNode); this.projectableNodes = projectableNodes; - if (this.debugMode) { - this._resetDebug(); - try { - return this.createInternal(rootSelectorOrNode); - } catch (e) { - this._rethrowWithContext(e, e.stack); - throw e; - } - } else { - return this.createInternal(rootSelectorOrNode); - } + return this.createInternal(rootSelectorOrNode); } /** @@ -150,28 +142,18 @@ export abstract class AppView { } selectOrCreateHostElement(elementName: string, rootSelectorOrNode: string | any, - debugCtx: DebugContext): any { + debugInfo: RenderDebugInfo): any { var hostElement; if (isPresent(rootSelectorOrNode)) { - hostElement = this.renderer.selectRootElement(rootSelectorOrNode, debugCtx); + hostElement = this.renderer.selectRootElement(rootSelectorOrNode, debugInfo); } else { - hostElement = this.renderer.createElement(null, elementName, debugCtx); + hostElement = this.renderer.createElement(null, elementName, debugInfo); } return hostElement; } injectorGet(token: any, nodeIndex: number, notFoundResult: any): any { - if (this.debugMode) { - this._resetDebug(); - try { - return this.injectorGetInternal(token, nodeIndex, notFoundResult); - } catch (e) { - this._rethrowWithContext(e, e.stack); - throw e; - } - } else { - return this.injectorGetInternal(token, nodeIndex, notFoundResult); - } + return this.injectorGetInternal(token, nodeIndex, notFoundResult); } /** @@ -210,22 +192,12 @@ export abstract class AppView { for (var i = 0; i < children.length; i++) { children[i]._destroyRecurse(); } - if (this.debugMode) { - this._resetDebug(); - try { - this._destroyLocal(); - } catch (e) { - this._rethrowWithContext(e, e.stack); - throw e; - } - } else { - this._destroyLocal(); - } + this.destroyLocal(); this.destroyed = true; } - private _destroyLocal() { + destroyLocal() { var hostElement = this.type === ViewType.COMPONENT ? this.declarationAppElement.nativeElement : null; for (var i = 0; i < this.disposables.length; i++) { @@ -250,8 +222,6 @@ export abstract class AppView { */ destroyInternal(): void {} - get debugMode(): boolean { return isPresent(this.staticNodeDebugInfos); } - get changeDetectorRef(): ChangeDetectorRef { return this.ref; } get parent(): AppView { @@ -293,17 +263,7 @@ export abstract class AppView { if (this.destroyed) { this.throwDestroyedError('detectChanges'); } - if (this.debugMode) { - this._resetDebug(); - try { - this.detectChangesInternal(throwOnChange); - } catch (e) { - this._rethrowWithContext(e, e.stack); - throw e; - } - } else { - this.detectChangesInternal(throwOnChange); - } + this.detectChangesInternal(throwOnChange); if (this.cdMode === ChangeDetectionStrategy.CheckOnce) this.cdMode = ChangeDetectionStrategy.Checked; @@ -355,6 +315,61 @@ export abstract class AppView { } } + eventHandler(cb: Function): Function { return cb; } + + throwDestroyedError(details: string): void { throw new ViewDestroyedException(details); } +} + +export class DebugAppView extends AppView { + private _currentDebugContext: DebugContext = null; + + constructor(clazz: any, componentType: RenderComponentType, type: ViewType, viewUtils: ViewUtils, + parentInjector: Injector, declarationAppElement: AppElement, + cdMode: ChangeDetectionStrategy, public staticNodeDebugInfos: StaticNodeDebugInfo[]) { + super(clazz, componentType, type, viewUtils, parentInjector, declarationAppElement, cdMode); + } + + create(context: T, givenProjectableNodes: Array, + rootSelectorOrNode: string | any): AppElement { + this._resetDebug(); + try { + return super.create(context, givenProjectableNodes, rootSelectorOrNode); + } catch (e) { + this._rethrowWithContext(e, e.stack); + throw e; + } + } + + injectorGet(token: any, nodeIndex: number, notFoundResult: any): any { + this._resetDebug(); + try { + return super.injectorGet(token, nodeIndex, notFoundResult); + } catch (e) { + this._rethrowWithContext(e, e.stack); + throw e; + } + } + + destroyLocal() { + this._resetDebug(); + try { + super.destroyLocal(); + } catch (e) { + this._rethrowWithContext(e, e.stack); + throw e; + } + } + + detectChanges(throwOnChange: boolean): void { + this._resetDebug(); + try { + super.detectChanges(throwOnChange); + } catch (e) { + this._rethrowWithContext(e, e.stack); + throw e; + } + } + private _resetDebug() { this._currentDebugContext = null; } debug(nodeIndex: number, rowNum: number, colNum: number): DebugContext { @@ -373,22 +388,17 @@ export abstract class AppView { } eventHandler(cb: Function): Function { - if (this.debugMode) { - return (event) => { - this._resetDebug(); - try { - return cb(event); - } catch (e) { - this._rethrowWithContext(e, e.stack); - throw e; - } - }; - } else { - return cb; - } + var superHandler = super.eventHandler(cb); + return (event) => { + this._resetDebug(); + try { + return superHandler(event); + } catch (e) { + this._rethrowWithContext(e, e.stack); + throw e; + } + }; } - - throwDestroyedError(details: string): void { throw new ViewDestroyedException(details); } } function _findLastRenderNode(node: any): any {