feat: add a flag in bootstrap to enable coalesce event change detecti…

…on to improve performance (#30533) PR Close #30533
angular · Oct 16, 2019 · 21c1e14 · 21c1e14
1 parent 4c63e6b
commit 21c1e14
Show file tree

Hide file tree

Showing 11 changed files with 171 additions and 21 deletions.
diff --git 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
@@ -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
@@ -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.
+   *
+   * <div (click)="doSomething()">
+   *   <button (click)="doSomethingElse()"></button>
+   * </div>
+   *
+   * 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
@@ -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
@@ -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<any> = 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: <any>{'isAngularZone': true},
+    properties:
+        <any>{'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,13 +357,16 @@ 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;
   readonly onUnstable: EventEmitter<any> = new EventEmitter();
   readonly onMicrotaskEmpty: EventEmitter<any> = new EventEmitter();
   readonly onStable: EventEmitter<any> = new EventEmitter();
   readonly onError: EventEmitter<any> = 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
@@ -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
@@ -16,7 +16,7 @@ import {EventEmitter, Injectable, NgZone} from '@angular/core';
 export class MockNgZone extends NgZone {
   onStable: EventEmitter<any> = 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
@@ -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
@@ -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('<div><div></div></div>');
       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('<div></div>');
+      const child = el('<div></div>');
+      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<T>(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
@@ -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
@@ -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<any>;
     readonly onMicrotaskEmpty: EventEmitter<any>;
     readonly onStable: EventEmitter<any>;
     readonly onUnstable: EventEmitter<any>;
-    constructor({ enableLongStackTrace }: {
+    readonly shouldCoalesceEventChangeDetection: boolean;
+    constructor({ enableLongStackTrace, shouldCoalesceEventChangeDetection }: {
         enableLongStackTrace?: boolean | undefined;
+        shouldCoalesceEventChangeDetection?: boolean | undefined;
     });
     run<T>(fn: (...args: any[]) => T, applyThis?: any, applyArgs?: any[]): T;
     runGuarded<T>(fn: (...args: any[]) => T, applyThis?: any, applyArgs?: any[]): T;