Skip to content
Draft
Changes from all commits
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
32 changes: 31 additions & 1 deletion test/simulation/workbench/stores/simulationRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -411,14 +411,21 @@ class SimulationExecutor {
private async interpretOutput(stream: AsyncIterableObject<RunOutput>, stdoutFile: string): Promise<void> {
const writtenFilesBaseDir = path.dirname(stdoutFile);
const entries: RunOutput[] = [];

const batchProcessor = new BatchProcess<RunOutput>(
(batch) => mobx.runInAction(() => batch.forEach((entry) => this.interpretOutputEntry(writtenFilesBaseDir, entry))),
20
);

try {
for await (const entry of stream) {
entries.push(entry);
mobx.runInAction(() => this.interpretOutputEntry(writtenFilesBaseDir, entry)); // TODO@ulugbekna: we should batch updates
batchProcessor.pushForProcessing(entry);
}
} catch (e) {
console.error('interpretOutput', JSON.stringify(e, null, '\t'));
} finally {
batchProcessor.drain();
await fs.promises.writeFile(stdoutFile, JSON.stringify(entries, null, '\t'));
this.currentCancellationTokenSource = undefined;
mobx.runInAction(() => {
Expand Down Expand Up @@ -475,3 +482,26 @@ function findInitialTestSummary(runOutput: RunOutput[]): IInitialTestSummaryOutp
}
return undefined;
}

class BatchProcess<T> {
private readonly batch: T[];

constructor(
private readonly processBatch: (batch: T[]) => void,
private readonly batchSize: number
) {
this.batch = [];
}

public pushForProcessing(entry: T) {
this.batch.push(entry);
if (this.batch.length >= this.batchSize) {
this.drain();
}
}

public drain() {
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

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

The drain() method processes the batch even when it's empty. This creates unnecessary MobX actions and function calls. Add a guard to return early if the batch is empty: if (this.batch.length === 0) { return; }

Suggested change
public drain() {
public drain() {
if (this.batch.length === 0) {
return;
}

Copilot uses AI. Check for mistakes.
this.processBatch(this.batch);
this.batch.length = 0;
Comment on lines +504 to +505
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

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

Race condition: if processBatch throws an error, the batch will not be cleared, causing entries to be reprocessed on the next drain() call. Move the clear operation into a finally block or use splice to clear before processing: const toProcess = this.batch.splice(0); then this.processBatch(toProcess);

Suggested change
this.processBatch(this.batch);
this.batch.length = 0;
const toProcess = this.batch.splice(0);
this.processBatch(toProcess);

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

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

Double check this

}
}