X Tutup
Skip to content

Commit b1a9e44

Browse files
tboschtbosch
committed
fix(perf): don’t use try/catch in production mode
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
1 parent 5297c9d commit b1a9e44

File tree

5 files changed

+113
-92
lines changed

5 files changed

+113
-92
lines changed

‎modules/angular2/src/compiler/identifiers.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {CompileIdentifierMetadata, CompileTokenMetadata} from './compile_metadata';
2-
import {AppView} from 'angular2/src/core/linker/view';
2+
import {AppView, DebugAppView} from 'angular2/src/core/linker/view';
33
import {StaticNodeDebugInfo, DebugContext} from 'angular2/src/core/linker/debug_context';
44
import {
55
ViewUtils,
@@ -47,6 +47,7 @@ var CD_MODULE_URL = 'asset:angular2/lib/src/core/change_detection/change_detecti
4747
// (only needed for Dart).
4848
var impViewUtils = ViewUtils;
4949
var impAppView = AppView;
50+
var impDebugAppView = DebugAppView;
5051
var impDebugContext = DebugContext;
5152
var impAppElement = AppElement;
5253
var impElementRef = ElementRef;
@@ -80,6 +81,8 @@ export class Identifiers {
8081
});
8182
static AppView = new CompileIdentifierMetadata(
8283
{name: 'AppView', moduleUrl: APP_VIEW_MODULE_URL, runtime: impAppView});
84+
static DebugAppView = new CompileIdentifierMetadata(
85+
{name: 'DebugAppView', moduleUrl: APP_VIEW_MODULE_URL, runtime: impDebugAppView});
8386
static AppElement = new CompileIdentifierMetadata({
8487
name: 'AppElement',
8588
moduleUrl: 'asset:angular2/lib/src/core/linker/element' + MODULE_SUFFIX,

‎modules/angular2/src/compiler/output/interpretive_view.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {isPresent} from 'angular2/src/facade/lang';
2-
import {AppView} from 'angular2/src/core/linker/view';
2+
import {AppView, DebugAppView} from 'angular2/src/core/linker/view';
33
import {AppElement} from 'angular2/src/core/linker/element';
44
import {BaseException} from 'angular2/src/facade/exceptions';
55
import {InstanceFactory, DynamicInstance} from './output_interpreter';
@@ -8,13 +8,19 @@ export class InterpretiveAppViewInstanceFactory implements InstanceFactory {
88
createInstance(superClass: any, clazz: any, args: any[], props: Map<string, any>,
99
getters: Map<string, Function>, methods: Map<string, Function>): any {
1010
if (superClass === AppView) {
11+
// We are always using DebugAppView as parent.
12+
// However, in prod mode we generate a constructor call that does
13+
// not have the argument for the debugNodeInfos.
14+
args = args.concat([null]);
15+
return new _InterpretiveAppView(args, props, getters, methods);
16+
} else if (superClass === DebugAppView) {
1117
return new _InterpretiveAppView(args, props, getters, methods);
1218
}
1319
throw new BaseException(`Can't instantiate class ${superClass} in interpretative mode`);
1420
}
1521
}
1622

17-
class _InterpretiveAppView extends AppView<any> implements DynamicInstance {
23+
class _InterpretiveAppView extends DebugAppView<any> implements DynamicInstance {
1824
constructor(args: any[], public props: Map<string, any>, public getters: Map<string, Function>,
1925
public methods: Map<string, Function>) {
2026
super(args[0], args[1], args[2], args[3], args[4], args[5], args[6], args[7]);

‎modules/angular2/src/compiler/view_compiler/view_builder.ts‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -398,19 +398,20 @@ function createViewClass(view: CompileView, renderCompTypeVar: o.ReadVarExpr,
398398
new o.FnParam(ViewConstructorVars.parentInjector.name, o.importType(Identifiers.Injector)),
399399
new o.FnParam(ViewConstructorVars.declarationEl.name, o.importType(Identifiers.AppElement))
400400
];
401-
var viewConstructor = new o.ClassMethod(null, viewConstructorArgs, [
402-
o.SUPER_EXPR.callFn([
403-
o.variable(view.className),
404-
renderCompTypeVar,
405-
ViewTypeEnum.fromValue(view.viewType),
406-
ViewConstructorVars.viewUtils,
407-
ViewConstructorVars.parentInjector,
408-
ViewConstructorVars.declarationEl,
409-
ChangeDetectionStrategyEnum.fromValue(getChangeDetectionMode(view)),
410-
nodeDebugInfosVar
411-
])
412-
.toStmt()
413-
]);
401+
var superConstructorArgs = [
402+
o.variable(view.className),
403+
renderCompTypeVar,
404+
ViewTypeEnum.fromValue(view.viewType),
405+
ViewConstructorVars.viewUtils,
406+
ViewConstructorVars.parentInjector,
407+
ViewConstructorVars.declarationEl,
408+
ChangeDetectionStrategyEnum.fromValue(getChangeDetectionMode(view))
409+
];
410+
if (view.genConfig.genDebugInfo) {
411+
superConstructorArgs.push(nodeDebugInfosVar);
412+
}
413+
var viewConstructor = new o.ClassMethod(null, viewConstructorArgs,
414+
[o.SUPER_EXPR.callFn(superConstructorArgs).toStmt()]);
414415

415416
var viewMethods = [
416417
new o.ClassMethod('createInternal', [new o.FnParam(rootSelectorVar.name, o.STRING_TYPE)],
@@ -431,9 +432,10 @@ function createViewClass(view: CompileView, renderCompTypeVar: o.ReadVarExpr,
431432
new o.ClassMethod('dirtyParentQueriesInternal', [], view.dirtyParentQueriesMethod.finish()),
432433
new o.ClassMethod('destroyInternal', [], view.destroyMethod.finish())
433434
].concat(view.eventHandlerMethods);
434-
var viewClass = new o.ClassStmt(
435-
view.className, o.importExpr(Identifiers.AppView, [getContextType(view)]), view.fields,
436-
view.getters, viewConstructor, viewMethods.filter((method) => method.body.length > 0));
435+
var superClass = view.genConfig.genDebugInfo ? Identifiers.DebugAppView : Identifiers.AppView;
436+
var viewClass = new o.ClassStmt(view.className, o.importExpr(superClass, [getContextType(view)]),
437+
view.fields, view.getters, viewConstructor,
438+
viewMethods.filter((method) => method.body.length > 0));
437439
return viewClass;
438440
}
439441

‎modules/angular2/src/core/linker/debug_context.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import {isPresent, isBlank, CONST} from 'angular2/src/facade/lang';
22
import {ListWrapper, StringMapWrapper} from 'angular2/src/facade/collection';
33
import {Injector} from 'angular2/src/core/di';
44
import {RenderDebugInfo} from 'angular2/src/core/render/api';
5-
import {AppView} from './view';
5+
import {DebugAppView} from './view';
66
import {ViewType} from './view_type';
77

88
@CONST()
@@ -12,7 +12,7 @@ export class StaticNodeDebugInfo {
1212
}
1313

1414
export class DebugContext implements RenderDebugInfo {
15-
constructor(private _view: AppView<any>, private _nodeIndex: number, private _tplRow: number,
15+
constructor(private _view: DebugAppView<any>, private _nodeIndex: number, private _tplRow: number,
1616
private _tplCol: number) {}
1717

1818
private get _staticNodeInfo(): StaticNodeDebugInfo {
@@ -31,7 +31,7 @@ export class DebugContext implements RenderDebugInfo {
3131
var componentView = this._view;
3232
while (isPresent(componentView.declarationAppElement) &&
3333
componentView.type !== ViewType.COMPONENT) {
34-
componentView = componentView.declarationAppElement.parentView;
34+
componentView = <DebugAppView<any>>componentView.declarationAppElement.parentView;
3535
}
3636
return isPresent(componentView.declarationAppElement) ?
3737
componentView.declarationAppElement.nativeElement :

‎modules/angular2/src/core/linker/view.ts‎

Lines changed: 80 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,12 @@ import {
2424
} from 'angular2/src/facade/lang';
2525

2626
import {ObservableWrapper} from 'angular2/src/facade/async';
27-
import {Renderer, RootRenderer, RenderComponentType} from 'angular2/src/core/render/api';
27+
import {
28+
Renderer,
29+
RootRenderer,
30+
RenderComponentType,
31+
RenderDebugInfo
32+
} from 'angular2/src/core/render/api';
2833
import {ViewRef_} from './view_ref';
2934

3035
import {ViewType} from './view_type';
@@ -78,16 +83,13 @@ export abstract class AppView<T> {
7883

7984
renderer: Renderer;
8085

81-
private _currentDebugContext: DebugContext = null;
82-
8386
private _hasExternalHostElement: boolean;
8487

8588
public context: T;
8689

8790
constructor(public clazz: any, public componentType: RenderComponentType, public type: ViewType,
8891
public viewUtils: ViewUtils, public parentInjector: Injector,
89-
public declarationAppElement: AppElement, public cdMode: ChangeDetectionStrategy,
90-
public staticNodeDebugInfos: StaticNodeDebugInfo[]) {
92+
public declarationAppElement: AppElement, public cdMode: ChangeDetectionStrategy) {
9193
this.ref = new ViewRef_(this);
9294
if (type === ViewType.COMPONENT || type === ViewType.HOST) {
9395
this.renderer = viewUtils.renderComponent(componentType);
@@ -115,17 +117,7 @@ export abstract class AppView<T> {
115117
}
116118
this._hasExternalHostElement = isPresent(rootSelectorOrNode);
117119
this.projectableNodes = projectableNodes;
118-
if (this.debugMode) {
119-
this._resetDebug();
120-
try {
121-
return this.createInternal(rootSelectorOrNode);
122-
} catch (e) {
123-
this._rethrowWithContext(e, e.stack);
124-
throw e;
125-
}
126-
} else {
127-
return this.createInternal(rootSelectorOrNode);
128-
}
120+
return this.createInternal(rootSelectorOrNode);
129121
}
130122

131123
/**
@@ -150,28 +142,18 @@ export abstract class AppView<T> {
150142
}
151143

152144
selectOrCreateHostElement(elementName: string, rootSelectorOrNode: string | any,
153-
debugCtx: DebugContext): any {
145+
debugInfo: RenderDebugInfo): any {
154146
var hostElement;
155147
if (isPresent(rootSelectorOrNode)) {
156-
hostElement = this.renderer.selectRootElement(rootSelectorOrNode, debugCtx);
148+
hostElement = this.renderer.selectRootElement(rootSelectorOrNode, debugInfo);
157149
} else {
158-
hostElement = this.renderer.createElement(null, elementName, debugCtx);
150+
hostElement = this.renderer.createElement(null, elementName, debugInfo);
159151
}
160152
return hostElement;
161153
}
162154

163155
injectorGet(token: any, nodeIndex: number, notFoundResult: any): any {
164-
if (this.debugMode) {
165-
this._resetDebug();
166-
try {
167-
return this.injectorGetInternal(token, nodeIndex, notFoundResult);
168-
} catch (e) {
169-
this._rethrowWithContext(e, e.stack);
170-
throw e;
171-
}
172-
} else {
173-
return this.injectorGetInternal(token, nodeIndex, notFoundResult);
174-
}
156+
return this.injectorGetInternal(token, nodeIndex, notFoundResult);
175157
}
176158

177159
/**
@@ -210,22 +192,12 @@ export abstract class AppView<T> {
210192
for (var i = 0; i < children.length; i++) {
211193
children[i]._destroyRecurse();
212194
}
213-
if (this.debugMode) {
214-
this._resetDebug();
215-
try {
216-
this._destroyLocal();
217-
} catch (e) {
218-
this._rethrowWithContext(e, e.stack);
219-
throw e;
220-
}
221-
} else {
222-
this._destroyLocal();
223-
}
195+
this.destroyLocal();
224196

225197
this.destroyed = true;
226198
}
227199

228-
private _destroyLocal() {
200+
destroyLocal() {
229201
var hostElement =
230202
this.type === ViewType.COMPONENT ? this.declarationAppElement.nativeElement : null;
231203
for (var i = 0; i < this.disposables.length; i++) {
@@ -250,8 +222,6 @@ export abstract class AppView<T> {
250222
*/
251223
destroyInternal(): void {}
252224

253-
get debugMode(): boolean { return isPresent(this.staticNodeDebugInfos); }
254-
255225
get changeDetectorRef(): ChangeDetectorRef { return this.ref; }
256226

257227
get parent(): AppView<any> {
@@ -293,17 +263,7 @@ export abstract class AppView<T> {
293263
if (this.destroyed) {
294264
this.throwDestroyedError('detectChanges');
295265
}
296-
if (this.debugMode) {
297-
this._resetDebug();
298-
try {
299-
this.detectChangesInternal(throwOnChange);
300-
} catch (e) {
301-
this._rethrowWithContext(e, e.stack);
302-
throw e;
303-
}
304-
} else {
305-
this.detectChangesInternal(throwOnChange);
306-
}
266+
this.detectChangesInternal(throwOnChange);
307267
if (this.cdMode === ChangeDetectionStrategy.CheckOnce)
308268
this.cdMode = ChangeDetectionStrategy.Checked;
309269

@@ -355,6 +315,61 @@ export abstract class AppView<T> {
355315
}
356316
}
357317

318+
eventHandler(cb: Function): Function { return cb; }
319+
320+
throwDestroyedError(details: string): void { throw new ViewDestroyedException(details); }
321+
}
322+
323+
export class DebugAppView<T> extends AppView<T> {
324+
private _currentDebugContext: DebugContext = null;
325+
326+
constructor(clazz: any, componentType: RenderComponentType, type: ViewType, viewUtils: ViewUtils,
327+
parentInjector: Injector, declarationAppElement: AppElement,
328+
cdMode: ChangeDetectionStrategy, public staticNodeDebugInfos: StaticNodeDebugInfo[]) {
329+
super(clazz, componentType, type, viewUtils, parentInjector, declarationAppElement, cdMode);
330+
}
331+
332+
create(context: T, givenProjectableNodes: Array<any | any[]>,
333+
rootSelectorOrNode: string | any): AppElement {
334+
this._resetDebug();
335+
try {
336+
return super.create(context, givenProjectableNodes, rootSelectorOrNode);
337+
} catch (e) {
338+
this._rethrowWithContext(e, e.stack);
339+
throw e;
340+
}
341+
}
342+
343+
injectorGet(token: any, nodeIndex: number, notFoundResult: any): any {
344+
this._resetDebug();
345+
try {
346+
return super.injectorGet(token, nodeIndex, notFoundResult);
347+
} catch (e) {
348+
this._rethrowWithContext(e, e.stack);
349+
throw e;
350+
}
351+
}
352+
353+
destroyLocal() {
354+
this._resetDebug();
355+
try {
356+
super.destroyLocal();
357+
} catch (e) {
358+
this._rethrowWithContext(e, e.stack);
359+
throw e;
360+
}
361+
}
362+
363+
detectChanges(throwOnChange: boolean): void {
364+
this._resetDebug();
365+
try {
366+
super.detectChanges(throwOnChange);
367+
} catch (e) {
368+
this._rethrowWithContext(e, e.stack);
369+
throw e;
370+
}
371+
}
372+
358373
private _resetDebug() { this._currentDebugContext = null; }
359374

360375
debug(nodeIndex: number, rowNum: number, colNum: number): DebugContext {
@@ -373,22 +388,17 @@ export abstract class AppView<T> {
373388
}
374389

375390
eventHandler(cb: Function): Function {
376-
if (this.debugMode) {
377-
return (event) => {
378-
this._resetDebug();
379-
try {
380-
return cb(event);
381-
} catch (e) {
382-
this._rethrowWithContext(e, e.stack);
383-
throw e;
384-
}
385-
};
386-
} else {
387-
return cb;
388-
}
391+
var superHandler = super.eventHandler(cb);
392+
return (event) => {
393+
this._resetDebug();
394+
try {
395+
return superHandler(event);
396+
} catch (e) {
397+
this._rethrowWithContext(e, e.stack);
398+
throw e;
399+
}
400+
};
389401
}
390-
391-
throwDestroyedError(details: string): void { throw new ViewDestroyedException(details); }
392402
}
393403

394404
function _findLastRenderNode(node: any): any {

0 commit comments

Comments
 (0)
X Tutup