From 21c1e14385a61b4fdd3c5973abcaa3eace1303c1 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 --- integration/_payload-limits.json | 2 +- .../side-effects/snapshots/core/esm5.js | 17 ++++++ packages/core/src/application_ref.ts | 33 +++++++++-- packages/core/src/util/raf.ts | 29 ++++++++++ packages/core/src/zone/ng_zone.ts | 55 +++++++++++++++++-- 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 | 6 +- 11 files changed, 171 insertions(+), 21 deletions(-) create mode 100644 packages/core/src/util/raf.ts diff --git a/integration/_payload-limits.json b/integration/_payload-limits.json index 614fd48e36..f78f88d5ca 100644 --- a/integration/_payload-limits.json +++ b/integration/_payload-limits.json @@ -21,7 +21,7 @@ "master": { "uncompressed": { "runtime": 1440, - "main": 123904, + "main": 125178, "polyfills": 45340 } } diff --git a/integration/side-effects/snapshots/core/esm5.js b/integration/side-effects/snapshots/core/esm5.js index e969f1091b..16cfc71279 100644 --- a/integration/side-effects/snapshots/core/esm5.js +++ b/integration/side-effects/snapshots/core/esm5.js @@ -14,6 +14,23 @@ var __global = "undefined" !== typeof global && global; var _global = __globalThis || __global || __window || __self; +function getNativeRequestAnimationFrame() { + var nativeRequestAnimationFrame = _global["requestAnimationFrame"]; + var nativeCancelAnimationFrame = _global["cancelAnimationFrame"]; + if ("undefined" !== typeof Zone && nativeRequestAnimationFrame && nativeCancelAnimationFrame) { + var unpatchedRequestAnimationFrame = nativeRequestAnimationFrame[Zone.__symbol__("OriginalDelegate")]; + if (unpatchedRequestAnimationFrame) nativeRequestAnimationFrame = unpatchedRequestAnimationFrame; + var unpatchedCancelAnimationFrame = nativeCancelAnimationFrame[Zone.__symbol__("OriginalDelegate")]; + if (unpatchedCancelAnimationFrame) nativeCancelAnimationFrame = unpatchedCancelAnimationFrame; + } + return { + nativeRequestAnimationFrame: nativeRequestAnimationFrame, + nativeCancelAnimationFrame: nativeCancelAnimationFrame + }; +} + +var nativeRequestAnimationFrame = getNativeRequestAnimationFrame().nativeRequestAnimationFrame; + if (ngDevMode) _global.$localize = _global.$localize || function() { throw new Error("It looks like your application or one of its dependencies is using i18n.\n" + "Angular 9 introduced a global `$localize()` function that needs to be loaded.\n" + "Please add `import '@angular/localize/init';` to your polyfills.ts file."); }; diff --git a/packages/core/src/application_ref.ts b/packages/core/src/application_ref.ts index e70537188f..520cd2e2e9 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 + * colesced 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 0000000000..db4e242f5b --- /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 85130297c1..14321fe032 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,11 @@ import {EventEmitter} from '../event_emitter'; * @publicApi */ export class NgZone { - readonly hasPendingMicrotasks: boolean = false; + readonly hasPendingZoneMicrotasks: boolean = false; + readonly lastRequestAnimationFrameId: number = -1; + readonly shouldCoalesceEventChangeDetection: boolean = true; readonly hasPendingMacrotasks: boolean = false; + readonly hasPendingMicrotasks: boolean = false; /** * Whether there are no outstanding microtasks or macrotasks. @@ -115,7 +121,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 +145,7 @@ export class NgZone { self._inner = self._inner.fork((Zone as any)['longStackTraceZoneSpec']); } + self.shouldCoalesceEventChangeDetection = shouldCoalesceEventChangeDetection; forkInnerZoneWithAngularBehavior(self); } @@ -221,16 +229,19 @@ export class NgZone { function noop() {} const EMPTY_PAYLOAD = {}; - +const {nativeRequestAnimationFrame} = getNativeRequestAnimationFrame(); interface NgZonePrivate extends NgZone { _outer: Zone; _inner: Zone; _nesting: number; + _hasPendingMicrotasks: boolean; - hasPendingMicrotasks: boolean; hasPendingMacrotasks: boolean; + hasPendingMicrotasks: boolean; + lastRequestAnimationFrameId: number; isStable: boolean; + shouldCoalesceEventChangeDetection: boolean; } function checkStable(zone: NgZonePrivate) { @@ -251,16 +262,35 @@ function checkStable(zone: NgZonePrivate) { } } +function delayChangeDetectionForEvents(zone: NgZonePrivate) { + if (zone.lastRequestAnimationFrameId !== -1) { + return; + } + zone.lastRequestAnimationFrameId = nativeRequestAnimationFrame.call(global, () => { + zone.lastRequestAnimationFrameId = -1; + updateMicroTaskStatus(zone); + checkStable(zone); + }); + updateMicroTaskStatus(zone); +} + function forkInnerZoneWithAngularBehavior(zone: NgZonePrivate) { + const delayChangeDetectionForEventsDelegate = () => { delayChangeDetectionForEvents(zone); }; + const maybeDelayChangeDetection = !!zone.shouldCoalesceEventChangeDetection && + 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 +313,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 +330,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) { @@ -317,6 +357,8 @@ function onLeave(zone: NgZonePrivate) { * to framework to perform rendering. */ export class NoopNgZone implements NgZone { + readonly hasPendingZoneMicrotasks: boolean = false; + readonly lastRequestAnimationFrameId = -1; readonly hasPendingMicrotasks: boolean = false; readonly hasPendingMacrotasks: boolean = false; readonly isStable: boolean = true; @@ -324,6 +366,7 @@ export class NoopNgZone implements NgZone { readonly onMicrotaskEmpty: EventEmitter = new EventEmitter(); readonly onStable: EventEmitter = new EventEmitter(); readonly onError: EventEmitter = new EventEmitter(); + readonly shouldCoalesceEventChangeDetection: boolean = false; run(fn: (...args: any[]) => any, applyThis?: any, applyArgs?: any): any { return fn.apply(applyThis, applyArgs); diff --git a/packages/core/test/fake_async_spec.ts b/packages/core/test/fake_async_spec.ts index ffbaeb82a6..036cabef96 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 a3356d8e6b..cad1508c9b 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 bc1f3bc98d..469d981dfe 100644 --- a/packages/core/testing/src/test_bed.ts +++ b/packages/core/testing/src/test_bed.ts @@ -402,7 +402,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 dd8b6af096..b3713ed6af 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 41fed84a72..2a71ef380b 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 ded0c9951d..0e70fa2273 100644 --- a/tools/public_api_guard/core/core.d.ts +++ b/tools/public_api_guard/core/core.d.ts @@ -624,13 +624,17 @@ export declare class NgProbeToken { export declare class NgZone { readonly hasPendingMacrotasks: boolean; readonly hasPendingMicrotasks: boolean; + readonly hasPendingZoneMicrotasks: boolean; readonly isStable: boolean; + readonly lastRequestAnimationFrameId: number; readonly onError: EventEmitter; readonly onMicrotaskEmpty: EventEmitter; readonly onStable: EventEmitter; readonly onUnstable: EventEmitter; - constructor({ enableLongStackTrace }: { + readonly shouldCoalesceEventChangeDetection: boolean; + 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;