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

[v5] Remove batching #2869

Merged
merged 19 commits into from
Jan 10, 2022
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add Mailbox
davidkpiano committed Dec 12, 2021
commit b546eb6cc27d00f89ffa45b65c511755402f8785
36 changes: 36 additions & 0 deletions packages/core/src/Mailbox.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
export class Mailbox<T> {
private events: T[] = [];
private index: number = 0;

public status: 'deferred' | 'idle' | 'processing' = 'deferred';

public get size(): number {
return this.events.length - this.index;
}

public clear(): void {
this.events.length = 0;
this.index = 0;
}

public enqueue(event: T): void {
this.events.push(event);
}

public dequeue(): T | undefined {
const event = this.events[this.index];

if (!event) {
return undefined;
}

this.index++;

if (this.index > this.events.length - 1) {
this.events.length = 0;
this.index = 0;
}
Copy link
Member

Choose a reason for hiding this comment

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

This strategy doesn't scale because we retain all the queued items up until we drain the queue. The items that were already processed should be immediately disposed because we don't have any use for them from that point in time.

I've reimplemented this mailbox using a simple linked list to avoid this problem altogether:
37547c3


return event;
}
}
22 changes: 16 additions & 6 deletions packages/core/src/StateMachine.ts
Original file line number Diff line number Diff line change
@@ -240,32 +240,42 @@ export class StateMachine<
return transitionNode(this.root, state.value, state, _event) || [];
}

public get first(): State<TContext, TEvent, TTypestate> {
const pseudoinitial = this.resolveState(
private get preInitialState(): State<TContext, TEvent, TTypestate> {
const preInitial = this.resolveState(
State.from(
getStateValue(this.root, getConfiguration([this.root])),
this.context
)
);
pseudoinitial._initial = true;
preInitial._initial = true;

return pseudoinitial;
return preInitial;
}

/**
* The initial State instance, which includes all actions to be executed from
* entering the initial state.
*/
public get initialState(): State<TContext, TEvent, TTypestate> {
const nextState = resolveMicroTransition(this, [], this.first, undefined);
const nextState = resolveMicroTransition(
this,
[],
this.preInitialState,
undefined
);
return macrostep(nextState, null as any, this);
}

/**
* Returns the initial `State` instance, with reference to `self` as an `ActorRef`.
*/
public getInitialState(): State<TContext, TEvent, TTypestate> {
const nextState = resolveMicroTransition(this, [], this.first, undefined);
const nextState = resolveMicroTransition(
this,
[],
this.preInitialState,
undefined
);
return macrostep(nextState, null as any, this) as State<
TContext,
TEvent,
21 changes: 11 additions & 10 deletions packages/core/src/interpreter.ts
Original file line number Diff line number Diff line change
@@ -48,6 +48,7 @@ import type {
Subscribable
} from './types';
import { isExecutableAction } from '../actions/ExecutableAction';
import { Mailbox } from './Mailbox';

export type StateListener<
TContext extends MachineContext,
@@ -110,8 +111,7 @@ export class Interpreter<
public options: Readonly<InterpreterOptions>;

public id: string;
private mailbox: Array<SCXML.Event<TEvent>> = [];
private mailboxStatus: 'deferred' | 'idle' | 'processing' = 'deferred';
private mailbox: Mailbox<SCXML.Event<TEvent>> = new Mailbox();
private delayedEventsMap: Record<string, number> = {};
private listeners: Set<
StateListener<TContext, TEvent, TTypestate>
@@ -418,18 +418,19 @@ export class Interpreter<
}

private flush() {
this.mailboxStatus = 'processing';
while (this.mailbox.length) {
const event = this.mailbox.shift()!;

this.mailbox.status = 'processing';
let event = this.mailbox.dequeue();
Copy link
Member

Choose a reason for hiding this comment

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

This implementation doesn't scale too well with multiple consumers of the mailbox because a lot of the delicate queueing/dequeuing management becomes the responsibility of each consumer. I think I've managed to avoid this in this PR:
37547c3

With these changes, the consumer only needs to provide a "process" callback to the created mailbox and call simple start, clear and enqueue methods. Thanks to that I've been able to reuse easily this Mailbox implementation in other 2 places that were managing this:
00f1d74
e530b5b

while (event) {
// TODO: handle errors
this.forward(event);

const nextState = this.nextState(event);

this.update(nextState);

event = this.mailbox.dequeue();
}
this.mailboxStatus = 'idle';
this.mailbox.status = 'idle';
}

/**
@@ -469,7 +470,7 @@ export class Interpreter<
this.clock.clearTimeout(this.delayedEventsMap[key]);
}

this.mailbox.length = 0;
this.mailbox.clear();
this.status = InterpreterStatus.Stopped;
registry.free(this.sessionId);

@@ -518,9 +519,9 @@ export class Interpreter<
);
}

this.mailbox.push(_event);
this.mailbox.enqueue(_event);

if (this.mailboxStatus === 'idle') {
if (this.mailbox.status === 'idle') {
this.flush();
}
};