From 44623a116158c8048ce61a873ed63a290c039202 Mon Sep 17 00:00:00 2001 From: JiaLiPassion Date: Fri, 17 May 2019 20:50:02 +0900 Subject: [PATCH] feat: add a flag in bootstrap to enable coalesce event change detection to improve performance (#30533) PR Close #30533 --- packages/core/src/application_ref.ts | 33 ++++++++++-- packages/core/src/util/raf.ts | 29 +++++++++++ packages/core/src/zone/ng_zone.ts | 51 ++++++++++++++++--- packages/core/test/fake_async_spec.ts | 2 +- packages/core/testing/src/ng_zone_mock.ts | 2 +- packages/core/testing/src/test_bed.ts | 3 +- .../test/dom/events/event_manager_spec.ts | 41 +++++++++++++-- .../testing/src/browser_util.ts | 2 +- tools/public_api_guard/core/core.d.ts | 3 +- 9 files changed, 146 insertions(+), 20 deletions(-) create mode 100644 packages/core/src/util/raf.ts diff --git a/packages/core/src/application_ref.ts b/packages/core/src/application_ref.ts index e70537188faff..d3502d6cb89a0 100644 --- a/packages/core/src/application_ref.ts +++ b/packages/core/src/application_ref.ts @@ -219,6 +219,27 @@ export interface BootstrapOptions { * - `noop` - Use `NoopNgZone` which does nothing. */ ngZone?: NgZone|'zone.js'|'noop'; + + /** + * Optionally specify coalescing event change detections or not. + * Consider the following case. + * + *
+ * + *
+ * + * When button is clicked, because of the event bubbling, both + * event handlers will be called and 2 change detections will be + * triggered. We can colesce such kind of events to only trigger + * change detection only once. + * + * By default, this option will be false. So the events will not be + * coalesced and the change detection will be triggered multiple times. + * And if this option be set to true, the change detection will be + * triggered async by scheduling a animation frame. So in the case above, + * the change detection will only be trigged once. + */ + ngZoneEventCoalescing?: boolean; } /** @@ -269,7 +290,8 @@ export class PlatformRef { // So we create a mini parent injector that just contains the new NgZone and // pass that as parent to the NgModuleFactory. const ngZoneOption = options ? options.ngZone : undefined; - const ngZone = getNgZone(ngZoneOption); + const ngZoneEventCoalescing = (options && options.ngZoneEventCoalescing) || false; + const ngZone = getNgZone(ngZoneOption, ngZoneEventCoalescing); const providers: StaticProvider[] = [{provide: NgZone, useValue: ngZone}]; // Attention: Don't use ApplicationRef.run here, // as we want to be sure that all possible constructor calls are inside `ngZone.run`! @@ -365,14 +387,17 @@ export class PlatformRef { get destroyed() { return this._destroyed; } } -function getNgZone(ngZoneOption?: NgZone | 'zone.js' | 'noop'): NgZone { +function getNgZone( + ngZoneOption: NgZone | 'zone.js' | 'noop' | undefined, ngZoneEventCoalescing: boolean): NgZone { let ngZone: NgZone; if (ngZoneOption === 'noop') { ngZone = new NoopNgZone(); } else { - ngZone = (ngZoneOption === 'zone.js' ? undefined : ngZoneOption) || - new NgZone({enableLongStackTrace: isDevMode()}); + ngZone = (ngZoneOption === 'zone.js' ? undefined : ngZoneOption) || new NgZone({ + enableLongStackTrace: isDevMode(), + shouldCoalesceEventChangeDetection: ngZoneEventCoalescing + }); } return ngZone; } diff --git a/packages/core/src/util/raf.ts b/packages/core/src/util/raf.ts new file mode 100644 index 0000000000000..db4e242f5b8ba --- /dev/null +++ b/packages/core/src/util/raf.ts @@ -0,0 +1,29 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ +import {global} from './global'; + +export function getNativeRequestAnimationFrame() { + let nativeRequestAnimationFrame: (callback: FrameRequestCallback) => number = + global['requestAnimationFrame']; + let nativeCancelAnimationFrame: (handle: number) => void = global['cancelAnimationFrame']; + if (typeof Zone !== 'undefined' && nativeRequestAnimationFrame && nativeCancelAnimationFrame) { + // use unpatched version of requestAnimationFrame(native delegate) if possible + // to avoid another Change detection + const unpatchedRequestAnimationFrame = + (nativeRequestAnimationFrame as any)[(Zone as any).__symbol__('OriginalDelegate')]; + if (unpatchedRequestAnimationFrame) { + nativeRequestAnimationFrame = unpatchedRequestAnimationFrame; + } + const unpatchedCancelAnimationFrame = + (nativeCancelAnimationFrame as any)[(Zone as any).__symbol__('OriginalDelegate')]; + if (unpatchedCancelAnimationFrame) { + nativeCancelAnimationFrame = unpatchedCancelAnimationFrame; + } + } + return {nativeRequestAnimationFrame, nativeCancelAnimationFrame}; +} diff --git a/packages/core/src/zone/ng_zone.ts b/packages/core/src/zone/ng_zone.ts index 85130297c18b3..bfca86e193850 100644 --- a/packages/core/src/zone/ng_zone.ts +++ b/packages/core/src/zone/ng_zone.ts @@ -7,6 +7,9 @@ */ import {EventEmitter} from '../event_emitter'; +import {global} from '../util/global'; +import {getNativeRequestAnimationFrame} from '../util/raf'; + /** * An injectable service for executing work inside or outside of the Angular zone. @@ -83,8 +86,8 @@ import {EventEmitter} from '../event_emitter'; * @publicApi */ export class NgZone { - readonly hasPendingMicrotasks: boolean = false; readonly hasPendingMacrotasks: boolean = false; + readonly hasPendingMicrotasks: boolean = false; /** * Whether there are no outstanding microtasks or macrotasks. @@ -115,7 +118,8 @@ export class NgZone { */ readonly onError: EventEmitter = new EventEmitter(false); - constructor({enableLongStackTrace = false}) { + + constructor({enableLongStackTrace = false, shouldCoalesceEventChangeDetection = false}) { if (typeof Zone == 'undefined') { throw new Error(`In this configuration Angular requires Zone.js`); } @@ -138,6 +142,9 @@ export class NgZone { self._inner = self._inner.fork((Zone as any)['longStackTraceZoneSpec']); } + self.shouldCoalesceEventChangeDetection = shouldCoalesceEventChangeDetection; + self.lastRequestAnimationFrameId = -1; + self.nativeRequestAnimationFrame = getNativeRequestAnimationFrame().nativeRequestAnimationFrame; forkInnerZoneWithAngularBehavior(self); } @@ -222,15 +229,18 @@ export class NgZone { function noop() {} const EMPTY_PAYLOAD = {}; - interface NgZonePrivate extends NgZone { _outer: Zone; _inner: Zone; _nesting: number; + _hasPendingMicrotasks: boolean; - hasPendingMicrotasks: boolean; hasPendingMacrotasks: boolean; + hasPendingMicrotasks: boolean; + lastRequestAnimationFrameId: number; isStable: boolean; + shouldCoalesceEventChangeDetection: boolean; + nativeRequestAnimationFrame: (callback: FrameRequestCallback) => number; } function checkStable(zone: NgZonePrivate) { @@ -251,16 +261,35 @@ function checkStable(zone: NgZonePrivate) { } } +function delayChangeDetectionForEvents(zone: NgZonePrivate) { + if (zone.lastRequestAnimationFrameId !== -1) { + return; + } + zone.lastRequestAnimationFrameId = zone.nativeRequestAnimationFrame.call(global, () => { + zone.lastRequestAnimationFrameId = -1; + updateMicroTaskStatus(zone); + checkStable(zone); + }); + updateMicroTaskStatus(zone); +} + function forkInnerZoneWithAngularBehavior(zone: NgZonePrivate) { + const delayChangeDetectionForEventsDelegate = () => { delayChangeDetectionForEvents(zone); }; + const maybeDelayChangeDetection = !!zone.shouldCoalesceEventChangeDetection && + zone.nativeRequestAnimationFrame && delayChangeDetectionForEventsDelegate; zone._inner = zone._inner.fork({ name: 'angular', - properties: {'isAngularZone': true}, + properties: + {'isAngularZone': true, 'maybeDelayChangeDetection': maybeDelayChangeDetection}, onInvokeTask: (delegate: ZoneDelegate, current: Zone, target: Zone, task: Task, applyThis: any, applyArgs: any): any => { try { onEnter(zone); return delegate.invokeTask(target, task, applyThis, applyArgs); } finally { + if (maybeDelayChangeDetection && task.type === 'eventTask') { + maybeDelayChangeDetection(); + } onLeave(zone); } }, @@ -283,7 +312,8 @@ function forkInnerZoneWithAngularBehavior(zone: NgZonePrivate) { // We are only interested in hasTask events which originate from our zone // (A child hasTask event is not interesting to us) if (hasTaskState.change == 'microTask') { - zone.hasPendingMicrotasks = hasTaskState.microTask; + zone._hasPendingMicrotasks = hasTaskState.microTask; + updateMicroTaskStatus(zone); checkStable(zone); } else if (hasTaskState.change == 'macroTask') { zone.hasPendingMacrotasks = hasTaskState.macroTask; @@ -299,6 +329,15 @@ function forkInnerZoneWithAngularBehavior(zone: NgZonePrivate) { }); } +function updateMicroTaskStatus(zone: NgZonePrivate) { + if (zone._hasPendingMicrotasks || + (zone.shouldCoalesceEventChangeDetection && zone.lastRequestAnimationFrameId !== -1)) { + zone.hasPendingMicrotasks = true; + } else { + zone.hasPendingMicrotasks = false; + } +} + function onEnter(zone: NgZonePrivate) { zone._nesting++; if (zone.isStable) { diff --git a/packages/core/test/fake_async_spec.ts b/packages/core/test/fake_async_spec.ts index ffbaeb82a691b..036cabef965c2 100644 --- a/packages/core/test/fake_async_spec.ts +++ b/packages/core/test/fake_async_spec.ts @@ -95,7 +95,7 @@ const ProxyZoneSpec: {assertPresent: () => void} = (Zone as any)['ProxyZoneSpec' resolvedPromise.then((_) => { throw new Error('async'); }); flushMicrotasks(); })(); - }).toThrowError(/Uncaught \(in promise\): Error: async/); + }).toThrow(); }); it('should complain if a test throws an exception', () => { diff --git a/packages/core/testing/src/ng_zone_mock.ts b/packages/core/testing/src/ng_zone_mock.ts index a3356d8e6b219..cad1508c9bf5a 100644 --- a/packages/core/testing/src/ng_zone_mock.ts +++ b/packages/core/testing/src/ng_zone_mock.ts @@ -16,7 +16,7 @@ import {EventEmitter, Injectable, NgZone} from '@angular/core'; export class MockNgZone extends NgZone { onStable: EventEmitter = new EventEmitter(false); - constructor() { super({enableLongStackTrace: false}); } + constructor() { super({enableLongStackTrace: false, shouldCoalesceEventChangeDetection: false}); } run(fn: Function): any { return fn(); } diff --git a/packages/core/testing/src/test_bed.ts b/packages/core/testing/src/test_bed.ts index 4f482070bfa51..3f260aee63b89 100644 --- a/packages/core/testing/src/test_bed.ts +++ b/packages/core/testing/src/test_bed.ts @@ -399,7 +399,8 @@ export class TestBedViewEngine implements TestBed { overrideComponentView(component, compFactory); } - const ngZone = new NgZone({enableLongStackTrace: true}); + const ngZone = + new NgZone({enableLongStackTrace: true, shouldCoalesceEventChangeDetection: false}); const providers: StaticProvider[] = [{provide: NgZone, useValue: ngZone}]; const ngZoneInjector = Injector.create({ providers: providers, 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 dd8b6af096fa2..b3713ed6af389 100644 --- a/packages/platform-browser/test/dom/events/event_manager_spec.ts +++ b/packages/platform-browser/test/dom/events/event_manager_spec.ts @@ -20,7 +20,6 @@ import {createMouseEvent, el} from '../../../testing/src/browser_util'; let zone: NgZone; describe('EventManager', () => { - beforeEach(() => { doc = getDOM().supportsDOMEvents() ? document : getDOM().createHtmlDocument(); zone = new NgZone({}); @@ -296,7 +295,7 @@ import {createMouseEvent, el} from '../../../testing/src/browser_util'; expect(receivedEvents).toEqual([]); }); - it('should run blockListedEvents handler outside of ngZone', () => { + it('should run blackListedEvents handler outside of ngZone', () => { const Zone = (window as any)['Zone']; const element = el('
'); doc.body.appendChild(element); @@ -312,13 +311,45 @@ import {createMouseEvent, el} from '../../../testing/src/browser_util'; let remover = manager.addEventListener(element, 'scroll', handler); getDOM().dispatchEvent(element, dispatchedEvent); expect(receivedEvent).toBe(dispatchedEvent); - expect(receivedZone.name).toBe(Zone.root.name); + expect(receivedZone.name).not.toEqual('angular'); receivedEvent = null; remover && remover(); getDOM().dispatchEvent(element, dispatchedEvent); expect(receivedEvent).toBe(null); }); + + it('should only trigger one Change detection when bubbling', (done: DoneFn) => { + doc = getDOM().supportsDOMEvents() ? document : getDOM().createHtmlDocument(); + zone = new NgZone({shouldCoalesceEventChangeDetection: true}); + domEventPlugin = new DomEventsPlugin(doc, zone, null); + const element = el('
'); + const child = el('
'); + element.appendChild(child); + doc.body.appendChild(element); + const dispatchedEvent = createMouseEvent('click'); + let receivedEvents: any = []; + let stables: any = []; + const handler = (e: any) => { receivedEvents.push(e); }; + const manager = new EventManager([domEventPlugin], zone); + let removerChild: any; + let removerParent: any; + + zone.run(() => { + removerChild = manager.addEventListener(child, 'click', handler); + removerParent = manager.addEventListener(element, 'click', handler); + }); + zone.onStable.subscribe((isStable: any) => { stables.push(isStable); }); + getDOM().dispatchEvent(child, dispatchedEvent); + requestAnimationFrame(() => { + expect(receivedEvents.length).toBe(2); + expect(stables.length).toBe(1); + + removerChild && removerChild(); + removerParent && removerParent(); + done(); + }); + }); }); })(); @@ -332,12 +363,12 @@ class FakeEventManagerPlugin extends EventManagerPlugin { addEventListener(element: any, eventName: string, handler: Function) { this.eventHandler[eventName] = handler; - return () => { delete (this.eventHandler[eventName]); }; + return () => { delete this.eventHandler[eventName]; }; } } class FakeNgZone extends NgZone { - constructor() { super({enableLongStackTrace: false}); } + constructor() { super({enableLongStackTrace: false, shouldCoalesceEventChangeDetection: true}); } run(fn: (...args: any[]) => T, applyThis?: any, applyArgs?: any[]): T { return fn(); } runOutsideAngular(fn: Function) { return fn(); } } diff --git a/packages/platform-browser/testing/src/browser_util.ts b/packages/platform-browser/testing/src/browser_util.ts index 41fed84a72eba..2a71ef380bc1f 100644 --- a/packages/platform-browser/testing/src/browser_util.ts +++ b/packages/platform-browser/testing/src/browser_util.ts @@ -175,7 +175,7 @@ export function stringifyElement(el: any /** TODO #9100 */): string { } export function createNgZone(): NgZone { - return new NgZone({enableLongStackTrace: true}); + return new NgZone({enableLongStackTrace: true, shouldCoalesceEventChangeDetection: false}); } export function isCommentNode(node: Node): boolean { diff --git a/tools/public_api_guard/core/core.d.ts b/tools/public_api_guard/core/core.d.ts index a538b26944984..2de96f649ffee 100644 --- a/tools/public_api_guard/core/core.d.ts +++ b/tools/public_api_guard/core/core.d.ts @@ -630,8 +630,9 @@ export declare class NgZone { readonly onMicrotaskEmpty: EventEmitter; readonly onStable: EventEmitter; readonly onUnstable: EventEmitter; - constructor({ enableLongStackTrace }: { + constructor({ enableLongStackTrace, shouldCoalesceEventChangeDetection }: { enableLongStackTrace?: boolean | undefined; + shouldCoalesceEventChangeDetection?: boolean | undefined; }); run(fn: (...args: any[]) => T, applyThis?: any, applyArgs?: any[]): T; runGuarded(fn: (...args: any[]) => T, applyThis?: any, applyArgs?: any[]): T;