Преглед на файлове

Reduce string allocations in IConsoleOutput implementations (#3978)

* Change IConsoleOutput.Write(string) overload parameter to ReadOnlySpan<char>

Allows the caller more flexibility about choosing a buffer per use case.

* NetOutput: Write StringBuilder directly to the std out text stream

* Add EscSeqUtils.CSI_WriteCursorPosition

Writes cursor position sequence to text writer without string allocation.

* NetOutput: Skip cursor position escape sequence string allocation

* Replace CSI_(Enable|Disable)MouseEvents static properties with readonly fields

Changed for the sake of consistency with rest of the EscSegutils fields rather than performance. Also prevents bugs from accidentally setting the properties.

* Use EscSeqUtils.CSI_Append(Foreground|Background)ColorRGB in v2 drivers

* WindowsOutput SetCursorVisibility: Remove intermediate string builder

* WindowsOutput.WriteToConsole: Use rented array as intermediate write buffer

The large intermediate string builder remains a challenge. :)

* NetOutput: Console.Out for the sake of consistency

Also might have missed one of the Console.Out.Write(StringBuilder) calls...

* Avoid Rune.ToString() in NetOutput.Write(IOutputBuffer)

---------

Co-authored-by: Tig <[email protected]>
Tonttu преди 4 месеца
родител
ревизия
ef20ff68a9

+ 31 - 0
Benchmarks/ConsoleDrivers/EscSeqUtils/CSI_SetVsWrite.cs

@@ -0,0 +1,31 @@
+using BenchmarkDotNet.Attributes;
+using Tui = Terminal.Gui;
+
+namespace Terminal.Gui.Benchmarks.ConsoleDrivers.EscSeqUtils;
+
+[MemoryDiagnoser]
+// Hide useless column from results.
+[HideColumns ("writer")]
+public class CSI_SetVsWrite
+{
+    [Benchmark (Baseline = true)]
+    [ArgumentsSource (nameof (TextWriterSource))]
+    public TextWriter Set (TextWriter writer)
+    {
+        writer.Write (Tui.EscSeqUtils.CSI_SetCursorPosition (1, 1));
+        return writer;
+    }
+
+    [Benchmark]
+    [ArgumentsSource (nameof (TextWriterSource))]
+    public TextWriter Write (TextWriter writer)
+    {
+        Tui.EscSeqUtils.CSI_WriteCursorPosition (writer, 1, 1);
+        return writer;
+    }
+
+    public static IEnumerable<object> TextWriterSource ()
+    {
+        return [StringWriter.Null];
+    }
+}

+ 29 - 3
Terminal.Gui/ConsoleDrivers/EscSeqUtils/EscSeqUtils.cs

@@ -1,5 +1,5 @@
 #nullable enable
-using Terminal.Gui.ConsoleDrivers;
+using System.Globalization;
 using static Terminal.Gui.ConsoleDrivers.ConsoleKeyMapping;
 
 namespace Terminal.Gui;
@@ -154,13 +154,13 @@ public static class EscSeqUtils
     /// <summary>
     ///     Control sequence for disabling mouse events.
     /// </summary>
-    public static string CSI_DisableMouseEvents { get; set; } =
+    public static readonly string CSI_DisableMouseEvents =
         CSI_DisableAnyEventMouse + CSI_DisableUrxvtExtModeMouse + CSI_DisableSgrExtModeMouse;
 
     /// <summary>
     ///     Control sequence for enabling mouse events.
     /// </summary>
-    public static string CSI_EnableMouseEvents { get; set; } =
+    public static readonly string CSI_EnableMouseEvents =
         CSI_EnableAnyEventMouse + CSI_EnableUrxvtExtModeMouse + CSI_EnableSgrExtModeMouse;
 
     /// <summary>
@@ -1688,6 +1688,32 @@ public static class EscSeqUtils
         builder.Append ($"{CSI}{row};{col}H");
     }
 
+    /// <summary>
+    ///     ESC [ y ; x H - CUP Cursor Position - Cursor moves to x ; y coordinate within the viewport, where x is the column
+    ///     of the y line
+    /// </summary>
+    /// <param name="writer">TextWriter where to write the cursor position sequence.</param>
+    /// <param name="row">Origin is (1,1).</param>
+    /// <param name="col">Origin is (1,1).</param>
+    public static void CSI_WriteCursorPosition (TextWriter writer, int row, int col)
+    {
+        const int maxInputBufferSize =
+            // CSI (2) + ';' + 'H'
+            4 +
+            // row + col (2x int sign + int max value)
+            2 + 20;
+        Span<char> buffer = stackalloc char[maxInputBufferSize];
+        if (!buffer.TryWrite (CultureInfo.InvariantCulture, $"{CSI}{row};{col}H", out int charsWritten))
+        {
+            string tooLongCursorPositionSequence = $"{CSI}{row};{col}H";
+            throw new InvalidOperationException (
+                $"{nameof(CSI_WriteCursorPosition)} buffer (len: {buffer.Length}) is too short for cursor position sequence '{tooLongCursorPositionSequence}' (len: {tooLongCursorPositionSequence.Length}).");
+        }
+
+        ReadOnlySpan<char> cursorPositionSequence = buffer[..charsWritten];
+        writer.Write (cursorPositionSequence);
+    }
+
     //ESC [ <y> ; <x> f - HVP     Horizontal Vertical Position* Cursor moves to<x>; <y> coordinate within the viewport, where <x> is the column of the<y> line
     //ESC [ s - ANSISYSSC       Save Cursor – Ansi.sys emulation	**With no parameters, performs a save cursor operation like DECSC
     //ESC [ u - ANSISYSRC       Restore Cursor – Ansi.sys emulation	**With no parameters, performs a restore cursor operation like DECRC

+ 1 - 1
Terminal.Gui/ConsoleDrivers/V2/IConsoleOutput.cs

@@ -11,7 +11,7 @@ public interface IConsoleOutput : IDisposable
     ///     <see cref="IOutputBuffer"/> overload.
     /// </summary>
     /// <param name="text"></param>
-    void Write (string text);
+    void Write (ReadOnlySpan<char> text);
 
     /// <summary>
     ///     Write the contents of the <paramref name="buffer"/> to the console

+ 29 - 21
Terminal.Gui/ConsoleDrivers/V2/NetOutput.cs

@@ -34,7 +34,10 @@ public class NetOutput : IConsoleOutput
     }
 
     /// <inheritdoc/>
-    public void Write (string text) { Console.Write (text); }
+    public void Write (ReadOnlySpan<char> text)
+    {
+        Console.Out.Write (text);
+    }
 
     /// <inheritdoc/>
     public void Write (IOutputBuffer buffer)
@@ -57,6 +60,9 @@ public class NetOutput : IConsoleOutput
         CursorVisibility? savedVisibility = _cachedCursorVisibility;
         SetCursorVisibility (CursorVisibility.Invisible);
 
+        const int maxCharsPerRune = 2;
+        Span<char> runeBuffer = stackalloc char[maxCharsPerRune];
+
         for (int row = top; row < rows; row++)
         {
             if (Console.WindowHeight < 1)
@@ -115,26 +121,28 @@ public class NetOutput : IConsoleOutput
                     {
                         redrawAttr = attr;
 
-                        output.Append (
-                                       EscSeqUtils.CSI_SetForegroundColorRGB (
-                                                                              attr.Foreground.R,
-                                                                              attr.Foreground.G,
-                                                                              attr.Foreground.B
-                                                                             )
-                                      );
-
-                        output.Append (
-                                       EscSeqUtils.CSI_SetBackgroundColorRGB (
-                                                                              attr.Background.R,
-                                                                              attr.Background.G,
-                                                                              attr.Background.B
-                                                                             )
-                                      );
+                        EscSeqUtils.CSI_AppendForegroundColorRGB (
+                            output,
+                            attr.Foreground.R,
+                            attr.Foreground.G,
+                            attr.Foreground.B
+                        );
+
+                        EscSeqUtils.CSI_AppendBackgroundColorRGB (
+                            output,
+                            attr.Background.R,
+                            attr.Background.G,
+                            attr.Background.B
+                        );
                     }
 
                     outputWidth++;
+
+                    // Avoid Rune.ToString() by appending the rune chars.
                     Rune rune = buffer.Contents [row, col].Rune;
-                    output.Append (rune);
+                    int runeCharsWritten = rune.EncodeToUtf16 (runeBuffer);
+                    ReadOnlySpan<char> runeChars = runeBuffer[..runeCharsWritten];
+                    output.Append (runeChars);
 
                     if (buffer.Contents [row, col].CombiningMarks.Count > 0)
                     {
@@ -162,7 +170,7 @@ public class NetOutput : IConsoleOutput
             if (output.Length > 0)
             {
                 SetCursorPositionImpl (lastCol, row);
-                Console.Write (output);
+                Console.Out.Write (output);
             }
         }
 
@@ -171,7 +179,7 @@ public class NetOutput : IConsoleOutput
             if (!string.IsNullOrWhiteSpace (s.SixelData))
             {
                 SetCursorPositionImpl (s.ScreenPosition.X, s.ScreenPosition.Y);
-                Console.Write (s.SixelData);
+                Console.Out.Write (s.SixelData);
             }
         }
 
@@ -185,7 +193,7 @@ public class NetOutput : IConsoleOutput
     private void WriteToConsole (StringBuilder output, ref int lastCol, int row, ref int outputWidth)
     {
         SetCursorPositionImpl (lastCol, row);
-        Console.Write (output);
+        Console.Out.Write (output);
         output.Clear ();
         lastCol += outputWidth;
         outputWidth = 0;
@@ -222,7 +230,7 @@ public class NetOutput : IConsoleOutput
 
         // + 1 is needed because non-Windows is based on 1 instead of 0 and
         // Console.CursorTop/CursorLeft isn't reliable.
-        Console.Out.Write (EscSeqUtils.CSI_SetCursorPosition (row + 1, col + 1));
+        EscSeqUtils.CSI_WriteCursorPosition (Console.Out, row + 1, col + 1);
 
         return true;
     }

+ 28 - 20
Terminal.Gui/ConsoleDrivers/V2/WindowsOutput.cs

@@ -1,4 +1,5 @@
 #nullable enable
+using System.Buffers;
 using System.ComponentModel;
 using System.Runtime.InteropServices;
 using Microsoft.Extensions.Logging;
@@ -6,12 +7,13 @@ using static Terminal.Gui.WindowsConsole;
 
 namespace Terminal.Gui;
 
-internal class WindowsOutput : IConsoleOutput
+internal partial class WindowsOutput : IConsoleOutput
 {
-    [DllImport ("kernel32.dll", EntryPoint = "WriteConsole", SetLastError = true, CharSet = CharSet.Unicode)]
-    private static extern bool WriteConsole (
+    [LibraryImport ("kernel32.dll", EntryPoint = "WriteConsoleW", SetLastError = true, StringMarshalling = StringMarshalling.Utf16)]
+    [return: MarshalAs (UnmanagedType.Bool)]
+    private static partial bool WriteConsole (
         nint hConsoleOutput,
-        string lpbufer,
+        ReadOnlySpan<char> lpbufer,
         uint numberOfCharsToWriten,
         out uint lpNumberOfCharsWritten,
         nint lpReserved
@@ -84,7 +86,7 @@ internal class WindowsOutput : IConsoleOutput
         }
     }
 
-    public void Write (string str)
+    public void Write (ReadOnlySpan<char> str)
     {
         if (!WriteConsole (_screenBuffer, str, (uint)str.Length, out uint _, nint.Zero))
         {
@@ -183,7 +185,6 @@ internal class WindowsOutput : IConsoleOutput
 
     public bool WriteToConsole (Size size, ExtendedCharInfo [] charInfoBuffer, Coord bufferSize, SmallRect window, bool force16Colors)
     {
-        var stringBuilder = new StringBuilder ();
 
         //Debug.WriteLine ("WriteToConsole");
 
@@ -213,10 +214,10 @@ internal class WindowsOutput : IConsoleOutput
         }
         else
         {
-            stringBuilder.Clear ();
+            StringBuilder stringBuilder = new();
 
             stringBuilder.Append (EscSeqUtils.CSI_SaveCursorPosition);
-            stringBuilder.Append (EscSeqUtils.CSI_SetCursorPosition (0, 0));
+            EscSeqUtils.CSI_AppendCursorPosition (stringBuilder, 0, 0);
 
             Attribute? prev = null;
 
@@ -227,8 +228,8 @@ internal class WindowsOutput : IConsoleOutput
                 if (attr != prev)
                 {
                     prev = attr;
-                    stringBuilder.Append (EscSeqUtils.CSI_SetForegroundColorRGB (attr.Foreground.R, attr.Foreground.G, attr.Foreground.B));
-                    stringBuilder.Append (EscSeqUtils.CSI_SetBackgroundColorRGB (attr.Background.R, attr.Background.G, attr.Background.B));
+                    EscSeqUtils.CSI_AppendForegroundColorRGB (stringBuilder, attr.Foreground.R, attr.Foreground.G, attr.Foreground.B);
+                    EscSeqUtils.CSI_AppendBackgroundColorRGB (stringBuilder, attr.Background.R, attr.Background.G, attr.Background.B);
                 }
 
                 if (info.Char != '\x1b')
@@ -247,14 +248,20 @@ internal class WindowsOutput : IConsoleOutput
             stringBuilder.Append (EscSeqUtils.CSI_RestoreCursorPosition);
             stringBuilder.Append (EscSeqUtils.CSI_HideCursor);
 
-            var s = stringBuilder.ToString ();
+            // TODO: Potentially could stackalloc whenever reasonably small (<= 8 kB?) write buffer is needed.
+            char [] rentedWriteArray = ArrayPool<char>.Shared.Rent (minimumLength: stringBuilder.Length);
+            try
+            {
+                Span<char> writeBuffer = rentedWriteArray.AsSpan(0, stringBuilder.Length);
+                stringBuilder.CopyTo (0, writeBuffer, stringBuilder.Length);
 
-            // TODO: requires extensive testing if we go down this route
-            // If console output has changed
-            //if (s != _lastWrite)
-            //{
-            // supply console with the new content
-            result = WriteConsole (_screenBuffer, s, (uint)s.Length, out uint _, nint.Zero);
+                // Supply console with the new content.
+                result = WriteConsole (_screenBuffer, writeBuffer, (uint)writeBuffer.Length, out uint _, nint.Zero);
+            }
+            finally
+            {
+                ArrayPool<char>.Shared.Return (rentedWriteArray);
+            }
 
             foreach (SixelToRender sixel in Application.Sixel)
             {
@@ -297,9 +304,10 @@ internal class WindowsOutput : IConsoleOutput
     /// <inheritdoc/>
     public void SetCursorVisibility (CursorVisibility visibility)
     {
-        var sb = new StringBuilder ();
-        sb.Append (visibility != CursorVisibility.Invisible ? EscSeqUtils.CSI_ShowCursor : EscSeqUtils.CSI_HideCursor);
-        Write (sb.ToString ());
+        string cursorVisibilitySequence = visibility != CursorVisibility.Invisible
+            ? EscSeqUtils.CSI_ShowCursor
+            : EscSeqUtils.CSI_HideCursor;
+        Write (cursorVisibilitySequence);
     }
 
     private Point _lastCursorPosition;

+ 5 - 4
Terminal.Gui/ConsoleDrivers/WindowsDriver/WindowsConsole.cs

@@ -6,7 +6,7 @@ using Terminal.Gui.ConsoleDrivers;
 
 namespace Terminal.Gui;
 
-internal class WindowsConsole
+internal partial class WindowsConsole
 {
     private CancellationTokenSource? _inputReadyCancellationTokenSource;
     private readonly BlockingCollection<InputRecord> _inputQueue = new (new ConcurrentQueue<InputRecord> ());
@@ -926,10 +926,11 @@ internal class WindowsConsole
         ref SmallRect lpWriteRegion
     );
 
-    [DllImport ("kernel32.dll", EntryPoint = "WriteConsole", SetLastError = true, CharSet = CharSet.Unicode)]
-    private static extern bool WriteConsole (
+    [LibraryImport ("kernel32.dll", EntryPoint = "WriteConsoleW", SetLastError = true, StringMarshalling = StringMarshalling.Utf16)]
+    [return: MarshalAs (UnmanagedType.Bool)]
+    private static partial bool WriteConsole (
         nint hConsoleOutput,
-        string lpbufer,
+        ReadOnlySpan<char> lpbufer,
         uint NumberOfCharsToWriten,
         out uint lpNumberOfCharsWritten,
         nint lpReserved

+ 18 - 3
Tests/UnitTests/Input/EscSeqUtilsTests.cs

@@ -1,4 +1,4 @@
-using JetBrains.Annotations;
+using System.Text;
 using UnitTests;
 
 // ReSharper disable HeuristicUnreachableCode
@@ -685,7 +685,7 @@ public class EscSeqUtilsTests
         top.Add (view);
         Application.Begin (top);
 
-        Application.RaiseMouseEvent (new() { Position = new (0, 0), Flags = 0 });
+        Application.RaiseMouseEvent (new () { Position = new (0, 0), Flags = 0 });
 
         ClearAll ();
 
@@ -741,7 +741,7 @@ public class EscSeqUtilsTests
                                          // set Application.WantContinuousButtonPressedView to null
                                          view.WantContinuousButtonPressed = false;
 
-                                         Application.RaiseMouseEvent (new() { Position = new (0, 0), Flags = 0 });
+                                         Application.RaiseMouseEvent (new () { Position = new (0, 0), Flags = 0 });
 
                                          Application.RequestStop ();
                                      }
@@ -1548,6 +1548,21 @@ public class EscSeqUtilsTests
         Assert.Equal (result, cki);
     }
 
+    [Theory]
+    [InlineData (0, 0, $"{EscSeqUtils.CSI}0;0H")]
+    [InlineData (int.MaxValue, int.MaxValue, $"{EscSeqUtils.CSI}2147483647;2147483647H")]
+    [InlineData (int.MinValue, int.MinValue, $"{EscSeqUtils.CSI}-2147483648;-2147483648H")]
+    public void CSI_WriteCursorPosition_ReturnsCorrectEscSeq (int row, int col, string expected)
+    {
+        StringBuilder builder = new();
+        using StringWriter writer = new(builder);
+
+        EscSeqUtils.CSI_WriteCursorPosition (writer, row, col);
+
+        string actual = builder.ToString();
+        Assert.Equal (expected, actual);
+    }
+
     private void ClearAll ()
     {
         EscSeqRequests.Clear ();