From c93aa60e9f073297d959fa1fff9323e48872d47e Mon Sep 17 00:00:00 2001 From: Ben Lesh Date: Sun, 25 Sep 2022 13:21:22 -0500 Subject: [PATCH] fix(schedulers): improve performance of animationFrameScheduler and asapScheduler (#7059) * refactor(schedulers): Appropriately type action ids * fix(animationFrameScheduler): improve performance of animationFrameScheduler + Changes the check for existing action ids to simply check the last action in the queue to see if its id matches. Previously we were doing an O(n) loop on each execution of an action to check to see if the scheduling id needed to be recycled. This is problematic in AsapScheduler and AnimationFrameScheduler, where we're not reusing an interval. Since AsapScheduler and AnimationFrameScheduler reuse the most recent action id until their scheduled microtask or animation frame fires, the last action in the actions queue array is all we really need to check (rather than checking them all with `some`). O(1) vs O(n). + Refactors a weird conditional gaff from `if ((X && A) || (!X && B))` to just be `if (X ? A : B)` resolves #7017 related #7018 related #6674 * chore: update api_guardian * refactor(QueueAction): Have requestActionId return 0 Changes this to return `0` as a compromise given it was returning `void` in the past. --- api_guard/dist/types/index.d.ts | 4 ++-- .../scheduler/AnimationFrameAction.ts | 13 +++++++----- src/internal/scheduler/AsapAction.ts | 11 ++++++---- src/internal/scheduler/AsyncAction.ts | 14 ++++++++----- src/internal/scheduler/AsyncScheduler.ts | 3 ++- src/internal/scheduler/QueueAction.ts | 20 +++++++++++-------- .../scheduler/VirtualTimeScheduler.ts | 7 ++++--- 7 files changed, 44 insertions(+), 28 deletions(-) diff --git a/api_guard/dist/types/index.d.ts b/api_guard/dist/types/index.d.ts index 1a14efdf38..387e385a6b 100644 --- a/api_guard/dist/types/index.d.ts +++ b/api_guard/dist/types/index.d.ts @@ -858,8 +858,8 @@ export declare class VirtualAction extends AsyncAction { protected work: (this: SchedulerAction, state?: T) => void; constructor(scheduler: VirtualTimeScheduler, work: (this: SchedulerAction, state?: T) => void, index?: number); protected _execute(state: T, delay: number): any; - protected recycleAsyncId(scheduler: VirtualTimeScheduler, id?: any, delay?: number): any; - protected requestAsyncId(scheduler: VirtualTimeScheduler, id?: any, delay?: number): any; + protected recycleAsyncId(scheduler: VirtualTimeScheduler, id?: any, delay?: number): TimerHandle | undefined; + protected requestAsyncId(scheduler: VirtualTimeScheduler, id?: any, delay?: number): TimerHandle; schedule(state?: T, delay?: number): Subscription; } diff --git a/src/internal/scheduler/AnimationFrameAction.ts b/src/internal/scheduler/AnimationFrameAction.ts index 771212f73d..f9c8f8e39d 100644 --- a/src/internal/scheduler/AnimationFrameAction.ts +++ b/src/internal/scheduler/AnimationFrameAction.ts @@ -2,13 +2,14 @@ import { AsyncAction } from './AsyncAction'; import { AnimationFrameScheduler } from './AnimationFrameScheduler'; import { SchedulerAction } from '../types'; import { animationFrameProvider } from './animationFrameProvider'; +import { TimerHandle } from './timerHandle'; export class AnimationFrameAction extends AsyncAction { constructor(protected scheduler: AnimationFrameScheduler, protected work: (this: SchedulerAction, state?: T) => void) { super(scheduler, work); } - protected requestAsyncId(scheduler: AnimationFrameScheduler, id?: any, delay: number = 0): any { + protected requestAsyncId(scheduler: AnimationFrameScheduler, id?: TimerHandle, delay: number = 0): TimerHandle { // If delay is greater than 0, request as an async action. if (delay !== null && delay > 0) { return super.requestAsyncId(scheduler, id, delay); @@ -20,18 +21,20 @@ export class AnimationFrameAction extends AsyncAction { // the current animation frame request id. return scheduler._scheduled || (scheduler._scheduled = animationFrameProvider.requestAnimationFrame(() => scheduler.flush(undefined))); } - protected recycleAsyncId(scheduler: AnimationFrameScheduler, id?: any, delay: number = 0): any { + + protected recycleAsyncId(scheduler: AnimationFrameScheduler, id?: TimerHandle, delay: number = 0): TimerHandle | undefined { // If delay exists and is greater than 0, or if the delay is null (the // action wasn't rescheduled) but was originally scheduled as an async // action, then recycle as an async action. - if ((delay != null && delay > 0) || (delay == null && this.delay > 0)) { + if (delay != null ? delay > 0 : this.delay > 0) { return super.recycleAsyncId(scheduler, id, delay); } // If the scheduler queue has no remaining actions with the same async id, // cancel the requested animation frame and set the scheduled flag to // undefined so the next AnimationFrameAction will request its own. - if (!scheduler.actions.some((action) => action.id === id)) { - animationFrameProvider.cancelAnimationFrame(id); + const { actions } = scheduler; + if (id != null && actions[actions.length - 1]?.id !== id) { + animationFrameProvider.cancelAnimationFrame(id as number); scheduler._scheduled = undefined; } // Return undefined so the action knows to request a new async id if it's rescheduled. diff --git a/src/internal/scheduler/AsapAction.ts b/src/internal/scheduler/AsapAction.ts index f8f5116e50..bd4b8697c3 100644 --- a/src/internal/scheduler/AsapAction.ts +++ b/src/internal/scheduler/AsapAction.ts @@ -2,13 +2,14 @@ import { AsyncAction } from './AsyncAction'; import { AsapScheduler } from './AsapScheduler'; import { SchedulerAction } from '../types'; import { immediateProvider } from './immediateProvider'; +import { TimerHandle } from './timerHandle'; export class AsapAction extends AsyncAction { constructor(protected scheduler: AsapScheduler, protected work: (this: SchedulerAction, state?: T) => void) { super(scheduler, work); } - protected requestAsyncId(scheduler: AsapScheduler, id?: any, delay: number = 0): any { + protected requestAsyncId(scheduler: AsapScheduler, id?: TimerHandle, delay: number = 0): TimerHandle { // If delay is greater than 0, request as an async action. if (delay !== null && delay > 0) { return super.requestAsyncId(scheduler, id, delay); @@ -20,17 +21,19 @@ export class AsapAction extends AsyncAction { // the current scheduled microtask id. return scheduler._scheduled || (scheduler._scheduled = immediateProvider.setImmediate(scheduler.flush.bind(scheduler, undefined))); } - protected recycleAsyncId(scheduler: AsapScheduler, id?: any, delay: number = 0): any { + + protected recycleAsyncId(scheduler: AsapScheduler, id?: TimerHandle, delay: number = 0): TimerHandle | undefined { // If delay exists and is greater than 0, or if the delay is null (the // action wasn't rescheduled) but was originally scheduled as an async // action, then recycle as an async action. - if ((delay != null && delay > 0) || (delay == null && this.delay > 0)) { + if (delay != null ? delay > 0 : this.delay > 0) { return super.recycleAsyncId(scheduler, id, delay); } // If the scheduler queue has no remaining actions with the same async id, // cancel the requested microtask and set the scheduled flag to undefined // so the next AsapAction will request its own. - if (!scheduler.actions.some((action) => action.id === id)) { + const { actions } = scheduler; + if (id != null && actions[actions.length - 1]?.id !== id) { immediateProvider.clearImmediate(id); scheduler._scheduled = undefined; } diff --git a/src/internal/scheduler/AsyncAction.ts b/src/internal/scheduler/AsyncAction.ts index f7d600472e..d7ffe51185 100644 --- a/src/internal/scheduler/AsyncAction.ts +++ b/src/internal/scheduler/AsyncAction.ts @@ -4,9 +4,10 @@ import { Subscription } from '../Subscription'; import { AsyncScheduler } from './AsyncScheduler'; import { intervalProvider } from './intervalProvider'; import { arrRemove } from '../util/arrRemove'; +import { TimerHandle } from './timerHandle'; export class AsyncAction extends Action { - public id: any; + public id: TimerHandle | undefined; public state?: T; // @ts-ignore: Property has no initializer and is not definitely assigned public delay: number; @@ -58,23 +59,26 @@ export class AsyncAction extends Action { this.delay = delay; // If this action has already an async Id, don't request a new one. - this.id = this.id || this.requestAsyncId(scheduler, this.id, delay); + this.id = this.id ?? this.requestAsyncId(scheduler, this.id, delay); return this; } - protected requestAsyncId(scheduler: AsyncScheduler, _id?: any, delay: number = 0): any { + protected requestAsyncId(scheduler: AsyncScheduler, _id?: TimerHandle, delay: number = 0): TimerHandle { return intervalProvider.setInterval(scheduler.flush.bind(scheduler, this), delay); } - protected recycleAsyncId(_scheduler: AsyncScheduler, id: any, delay: number | null = 0): any { + protected recycleAsyncId(_scheduler: AsyncScheduler, id?: TimerHandle, delay: number | null = 0): TimerHandle | undefined { // If this action is rescheduled with the same delay time, don't clear the interval id. if (delay != null && this.delay === delay && this.pending === false) { return id; } // Otherwise, if the action's delay time is different from the current delay, // or the action has been rescheduled before it's executed, clear the interval id - intervalProvider.clearInterval(id); + if (id != null) { + intervalProvider.clearInterval(id); + } + return undefined; } diff --git a/src/internal/scheduler/AsyncScheduler.ts b/src/internal/scheduler/AsyncScheduler.ts index 518ab24b21..fc04d66236 100644 --- a/src/internal/scheduler/AsyncScheduler.ts +++ b/src/internal/scheduler/AsyncScheduler.ts @@ -1,6 +1,7 @@ import { Scheduler } from '../Scheduler'; import { Action } from './Action'; import { AsyncAction } from './AsyncAction'; +import { TimerHandle } from './timerHandle'; export class AsyncScheduler extends Scheduler { public actions: Array> = []; @@ -18,7 +19,7 @@ export class AsyncScheduler extends Scheduler { * @type {any} * @internal */ - public _scheduled: any = undefined; + public _scheduled: TimerHandle | undefined; constructor(SchedulerAction: typeof Action, now: () => number = Scheduler.now) { super(SchedulerAction, now); diff --git a/src/internal/scheduler/QueueAction.ts b/src/internal/scheduler/QueueAction.ts index 46fba03ac7..9016edbc56 100644 --- a/src/internal/scheduler/QueueAction.ts +++ b/src/internal/scheduler/QueueAction.ts @@ -2,11 +2,10 @@ import { AsyncAction } from './AsyncAction'; import { Subscription } from '../Subscription'; import { QueueScheduler } from './QueueScheduler'; import { SchedulerAction } from '../types'; +import { TimerHandle } from './timerHandle'; export class QueueAction extends AsyncAction { - - constructor(protected scheduler: QueueScheduler, - protected work: (this: SchedulerAction, state?: T) => void) { + constructor(protected scheduler: QueueScheduler, protected work: (this: SchedulerAction, state?: T) => void) { super(scheduler, work); } @@ -21,12 +20,10 @@ export class QueueAction extends AsyncAction { } public execute(state: T, delay: number): any { - return (delay > 0 || this.closed) ? - super.execute(state, delay) : - this._execute(state, delay) ; + return delay > 0 || this.closed ? super.execute(state, delay) : this._execute(state, delay); } - protected requestAsyncId(scheduler: QueueScheduler, id?: any, delay: number = 0): any { + protected requestAsyncId(scheduler: QueueScheduler, id?: TimerHandle, delay: number = 0): TimerHandle { // If delay exists and is greater than 0, or if the delay is null (the // action wasn't rescheduled) but was originally scheduled as an async // action, then recycle as an async action. @@ -34,7 +31,14 @@ export class QueueAction extends AsyncAction { if ((delay != null && delay > 0) || (delay == null && this.delay > 0)) { return super.requestAsyncId(scheduler, id, delay); } + // Otherwise flush the scheduler starting with this action. - return scheduler.flush(this); + scheduler.flush(this); + + // HACK: In the past, this was returning `void`. However, `void` isn't a valid + // `TimerHandle`, and generally the return value here isn't really used. So the + // compromise is to return `0` which is both "falsy" and a valid `TimerHandle`, + // as opposed to refactoring every other instanceo of `requestAsyncId`. + return 0; } } diff --git a/src/internal/scheduler/VirtualTimeScheduler.ts b/src/internal/scheduler/VirtualTimeScheduler.ts index 34ce4552c3..870a144a76 100644 --- a/src/internal/scheduler/VirtualTimeScheduler.ts +++ b/src/internal/scheduler/VirtualTimeScheduler.ts @@ -2,6 +2,7 @@ import { AsyncAction } from './AsyncAction'; import { Subscription } from '../Subscription'; import { AsyncScheduler } from './AsyncScheduler'; import { SchedulerAction } from '../types'; +import { TimerHandle } from './timerHandle'; export class VirtualTimeScheduler extends AsyncScheduler { /** @deprecated Not used in VirtualTimeScheduler directly. Will be removed in v8. */ @@ -92,15 +93,15 @@ export class VirtualAction extends AsyncAction { } } - protected requestAsyncId(scheduler: VirtualTimeScheduler, id?: any, delay: number = 0): any { + protected requestAsyncId(scheduler: VirtualTimeScheduler, id?: any, delay: number = 0): TimerHandle { this.delay = scheduler.frame + delay; const { actions } = scheduler; actions.push(this); (actions as Array>).sort(VirtualAction.sortActions); - return true; + return 1; } - protected recycleAsyncId(scheduler: VirtualTimeScheduler, id?: any, delay: number = 0): any { + protected recycleAsyncId(scheduler: VirtualTimeScheduler, id?: any, delay: number = 0): TimerHandle | undefined { return undefined; }