Skip to content

Commit

Permalink
fix(schedulers): improve performance of animationFrameScheduler and a…
Browse files Browse the repository at this point in the history
…sapScheduler (#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.
  • Loading branch information
benlesh authored Sep 25, 2022
1 parent 2d57b38 commit c93aa60
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 28 deletions.
4 changes: 2 additions & 2 deletions api_guard/dist/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -858,8 +858,8 @@ export declare class VirtualAction<T> extends AsyncAction<T> {
protected work: (this: SchedulerAction<T>, state?: T) => void;
constructor(scheduler: VirtualTimeScheduler, work: (this: SchedulerAction<T>, 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;
}

Expand Down
13 changes: 8 additions & 5 deletions src/internal/scheduler/AnimationFrameAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> extends AsyncAction<T> {
constructor(protected scheduler: AnimationFrameScheduler, protected work: (this: SchedulerAction<T>, 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);
Expand All @@ -20,18 +21,20 @@ export class AnimationFrameAction<T> extends AsyncAction<T> {
// 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.
Expand Down
11 changes: 7 additions & 4 deletions src/internal/scheduler/AsapAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> extends AsyncAction<T> {
constructor(protected scheduler: AsapScheduler, protected work: (this: SchedulerAction<T>, 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);
Expand All @@ -20,17 +21,19 @@ export class AsapAction<T> extends AsyncAction<T> {
// 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;
}
Expand Down
14 changes: 9 additions & 5 deletions src/internal/scheduler/AsyncAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> extends Action<T> {
public id: any;
public id: TimerHandle | undefined;
public state?: T;
// @ts-ignore: Property has no initializer and is not definitely assigned
public delay: number;
Expand Down Expand Up @@ -58,23 +59,26 @@ export class AsyncAction<T> extends Action<T> {

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;
}

Expand Down
3 changes: 2 additions & 1 deletion src/internal/scheduler/AsyncScheduler.ts
Original file line number Diff line number Diff line change
@@ -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<AsyncAction<any>> = [];
Expand All @@ -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);
Expand Down
20 changes: 12 additions & 8 deletions src/internal/scheduler/QueueAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> extends AsyncAction<T> {

constructor(protected scheduler: QueueScheduler,
protected work: (this: SchedulerAction<T>, state?: T) => void) {
constructor(protected scheduler: QueueScheduler, protected work: (this: SchedulerAction<T>, state?: T) => void) {
super(scheduler, work);
}

Expand All @@ -21,20 +20,25 @@ export class QueueAction<T> extends AsyncAction<T> {
}

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.

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;
}
}
7 changes: 4 additions & 3 deletions src/internal/scheduler/VirtualTimeScheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -92,15 +93,15 @@ export class VirtualAction<T> extends AsyncAction<T> {
}
}

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<VirtualAction<T>>).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;
}

Expand Down

0 comments on commit c93aa60

Please sign in to comment.