From 6279e50d78554781111053e41cafcc8ca50ecc97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mis=CC=8Cko=20Hevery?= Date: Thu, 13 Jul 2017 15:36:11 -0700 Subject: [PATCH] perf(core): use native addEventListener for faster rendering. (#18107) Angular can make many assumptions about its event handlers. As a result the bookkeeping for native addEventListener is significantly cheaper than Zone's addEventLister which can't make such assumptions. This change bypasses the Zone's addEventListener if present and always uses the native addEventHandler. As a result registering event listeners is about 3 times faster. PR Close #18107 --- packages/core/src/view/element.ts | 9 +---- packages/core/src/view/provider.ts | 9 +---- packages/core/src/view/util.ts | 18 ++++++--- packages/core/src/zone/ng_zone.ts | 38 ++++++++++++++++-- .../core/test/view/component_view_spec.ts | 10 ++++- packages/core/test/view/element_spec.ts | 19 ++++++--- packages/core/test/zone/ng_zone_spec.ts | 7 ++++ .../src/dom/events/dom_events.ts | 40 +++++++++++++++++-- .../test/dom/events/event_manager_spec.ts | 4 +- .../platform-server/src/server_renderer.ts | 3 +- tools/public_api_guard/core/core.d.ts | 7 ++-- 11 files changed, 122 insertions(+), 42 deletions(-) diff --git a/packages/core/src/view/element.ts b/packages/core/src/view/element.ts index cf69acb758..ecc7c59181 100644 --- a/packages/core/src/view/element.ts +++ b/packages/core/src/view/element.ts @@ -189,14 +189,7 @@ export function listenToElementOutputs(view: ViewData, compView: ViewData, def: } function renderEventHandlerClosure(view: ViewData, index: number, eventName: string) { - return (event: any) => { - try { - return dispatchEvent(view, index, eventName, event); - } catch (e) { - // Attention: Don't rethrow, to keep in sync with directive events. - view.root.errorHandler.handleError(e); - } - } + return (event: any) => dispatchEvent(view, index, eventName, event); } diff --git a/packages/core/src/view/provider.ts b/packages/core/src/view/provider.ts index be71c31a1f..798d599d79 100644 --- a/packages/core/src/view/provider.ts +++ b/packages/core/src/view/provider.ts @@ -138,14 +138,7 @@ export function createDirectiveInstance(view: ViewData, def: NodeDef): any { } function eventHandlerClosure(view: ViewData, index: number, eventName: string) { - return (event: any) => { - try { - return dispatchEvent(view, index, eventName, event); - } catch (e) { - // Attention: Don't rethrow, as it would cancel Observable subscriptions! - view.root.errorHandler.handleError(e); - } - } + return (event: any) => dispatchEvent(view, index, eventName, event); } export function checkAndUpdateDirectiveInline( diff --git a/packages/core/src/view/util.ts b/packages/core/src/view/util.ts index a10511389b..48016f29f4 100644 --- a/packages/core/src/view/util.ts +++ b/packages/core/src/view/util.ts @@ -126,12 +126,18 @@ export function markParentViewsForCheckProjectedViews(view: ViewData, endView: V } export function dispatchEvent( - view: ViewData, nodeIndex: number, eventName: string, event: any): boolean { - const nodeDef = view.def.nodes[nodeIndex]; - const startView = - nodeDef.flags & NodeFlags.ComponentView ? asElementData(view, nodeIndex).componentView : view; - markParentViewsForCheck(startView); - return Services.handleEvent(view, nodeIndex, eventName, event); + view: ViewData, nodeIndex: number, eventName: string, event: any): boolean|undefined { + try { + const nodeDef = view.def.nodes[nodeIndex]; + const startView = nodeDef.flags & NodeFlags.ComponentView ? + asElementData(view, nodeIndex).componentView : + view; + markParentViewsForCheck(startView); + return Services.handleEvent(view, nodeIndex, eventName, event); + } catch (e) { + // Attention: Don't rethrow, as it would cancel Observable subscriptions! + view.root.errorHandler.handleError(e); + } } export function declaredViewContainer(view: ViewData): ElementData|null { diff --git a/packages/core/src/zone/ng_zone.ts b/packages/core/src/zone/ng_zone.ts index 8e77320e2b..1f16d2e3ca 100644 --- a/packages/core/src/zone/ng_zone.ts +++ b/packages/core/src/zone/ng_zone.ts @@ -164,13 +164,39 @@ export class NgZone { * * If a synchronous error happens it will be rethrown and not reported via `onError`. */ - run(fn: () => any): any { return (this as any as NgZonePrivate)._inner.run(fn); } + run(fn: (...args: any[]) => T, applyThis?: any, applyArgs?: any[]): T { + return (this as any as NgZonePrivate)._inner.run(fn, applyThis, applyArgs) as T; + } + + /** + * Executes the `fn` function synchronously within the Angular zone as a task and returns value + * returned by the function. + * + * Running functions via `run` allows you to reenter Angular zone from a task that was executed + * outside of the Angular zone (typically started via {@link #runOutsideAngular}). + * + * Any future tasks or microtasks scheduled from within this function will continue executing from + * within the Angular zone. + * + * If a synchronous error happens it will be rethrown and not reported via `onError`. + */ + runTask(fn: (...args: any[]) => T, applyThis?: any, applyArgs?: any[], name?: string): T { + const zone = (this as any as NgZonePrivate)._inner; + const task = zone.scheduleEventTask('NgZoneEvent: ' + name, fn, EMPTY_PAYLOAD, noop, noop); + try { + return zone.runTask(task, applyThis, applyArgs) as T; + } finally { + zone.cancelTask(task); + } + } /** * Same as `run`, except that synchronous errors are caught and forwarded via `onError` and not * rethrown. */ - runGuarded(fn: () => any): any { return (this as any as NgZonePrivate)._inner.runGuarded(fn); } + runGuarded(fn: (...args: any[]) => T, applyThis?: any, applyArgs?: any[]): T { + return (this as any as NgZonePrivate)._inner.runGuarded(fn, applyThis, applyArgs) as T; + } /** * Executes the `fn` function synchronously in Angular's parent zone and returns value returned by @@ -185,9 +211,15 @@ export class NgZone { * * Use {@link #run} to reenter the Angular zone and do work that updates the application model. */ - runOutsideAngular(fn: () => any): any { return (this as any as NgZonePrivate)._outer.run(fn); } + runOutsideAngular(fn: (...args: any[]) => T): T { + return (this as any as NgZonePrivate)._outer.run(fn) as T; + } } +function noop(){}; +const EMPTY_PAYLOAD = {}; + + interface NgZonePrivate extends NgZone { _outer: Zone; _inner: Zone; diff --git a/packages/core/test/view/component_view_spec.ts b/packages/core/test/view/component_view_spec.ts index a7e93fd3e4..017669cad1 100644 --- a/packages/core/test/view/component_view_spec.ts +++ b/packages/core/test/view/component_view_spec.ts @@ -12,6 +12,12 @@ import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter'; import {createRootView, isBrowser, recordNodeToRemove} from './helper'; +/** + * We map addEventListener to the Zones internal name. This is because we want to be fast + * and bypass the zone bookkeeping. We know that we can do the bookkeeping faster. + */ +const addEventListener = '__zone_symbol__addEventListener'; + export function main() { describe(`Component Views`, () => { function compViewDef( @@ -180,7 +186,7 @@ export function main() { const update = jasmine.createSpy('updater'); - const addListenerSpy = spyOn(HTMLElement.prototype, 'addEventListener').and.callThrough(); + const addListenerSpy = spyOn(HTMLElement.prototype, addEventListener).and.callThrough(); const {view} = createAndGetRootNodes(compViewDef( [ @@ -301,4 +307,4 @@ export function main() { }); }); -} \ No newline at end of file +} diff --git a/packages/core/test/view/element_spec.ts b/packages/core/test/view/element_spec.ts index babde20bcf..dbd3999e16 100644 --- a/packages/core/test/view/element_spec.ts +++ b/packages/core/test/view/element_spec.ts @@ -14,6 +14,13 @@ import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter'; import {ARG_TYPE_VALUES, checkNodeInlineOrDynamic, createRootView, isBrowser, recordNodeToRemove} from './helper'; +/** + * We map addEventListener to the Zones internal name. This is because we want to be fast + * and bypass the zone bookkeeping. We know that we can do the bookkeeping faster. + */ +const addEventListener = '__zone_symbol__addEventListener'; +const removeEventListener = '__zone_symbol__removeEventListener'; + export function main() { describe(`View Elements`, () => { function compViewDef( @@ -190,7 +197,7 @@ export function main() { it('should listen to DOM events', () => { const handleEventSpy = jasmine.createSpy('handleEvent'); const removeListenerSpy = - spyOn(HTMLElement.prototype, 'removeEventListener').and.callThrough(); + spyOn(HTMLElement.prototype, removeEventListener).and.callThrough(); const {view, rootNodes} = createAndAttachAndGetRootNodes(compViewDef([elementDef( NodeFlags.None, null !, null !, 0, 'button', null !, null !, [[null !, 'click']], handleEventSpy)])); @@ -210,8 +217,8 @@ export function main() { it('should listen to window events', () => { const handleEventSpy = jasmine.createSpy('handleEvent'); - const addListenerSpy = spyOn(window, 'addEventListener'); - const removeListenerSpy = spyOn(window, 'removeEventListener'); + const addListenerSpy = spyOn(window, addEventListener); + const removeListenerSpy = spyOn(window, removeEventListener); const {view, rootNodes} = createAndAttachAndGetRootNodes(compViewDef([elementDef( NodeFlags.None, null !, null !, 0, 'button', null !, null !, [['window', 'windowClick']], handleEventSpy)])); @@ -233,8 +240,8 @@ export function main() { it('should listen to document events', () => { const handleEventSpy = jasmine.createSpy('handleEvent'); - const addListenerSpy = spyOn(document, 'addEventListener'); - const removeListenerSpy = spyOn(document, 'removeEventListener'); + const addListenerSpy = spyOn(document, addEventListener); + const removeListenerSpy = spyOn(document, removeEventListener); const {view, rootNodes} = createAndAttachAndGetRootNodes(compViewDef([elementDef( NodeFlags.None, null !, null !, 0, 'button', null !, null !, [['document', 'documentClick']], handleEventSpy)])); @@ -284,7 +291,7 @@ export function main() { it('should report debug info on event errors', () => { const handleErrorSpy = spyOn(TestBed.get(ErrorHandler), 'handleError'); - const addListenerSpy = spyOn(HTMLElement.prototype, 'addEventListener').and.callThrough(); + const addListenerSpy = spyOn(HTMLElement.prototype, addEventListener).and.callThrough(); const {view, rootNodes} = createAndAttachAndGetRootNodes(compViewDef([elementDef( NodeFlags.None, null !, null !, 0, 'button', null !, null !, [[null !, 'click']], () => { throw new Error('Test'); })])); diff --git a/packages/core/test/zone/ng_zone_spec.ts b/packages/core/test/zone/ng_zone_spec.ts index cfe2dd29f6..9922da9427 100644 --- a/packages/core/test/zone/ng_zone_spec.ts +++ b/packages/core/test/zone/ng_zone_spec.ts @@ -220,6 +220,13 @@ function commonTests() { macroTask(() => { async.done(); }); }), testTimeout); + it('should return the body return value from runTask', + inject([AsyncTestCompleter], (async: AsyncTestCompleter) => { + macroTask(() => { expect(_zone.runTask(() => 6)).toEqual(6); }); + + macroTask(() => { async.done(); }); + }), testTimeout); + it('should call onUnstable and onMicrotaskEmpty', inject([AsyncTestCompleter], (async: AsyncTestCompleter) => { runNgZoneNoLog(() => macroTask(_log.fn('run'))); diff --git a/packages/platform-browser/src/dom/events/dom_events.ts b/packages/platform-browser/src/dom/events/dom_events.ts index 324603e035..3174a91dc1 100644 --- a/packages/platform-browser/src/dom/events/dom_events.ts +++ b/packages/platform-browser/src/dom/events/dom_events.ts @@ -6,22 +6,54 @@ * found in the LICENSE file at https://angular.io/license */ -import {Inject, Injectable} from '@angular/core'; +import {Inject, Injectable, NgZone, ɵglobal as global} from '@angular/core'; import {DOCUMENT} from '../dom_tokens'; import {EventManagerPlugin} from './event_manager'; +/** + * Detect if Zone is present. If it is then bypass 'addEventListener' since Angular can do much more + * efficient bookkeeping than Zone can, because we have additional information. This speeds up + * addEventListener by 3x. + */ +const Zone = global['Zone']; +const __symbol__ = Zone && Zone['__symbol__'] || function(v: T): T { + return v; +}; +const ADD_EVENT_LISTENER: 'addEventListener' = __symbol__('addEventListener'); +const REMOVE_EVENT_LISTENER: 'removeEventListener' = __symbol__('removeEventListener'); + @Injectable() export class DomEventsPlugin extends EventManagerPlugin { - constructor(@Inject(DOCUMENT) doc: any) { super(doc); } + constructor(@Inject(DOCUMENT) doc: any, private ngZone: NgZone) { super(doc); } // This plugin should come last in the list of plugins, because it accepts all // events. supports(eventName: string): boolean { return true; } addEventListener(element: HTMLElement, eventName: string, handler: Function): Function { - element.addEventListener(eventName, handler as any, false); - return () => element.removeEventListener(eventName, handler as any, false); + /** + * This code is about to add a listener to the DOM. If Zone.js is present, than + * `addEventListener` has been patched. The patched code adds overhead in both + * memory and speed (3x slower) than native. For this reason if we detect that + * Zone.js is present we bypass zone and use native addEventListener instead. + * The result is faster registration but the zone will not be restored. We do + * manual zone restoration in element.ts renderEventHandlerClosure method. + * + * NOTE: it is possible that the element is from different iframe, and so we + * have to check before we execute the method. + */ + const self = this; + let byPassZoneJS = element[ADD_EVENT_LISTENER]; + let callback: EventListener = handler as EventListener; + if (byPassZoneJS) { + callback = function() { + return self.ngZone.runTask(handler as any, null, arguments as any, eventName); + }; + } + element[byPassZoneJS ? ADD_EVENT_LISTENER : 'addEventListener'](eventName, callback, false); + return () => element[byPassZoneJS ? REMOVE_EVENT_LISTENER : 'removeEventListener']( + eventName, callback as any, false); } } diff --git a/packages/platform-browser/test/dom/events/event_manager_spec.ts b/packages/platform-browser/test/dom/events/event_manager_spec.ts index 130539b555..8d924ba190 100644 --- a/packages/platform-browser/test/dom/events/event_manager_spec.ts +++ b/packages/platform-browser/test/dom/events/event_manager_spec.ts @@ -16,12 +16,14 @@ import {el} from '../../../testing/src/browser_util'; export function main() { let domEventPlugin: DomEventsPlugin; let doc: any; + let zone: NgZone; describe('EventManager', () => { beforeEach(() => { doc = getDOM().supportsDOMEvents() ? document : getDOM().createHtmlDocument(); - domEventPlugin = new DomEventsPlugin(doc); + zone = new NgZone({}); + domEventPlugin = new DomEventsPlugin(doc, zone); }); it('should delegate event bindings to plugins that are passed in from the most generic one to the most specific one', diff --git a/packages/platform-server/src/server_renderer.ts b/packages/platform-server/src/server_renderer.ts index 1fed05cd98..086d983402 100644 --- a/packages/platform-server/src/server_renderer.ts +++ b/packages/platform-server/src/server_renderer.ts @@ -172,7 +172,8 @@ class DefaultServerRenderer2 implements Renderer2 { const el = typeof target === 'string' ? getDOM().getGlobalEventTarget(this.document, target) : target; const outsideHandler = (event: any) => this.ngZone.runGuarded(() => callback(event)); - return this.ngZone.runOutsideAngular(() => getDOM().onAndCancel(el, eventName, outsideHandler)); + return this.ngZone.runOutsideAngular( + () => getDOM().onAndCancel(el, eventName, outsideHandler) as any); } } diff --git a/tools/public_api_guard/core/core.d.ts b/tools/public_api_guard/core/core.d.ts index f4f31ca1a5..609b804790 100644 --- a/tools/public_api_guard/core/core.d.ts +++ b/tools/public_api_guard/core/core.d.ts @@ -650,9 +650,10 @@ export declare class NgZone { constructor({enableLongStackTrace}: { enableLongStackTrace?: boolean; }); - run(fn: () => any): any; - runGuarded(fn: () => any): any; - runOutsideAngular(fn: () => any): any; + run(fn: (...args: any[]) => T, applyThis?: any, applyArgs?: any[]): T; + runGuarded(fn: (...args: any[]) => T, applyThis?: any, applyArgs?: any[]): T; + runOutsideAngular(fn: (...args: any[]) => T): T; + runTask(fn: (...args: any[]) => T, applyThis?: any, applyArgs?: any[], name?: string): T; static assertInAngularZone(): void; static assertNotInAngularZone(): void; static isInAngularZone(): boolean;