Repository navigation
perf(shared): skip parsing cached terminal styles - #8
Closed
UtkarshUsername wants to merge 1 commit into
Closed
UtkarshUsername wants to merge 1 commit into
UtkarshUsername wants to merge 1 commit into
Conversation
Owner
Author
|
Opened upstream PR pingdotgg#27041 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The command-output terminal parser parses SGR numeric parameters before looking in its existing style-transition cache. Repeated colors and resets in build/test output therefore still allocate and parse arrays even when the resulting style is already cached. Both the desktop command-output client and its server use this parser.
Change
Move the existing SGR branch before numeric parsing, so only cache misses parse SGR parameters. Cursor/erase commands retain their existing parsing. Extend the existing chunk-boundary test with repeated bold/reset/RGB styles and both small and generous tail limits.
No new cache, API, output format, or terminal behavior. The existing 512-entry cache and eviction policy are unchanged.
Scope and approval
Owner-requested performance work on this fork. This is a focused removal of redundant work in an existing cache, not a new feature or workflow; no upstream maintainer approval is implied. No mobile-specific implementation or UI testing. The repository's affected-package typecheck includes shared-code dependents.
Verification
CommandOutputHub: 8 passed.vp run check:changed: formatting, lint, both knip passes, and its 17 tests (shared terminal + agent-context budget) passed. Its cached typecheck could not launch because this sandbox lacksseccomp_unotify. The same affected-package scripts were rerun throughvp run --no-cache; all 31 affected packages passed (18 passed in the first uncached run; the remaining 13 passed sequentially after four concurrent large checks exited 137 under memory pressure).git diff --checkand forwardgit apply --checkpassed. Independent static review found no correctness issues.Performance evidence
Baseline:
37fbefd147197fab9d88f9c120560c3f21b52cfc. Node 26.12.1, V8 14.6.202.34-node.40, Linux x64, Intel Xeon Platinum 8573C (9 visible CPUs). Project Vite+ 1.4.0 built the exact before/after module as isolated, minified ES2022 libraries. This is a pure-parser throughput benchmark, not a whole Electron/UI benchmark.Synthetic representative workload: 3,237 npm-style colored result lines, 262,197 UTF-16 code units / 268,671 UTF-8 bytes, delivered in 65 chunks of at most 4,096 code units through the real
appendTerminalOutput, including its default 65,536-code-unit tail handling.Timing used five fresh-process A/A pairs, then five alternating A/B pairs, pinned to CPU 0. Each process ran 100 warm-up replays and 20 batches of 10 measured replays. Figures use conventional per-process medians of batch means, then the median of five processes. No outlier was removed. All five A/B pairs improved; the maximum paired A/A difference divided by the smaller observation was 12.3%. A/A lane medians differed 4.34% relative to the first lane. CPU governor and hardware instruction counters were unavailable; CPU time and deterministic work counts are reported alongside wall time.
All primary timing trials (ms per replay)
An earlier unpinned artifact campaign had a 31.5% A/A outlier and was inconclusive in isolation. It was retained, not silently discarded; the longer affinity-pinned campaign above is a separately recorded confirmation. One campaign known to overlap aggregate checks was separately labeled contaminated and excluded on that interference condition. Raw scripts originally recorded an upper-middle
medianfield; final results were recomputed from every raw batch array using conventional medians.Allocation used three separate inspector profiles per arm, a 16 KiB sampling interval, both minor- and major-collected objects included, and 50 measured replays after 30 warm-ups. These are sampled allocation estimates, not exact byte counters. An independent 81-replay GC trace showed total pause 124.308 → 100.308 ms, maximum pause 5.510 → 4.823 ms, minor collections 71 → 68, and major collections 2 → 2. Promoted bytes increased from 33.66 to 35.17 MB; the trace is descriptive, not a statistical pause guarantee or retained-memory claim. Emitted
csiandfeednaturally reached TurboFan in both arms; optimization was not forced.Uncolored controls showed no causal performance benefit. The isolated module grows 14 bytes (8 gzipped); no new cache or retained state is added. Full Electron/browser integration was unavailable because the checkout requires T3's Browser panel, whose tools were absent; no alternative UI automation or mobile UI testing was used.
Reproduce the primary isolated-module benchmark
Run from the checkout after
vp i, with Node 26.12.1. This writes only scratch files outside the repository. CPU 0 must be available on Linux; choose an allowed core if needed and report the change. Keep other builds/tests stopped while measuring.Measured emitted SHA-256: before
6007e13ab47033739362e69fb5d68fa88d325d12132f8c04ff16f9806ba3a391; after7d92eb2c04194732d311430775ff1aa7cf4524a25a6dc6ec9dd3748821576e8e.Model: OpenAI (exact model identifier not exposed by the execution environment). Harness: ChatGPT Work cloud agent.