Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add a flag in bootstrap to enable coalesce event to improve performance #30533

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 29 additions & 4 deletions packages/core/src/application_ref.ts
Expand Up @@ -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
* 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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs public API docs here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add docs explaining what this does?

}

/**
Expand Down Expand Up @@ -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`!
Expand Down Expand Up @@ -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;
}
Expand Down
29 changes: 29 additions & 0 deletions 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};
}
51 changes: 45 additions & 6 deletions packages/core/src/zone/ng_zone.ts
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -115,7 +118,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`);
}
Expand All @@ -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);
}

Expand Down Expand Up @@ -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) {
Expand All @@ -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: <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);
}
},
Expand All @@ -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;
Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/fake_async_spec.ts
Expand Up @@ -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', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/testing/src/ng_zone_mock.ts
Expand Up @@ -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(); }

Expand Down
3 changes: 2 additions & 1 deletion packages/core/testing/src/test_bed.ts
Expand Up @@ -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,
Expand Down
41 changes: 36 additions & 5 deletions packages/platform-browser/test/dom/events/event_manager_spec.ts
Expand Up @@ -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({});
Expand Down Expand Up @@ -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);
Expand All @@ -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();
});
});
});
})();

Expand All @@ -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(); }
}
2 changes: 1 addition & 1 deletion packages/platform-browser/testing/src/browser_util.ts
Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion tools/public_api_guard/core/core.d.ts
Expand Up @@ -630,8 +630,9 @@ export declare class NgZone {
readonly onMicrotaskEmpty: EventEmitter<any>;
readonly onStable: EventEmitter<any>;
readonly onUnstable: EventEmitter<any>;
constructor({ enableLongStackTrace }: {
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;
Expand Down