Skip to content

Reduce terminal checkpoint stalls with bounded native JSON encoding - #26177

Open
nwparker wants to merge 1 commit into
mainfrom
nwparker/perf-checkpoint-string-encoding
Open

nwparker wants to merge 1 commit into
mainfrom
nwparker/perf-checkpoint-string-encoding

Conversation

@nwparker

@nwparker nwparker commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​67 $\color{#cf222e}{\Huge{\mathbf{−}}}$​11 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​56
Prod 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​15 $\color{#cf222e}{\Huge{\mathbf{−}}}$​58 $\color{#cf222e}{\Huge{\mathbf{−}}}$​43

ELI5

Saving a large terminal history can briefly delay input in other terminals. Checkpoints now encode text in small native JSON conversions, reducing that pause while saving the same bytes.

What Changed

The existing bounded writer replaces its character-by-character string escaping with native JSON encoding of at most 16,384 UTF-16 units per conversion. It reduces that bound near the byte limit and reuses the existing surrogate-safe splitter. Object traversal, byte limits, history trimming, saved fields and disk writes keep their current behavior.

Why

Color-dense histories made the old writer assemble many short escape strings in JavaScript. Encoding the entire checkpoint at once would allocate an oversized candidate before rejecting it. Bounded string conversions avoid both costs.

Four warmed trials through the real history manager, checkpoint writer and disk commit on macOS/Node produced identical normalized checkpoint hashes:

Generated terminal history Saved size Median checkpoint time Native PTY input delay
160 columns, default 5,000-row durable window, dense colors 8.82 MB 169.2 → 20.2 ms 166.2 → 8.3 ms
160 columns, default 1,000-row live window 178 KB 1.36 → 1.03 ms 0.70 → 0.31 ms

The color-dense fixture demonstrates a supported workload; it is not a captured user session or a reproduction of the reported Codex typing pause. Full checkpoints run on lifecycle/recovery boundaries or incremental-log overflow; routine persistence remains incremental.

A separate 48.36 MB stress fixture sampled additional heap immediately after serialization at 502.4 MB before and 52.8 MB after. This sampled point is not a peak-memory measurement. Separate processes retained the same flattened output after forced GC: 47.985 versus 47.991 MB, with identical UTF-8 buffer sizes.

Linked Issue

N/A — maintainer performance audit.

Visual Proof

N/A — saved JSON and terminal rendering remain identical. Input measurements used a background native PTY without showing or focusing a window.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated

176 history/checkpoint tests across 12 files passed. Another 1,697 snapshot, ownership and cross-version terminal tests passed, with one skipped. Node typecheck, full ordinary oxlint, formatting and changed-code quality passed.

Tests cover hardcoded control and surrogate JSON at chunk boundaries, exact UTF-8 caps, one-byte-below trimming, bounded conversions and huge escaped metadata rejection. Independent comparisons against the frozen original covered every UTF-16 code unit and 81 generated inputs across six cap variants, including fractional and unbounded caps. Production disk hashes matched in every input-delay trial.

Review

The implementation uses existing Node primitives and preserves checkpoint content, including terminal ownership and modes. SSH and WSL use the same serializer; no wire fields, capabilities, process ownership or workspace paths change. Timing evidence is macOS/Node only; Linux, Windows and phone performance are not claimed.

Agent skill upstream boundary

  • Not applicable; no upstream skill source copied.

Notes

The 800-column stress fixture still delayed input by about 42 ms. This reduces encoding cost and allocation; checkpoint scheduling remains unchanged.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • N/A visual proof reason supplied
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and folder workspace impact considered
  • Typecheck, focused tests, full oxlint and changed quality pass; CI covers the full build

@coderabbitai

coderabbitai Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1fdf2fc2-41e0-49cb-a2e3-36dc759d6e3c
📥 Commits

Reviewing files that changed from the base of the PR and between 5b0d387 and 9fea597.

📒 Files selected for processing (2)
  • src/main/daemon/terminal-checkpoint-serializer.test.ts
  • src/main/daemon/terminal-checkpoint-serializer.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The serializer now escapes strings with bounded JSON.stringify chunks. It adjusts chunk boundaries to preserve surrogate pairs and checks each escaped chunk’s UTF-8 byte length against the writer limit. Tests cover conversion input bounds, snapshot reads, escaping boundaries, exact byte-limit acceptance, and rejection of oversized metadata.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 9fea5

The checkpoint writer now encodes strings with bounded native JSON conversion. Checkpoint content is described as unchanged, and no merge-blocking risk was found.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: bounded native JSON encoding to reduce terminal checkpoint stalls.
Description check ✅ Passed The description covers the required sections, explains the change and rationale, reports testing and performance results, and provides an applicable maintainer note for the linked issue section.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
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.

1 participant