Skip to content

Conversation

@ulugbekna
Copy link
Contributor

No description provided.

Copilot AI review requested due to automatic review settings October 28, 2025 16:14
@ulugbekna ulugbekna enabled auto-merge October 28, 2025 16:14
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements batching of UI updates during simulation execution to improve performance by reducing the frequency of MobX reactive updates. Instead of triggering a UI update for every output entry, entries are now accumulated and processed in batches of 20.

Key Changes

  • Introduced a BatchProcess class to accumulate and process entries in configurable batch sizes
  • Updated interpretOutput to use the batch processor instead of processing entries individually
  • Added a drain() call in the finally block to ensure all remaining entries are processed

}
}

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.
Comment on lines +504 to +505
this.processBatch(this.batch);
this.batch.length = 0;
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

Comment on lines +504 to +505
this.processBatch(this.batch);
this.batch.length = 0;
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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants