-
Notifications
You must be signed in to change notification settings - Fork 1.3k
swb: batch UI updates from running #1676
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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
BatchProcessclass to accumulate and process entries in configurable batch sizes - Updated
interpretOutputto 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() { |
Copilot
AI
Oct 28, 2025
There was a problem hiding this comment.
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; }
| public drain() { | |
| public drain() { | |
| if (this.batch.length === 0) { | |
| return; | |
| } |
| this.processBatch(this.batch); | ||
| this.batch.length = 0; |
Copilot
AI
Oct 28, 2025
There was a problem hiding this comment.
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);
| this.processBatch(this.batch); | |
| this.batch.length = 0; | |
| const toProcess = this.batch.splice(0); | |
| this.processBatch(toProcess); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double check this
| this.processBatch(this.batch); | ||
| this.batch.length = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double check this
No description provided.