Skip to content

Commit

Permalink
fix(perf): don’t use try/catch in production mode
Browse files Browse the repository at this point in the history 
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 angular#8338
  • Loading branch information
@tbosch
tbosch committed May 19, 2016
1 parent 3e59734 commit 5d4827d
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 91 deletions.
5 changes: 4 additions & 1 deletion modules/angular2/src/compiler/identifiers.ts
View file
Original file line number Diff line number Diff line change
@@ -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 {
flattenNestedViewRenderNodes,
Expand Down Expand Up @@ -36,6 +36,7 @@ var CD_MODULE_URL = 'asset:angular2/lib/src/core/change_detection/change_detecti
// (only needed for Dart).
var impAppViewManager_ = AppViewManager_;
var impAppView = AppView;
var impDebugAppView = DebugAppView;
var impDebugContext = DebugContext;
var impAppElement = AppElement;
var impElementRef = ElementRef;
Expand Down Expand Up @@ -68,6 +69,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,
Expand Down
11 changes: 9 additions & 2 deletions modules/angular2/src/compiler/output/interpretive_view.ts
View file
Original file line number Diff line number Diff line change
@@ -1,19 +1,26 @@
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';

export class InterpretiveAppViewInstanceFactory implements InstanceFactory {
createInstance(superClass: any, clazz: any, args: any[], props: Map<string, any>,
getters: Map<string, Function>, methods: Map<string, Function>): 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<any> implements DynamicInstance {
class _InterpretiveAppView extends DebugAppView<any> implements DynamicInstance {
constructor(args: any[], public props: Map<string, any>, public getters: Map<string, Function>,
public methods: Map<string, Function>) {
super(args[0], args[1], args[2], args[3], args[4], args[5], args[6], args[7], args[8], args[9],
Expand Down
40 changes: 21 additions & 19 deletions modules/angular2/src/compiler/view_compiler/view_builder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -422,22 +422,23 @@ 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),
o.literalMap(emptyTemplateVariableBindings),
ViewConstructorVars.viewManager,
ViewConstructorVars.parentInjector,
ViewConstructorVars.declarationEl,
ChangeDetectionStrategyEnum.fromValue(getChangeDetectionMode(view)),
o.literal(view.literalArrayCount),
o.literal(view.literalMapCount),
nodeDebugInfosVar
])
.toStmt()
]);
var superConstructorArgs = [
o.variable(view.className),
renderCompTypeVar,
ViewTypeEnum.fromValue(view.viewType),
o.literalMap(emptyTemplateVariableBindings),
ViewConstructorVars.viewManager,
ViewConstructorVars.parentInjector,
ViewConstructorVars.declarationEl,
ChangeDetectionStrategyEnum.fromValue(getChangeDetectionMode(view)),
o.literal(view.literalArrayCount),
o.literal(view.literalMapCount),
];
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)],
Expand All @@ -458,9 +459,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;
}

Expand Down
6 changes: 3 additions & 3 deletions modules/angular2/src/core/linker/debug_context.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -12,7 +12,7 @@ export class StaticNodeDebugInfo {
}

export class DebugContext implements RenderDebugInfo {
constructor(private _view: AppView<any>, private _nodeIndex: number, private _tplRow: number,
constructor(private _view: DebugAppView<any>, private _nodeIndex: number, private _tplRow: number,
private _tplCol: number) {}

private get _staticNodeInfo(): StaticNodeDebugInfo {
Expand All @@ -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 = <DebugAppView<any>>componentView.declarationAppElement.parentView;
}
return isPresent(componentView.declarationAppElement) ?
componentView.declarationAppElement.nativeElement :
Expand Down
141 changes: 75 additions & 66 deletions modules/angular2/src/core/linker/view.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,11 @@ export abstract class AppView<T> {

renderer: Renderer;

private _currentDebugContext: DebugContext = null;

constructor(public clazz: any, public componentType: RenderComponentType, public type: ViewType,
public locals: {[key: string]: any}, public viewManager: AppViewManager_,
public parentInjector: Injector, public declarationAppElement: AppElement,
public cdMode: ChangeDetectionStrategy, literalArrayCacheSize: number,
literalMapCacheSize: number, public staticNodeDebugInfos: StaticNodeDebugInfo[]) {
literalMapCacheSize: number) {
this.ref = new ViewRef_(this);
if (type === ViewType.COMPONENT || type === ViewType.HOST) {
this.renderer = viewManager.renderComponent(componentType);
Expand All @@ -107,7 +105,7 @@ export abstract class AppView<T> {
this._literalMapCache = ListWrapper.createFixedSize(literalMapCacheSize);
}

create(givenProjectableNodes: Array<any | any[]>, rootSelector: string) {
create(givenProjectableNodes: Array<any | any[]>, rootSelectorOrNode: string) {
var context;
var projectableNodes;
switch (this.type) {
Expand All @@ -128,17 +126,7 @@ export abstract class AppView<T> {
}
this.context = context;
this.projectableNodes = projectableNodes;
if (this.debugMode) {
this._resetDebug();
try {
this.createInternal(rootSelector);
} catch (e) {
this._rethrowWithContext(e, e.stack);
throw e;
}
} else {
this.createInternal(rootSelector);
}
return this.createInternal(rootSelectorOrNode);
}

/**
Expand All @@ -165,17 +153,7 @@ export abstract class AppView<T> {
getHostViewElement(): AppElement { return this.namedAppElements[HOST_VIEW_ELEMENT_NAME]; }

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

/**
Expand Down Expand Up @@ -205,22 +183,12 @@ export abstract class AppView<T> {
for (var i = 0; i < children.length; i++) {
children[i].destroy();
}
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;
this.renderer.destroyView(hostElement, this.allNodes);
Expand All @@ -240,8 +208,6 @@ export abstract class AppView<T> {
*/
destroyInternal(): void {}

get debugMode(): boolean { return isPresent(this.staticNodeDebugInfos); }

get changeDetectorRef(): ChangeDetectorRef { return this.ref; }

get flatRootNodes(): any[] { return flattenNestedViewRenderNodes(this.rootNodesOrAppElements); }
Expand Down Expand Up @@ -285,17 +251,7 @@ export abstract class AppView<T> {
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;

Expand Down Expand Up @@ -357,6 +313,64 @@ export abstract class AppView<T> {
}
}

eventHandler(cb: Function): Function { return cb; }

throwDestroyedError(details: string): void { throw new ViewDestroyedException(details); }
}

export class DebugAppView<T> extends AppView<T> {
private _currentDebugContext: DebugContext = null;

constructor(clazz: any, componentType: RenderComponentType, type: ViewType,
locals: {[key: string]: any}, viewManager: AppViewManager_,
parentInjector: Injector, declarationAppElement: AppElement,
cdMode: ChangeDetectionStrategy, literalArrayCacheSize: number,
literalMapCacheSize: number, public staticNodeDebugInfos: StaticNodeDebugInfo[]) {
super(clazz, componentType, type, locals, viewManager, parentInjector, declarationAppElement, cdMode,
literalArrayCacheSize, literalMapCacheSize);
}

create(givenProjectableNodes: Array<any | any[]>,
rootSelectorOrNode: string | any): void {
this._resetDebug();
try {
return super.create(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 {
Expand All @@ -375,22 +389,17 @@ export abstract class AppView<T> {
}

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

@CONST()
Expand Down

0 comments on commit 5d4827d

Please sign in to comment.