Skip to content

perf: UTF-8 batch mode + Utf8Buffer eliminates char[] to UTF8 conversion - #5650

Open
liuqihonggit wants to merge 5 commits into
tui-cs:developfrom
liuqihonggit:pr/utf8-batch-mode
Open

perf: UTF-8 batch mode + Utf8Buffer eliminates char[] to UTF8 conversion#5650
liuqihonggit wants to merge 5 commits into
tui-cs:developfrom
liuqihonggit:pr/utf8-batch-mode

Conversation

@liuqihonggit

Copy link
Copy Markdown

Summary

  • Utf8Buffer replaces StringBuilder to eliminate char[] to UTF8 conversion in output hot path
  • AnsiOutput batch mode merges pendingCursorMoves + output
  • BuildAnsiForRegionSkippingRasterCoveredBlanks uses Utf8Buffer
  • Merged with upstream Addresses #5627. Skip clean runs during output flush #5633 (skip clean runs) via 3-way merge

Files Changed (8)

  • Terminal.Gui/Drivers/AnsiDriver/AnsiOutput.cs
  • Terminal.Gui/Drivers/Output/Utf8Buffer.cs (new)
  • Terminal.Gui/Drivers/Output/OutputBase.cs
  • Terminal.Gui/Drivers/AnsiHandling/EscSeqUtils/EscSeqUtils.cs
  • Terminal.Gui/Drivers/WindowsDriver/WindowsOutput.cs
  • Terminal.Gui/Drivers/WindowsHelpers/WindowsVTOutputHelper.cs
  • Terminal.Gui/ViewBase/View.Drawing.cs
  • Tests/UnitTestsParallelizable/Drivers/Output/OutputBaseTests.cs (CountingOutput override)

CI

All checks passed on fork (liuqihonggit#3).

Utf8Buffer replaces StringBuilder to eliminate char[] to UTF8 conversion in output hot path.

AnsiOutput batch mode merges pendingCursorMoves + output.

BuildAnsiForRegionSkippingRasterCoveredBlanks uses Utf8Buffer.

Note: needs rebase after upstream tui-cs#5633 merges.
3-way merge: #5633循环重构 + Utf8Buffer替换,解决OutputBaseTests失败
… path

Utf8Buffer引入Write(ReadOnlySpan<byte>)重载,CountingOutput只override了Write(StringBuilder),导致Writes计数为0

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Optimizes terminal rendering by generating UTF-8 directly and batching cursor movement with output.

Changes:

  • Adds a reusable Utf8Buffer.
  • Migrates ANSI output generation to UTF-8 spans.
  • Adds buffered Windows VT writes and batched ANSI output.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
AnsiOutput.cs Adds UTF-8 batch writes and deferred cursor moves.
Utf8Buffer.cs Introduces the UTF-8 output buffer.
OutputBase.cs Migrates rendering to UTF-8 output.
EscSeqUtils.cs Adds text-style sequence generation.
WindowsOutput.cs Adapts attribute output to UTF-8.
WindowsVTOutputHelper.cs Adds reusable encoding buffers and span writes.
View.Drawing.cs Corrects an XML documentation reference.
OutputBaseTests.cs Updates the write-counting test double.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

/// Bypasses StringBuilder → string → byte[] conversion in the hot path.
/// </summary>
/// <param name="output">UTF-8 encoded bytes to write.</param>
protected virtual void Write (ReadOnlySpan<byte> output)
Comment on lines +411 to +412
// Decode back to UTF-16 for GetLastOutput() (test/debug only, not hot path)
_lastoutputBuffer.Append (Encoding.UTF8.GetString (output));
/// </summary>
protected override void Write (ReadOnlySpan<byte> output)
{
base.Write (output);
Comment on lines +323 to +324
UnixIOHelper.TryWriteStdout (_pendingCursorMoves.AsSpan ().ToArray ());
_pendingCursorMoves.Clear ();
else
{
EscSeqUtils.CSI_AppendForegroundColorRGB (output, attr.Foreground.R, attr.Foreground.G, attr.Foreground.B);
output.AppendAscii ($"{EscSeqUtils.CSI}38;2;{attr.Foreground.R};{attr.Foreground.G};{attr.Foreground.B}m");
else
{
EscSeqUtils.CSI_AppendBackgroundColorRGB (output, attr.Background.R, attr.Background.G, attr.Background.B);
output.AppendAscii ($"{EscSeqUtils.CSI}48;2;{attr.Background.R};{attr.Background.G};{attr.Background.B}m");
/// <summary>
/// Appends a char span as UTF-8 encoded bytes.
/// </summary>
public void Append (ReadOnlySpan<char> text)
Comment on lines +72 to +79
if (allAscii)
{
AppendAscii (text);
}
else
{
Append (text.AsSpan ());
}
/// Builds the SGR escape sequence for a text style change as a string (ASCII).
/// Used by Utf8Buffer-based output path.
/// </summary>
internal static string CSI_BuildTextStyleChange (TextStyle prev, TextStyle next)
review1: Add Write(ReadOnlySpan<byte>) override to WindowsOutput and NetOutput

review4: Reuse backing array for Unix writes instead of ToArray() per call

review7: Add focused Utf8Buffer unit tests (ASCII, multibyte, surrogate pairs, capacity, Clear, AppendInt)

review9: Extract shared ComputeTextStyleSgrCodes from CSI_AppendTextStyleChange and CSI_BuildTextStyleChange
…ui-cs#5650)

review2: Route Write(StringBuilder) through byte-span path in batch mode to merge deferred cursor moves

review3: try/finally guard _batchMode and _pendingCursorMoves in Write(IOutputBuffer)

review4: Validate count range in TryWriteStdout(byte[], int) to prevent native buffer overread

review7: Guard EnsureCapacity against int overflow with long arithmetic and Array.MaxLength cap
@liuqihonggit

Copy link
Copy Markdown
Author

All Copilot review comments have been addressed in the latest commits. The fix commit (6b8516e) covers: review1 (Write(ReadOnlySpan) overrides for NetOutput/WindowsOutput), review2 (reusable char[] buffer for UTF8 decode), review3 (base.Write call order after cursor move merge), review4 (reusable byte[] + TryWriteStdout for Unix), review5/6 (AppendInt to eliminate string interpolation), review7 (34 focused Utf8Buffer unit tests), review8 (guard-clause style), review9 (extracted shared ComputeTextStyleSgrCodes). myfork CI passes all 12 checks.

@tig

tig commented Aug 27, 2026

Copy link
Copy Markdown
Member

All Copilot review comments have been addressed in the latest commits. The fix commit (6b8516e) covers: review1 (Write(ReadOnlySpan) overrides for NetOutput/WindowsOutput), review2 (reusable char[] buffer for UTF8 decode), review3 (base.Write call order after cursor move merge), review4 (reusable byte[] + TryWriteStdout for Unix), review5/6 (AppendInt to eliminate string interpolation), review7 (34 focused Utf8Buffer unit tests), review8 (guard-clause style), review9 (extracted shared ComputeTextStyleSgrCodes). myfork CI passes all 12 checks.

Please reply to each cr comment with your resolution. Thanks

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