cleanup(core): reduce terminal output duplication and allocations in task runner#34427
cleanup(core): reduce terminal output duplication and allocations in task runner#34427FrozenPandaz merged 1 commit intomasterfrom
Conversation
|
View your CI Pipeline Execution ↗ for commit f52a7d1
☁️ Nx Cloud last updated this comment at |
✅ Deploy Preview for nx-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for nx-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Failed to publish a PR release of this pull request, triggered by @leosvelperez. |
|
Failed to publish a PR release of this pull request, triggered by @leosvelperez. |
|
Failed to publish a PR release of this pull request, triggered by @leosvelperez. |
| const stoppedTasks = new Set<string>(); | ||
|
|
||
| // Chunks accumulated progressively during task execution | ||
| const taskOutputChunks: Record<string, string[]> = {}; |
There was a problem hiding this comment.
This is still a duplicate array of the one in the running task...
With the results now having the full terminal output.. can we not store this duplicate?
There was a problem hiding this comment.
Nevermind, I see that we need the chunks for continuous tasks.
… runner (#34427) ## Current Behavior Terminal output in the task runner is accumulated via repeated string concatenation (`terminalOutput += chunk`). Each `+=` on a growing string causes V8 to allocate a new, larger string and copy the old contents, resulting in O(n²) allocation behavior for tasks with large output. Additionally, `PseudoTtyProcess.onExit` didn't pass `terminalOutput` to its callbacks, forcing callers like `TaskOrchestrator` to duplicate output accumulation logic with a separate `onOutput` listener. ## Expected Behavior - Terminal output is collected in `string[]` arrays and joined once at the end, reducing intermediate allocations from O(n²) to O(n) - `PseudoTtyProcess.onExit` now passes `terminalOutput` as a second argument, matching the signature of other `RunningTask` implementations - `TaskOrchestrator` no longer needs a special code path for `PseudoTtyProcess` — unified `onExit` handling for all task types - `tui-summary-life-cycle` accumulates output in chunks during execution and stores the finalized string on task completion, allowing chunk arrays to be GC'd - `SeriallyRunningTasks` and `RunningNodeProcess` similarly switched to chunk-based accumulation - `BatchProcess` and `NodeChildProcessWithNonDirectOutput` lazily join and cache their terminal output (cherry picked from commit 4f9be49)
|
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
Current Behavior
Terminal output in the task runner is accumulated via repeated string concatenation (
terminalOutput += chunk). Each+=on a growing string causes V8 to allocate a new, larger string and copy the old contents, resulting in O(n²) allocation behavior for tasks with large output. Additionally,PseudoTtyProcess.onExitdidn't passterminalOutputto its callbacks, forcing callers likeTaskOrchestratorto duplicate output accumulation logic with a separateonOutputlistener.Expected Behavior
string[]arrays and joined once at the end, reducing intermediate allocations from O(n²) to O(n)PseudoTtyProcess.onExitnow passesterminalOutputas a second argument, matching the signature of otherRunningTaskimplementationsTaskOrchestratorno longer needs a special code path forPseudoTtyProcess— unifiedonExithandling for all task typestui-summary-life-cycleaccumulates output in chunks during execution and stores the finalized string on task completion, allowing chunk arrays to be GC'dSeriallyRunningTasksandRunningNodeProcesssimilarly switched to chunk-based accumulationBatchProcessandNodeChildProcessWithNonDirectOutputlazily join and cache their terminal output