From 51ebf0393808c4b39232aa4dae6a21595abca1ab Mon Sep 17 00:00:00 2001 From: Tonttu <15074459+TheTonttu@users.noreply.github.com> Date: Fri, 14 Mar 2025 21:29:56 +0200 Subject: [PATCH 01/12] Add tests for TextFormatter StripCRLF and ReplaceCRLFWithSpace Also made the helper methods internal so they can be accessed from the test project. --- Terminal.Gui/Text/TextFormatter.cs | 4 +- .../Text/TextFormatterTests.cs | 73 +++++++++++++++++++ 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/Terminal.Gui/Text/TextFormatter.cs b/Terminal.Gui/Text/TextFormatter.cs index e576b83cf3..82bef1f0dc 100644 --- a/Terminal.Gui/Text/TextFormatter.cs +++ b/Terminal.Gui/Text/TextFormatter.cs @@ -1185,7 +1185,7 @@ public static bool IsTopToBottom (TextDirection textDirection) } // TODO: Move to StringExtensions? - private static string StripCRLF (string str, bool keepNewLine = false) + internal static string StripCRLF (string str, bool keepNewLine = false) { List runes = str.ToRuneList (); @@ -1229,7 +1229,7 @@ private static string StripCRLF (string str, bool keepNewLine = false) } // TODO: Move to StringExtensions? - private static string ReplaceCRLFWithSpace (string str) + internal static string ReplaceCRLFWithSpace (string str) { List runes = str.ToRuneList (); diff --git a/Tests/UnitTestsParallelizable/Text/TextFormatterTests.cs b/Tests/UnitTestsParallelizable/Text/TextFormatterTests.cs index 18b7e50410..a4dbeafc4e 100644 --- a/Tests/UnitTestsParallelizable/Text/TextFormatterTests.cs +++ b/Tests/UnitTestsParallelizable/Text/TextFormatterTests.cs @@ -2886,4 +2886,77 @@ public void WordWrap_WithNewLines (string text, int maxWidth, int widthOffset, I Assert.Equal (resultLines, wrappedLines); } + + + [Theory] + [InlineData ("No crlf", "No crlf")] + // CRLF + [InlineData ("\r\nThis has crlf in the beginning", "This has crlf in the beginning")] + [InlineData ("This has crlf\r\nin the middle", "This has crlfin the middle")] + [InlineData ("This has crlf in the end\r\n", "This has crlf in the end")] + // LFCR + [InlineData ("\n\rThis has lfcr in the beginning", "\rThis has lfcr in the beginning")] + [InlineData ("This has lfcr\n\rin the middle", "This has lfcr\rin the middle")] + [InlineData ("This has lfcr in the end\n\r", "This has lfcr in the end\r")] + // CR + [InlineData ("\rThis has cr in the beginning", "This has cr in the beginning")] + [InlineData ("This has cr\rin the middle", "This has crin the middle")] + [InlineData ("This has cr in the end\r", "This has cr in the end")] + // LF + [InlineData ("\nThis has lf in the beginning", "This has lf in the beginning")] + [InlineData ("This has lf\nin the middle", "This has lfin the middle")] + [InlineData ("This has lf in the end\n", "This has lf in the end")] + public void StripCRLF_RemovesCrLf (string input, string expected) + { + string actual = TextFormatter.StripCRLF(input, keepNewLine: false); + Assert.Equal (expected, actual); + } + + [Theory] + [InlineData ("No crlf", "No crlf")] + // CRLF + [InlineData ("\r\nThis has crlf in the beginning", "\nThis has crlf in the beginning")] + [InlineData ("This has crlf\r\nin the middle", "This has crlf\nin the middle")] + [InlineData ("This has crlf in the end\r\n", "This has crlf in the end\n")] + // LFCR + [InlineData ("\n\rThis has lfcr in the beginning", "\n\rThis has lfcr in the beginning")] + [InlineData ("This has lfcr\n\rin the middle", "This has lfcr\n\rin the middle")] + [InlineData ("This has lfcr in the end\n\r", "This has lfcr in the end\n\r")] + // CR + [InlineData ("\rThis has cr in the beginning", "\rThis has cr in the beginning")] + [InlineData ("This has cr\rin the middle", "This has cr\rin the middle")] + [InlineData ("This has cr in the end\r", "This has cr in the end\r")] + // LF + [InlineData ("\nThis has lf in the beginning", "\nThis has lf in the beginning")] + [InlineData ("This has lf\nin the middle", "This has lf\nin the middle")] + [InlineData ("This has lf in the end\n", "This has lf in the end\n")] + public void StripCRLF_KeepNewLine_RemovesCarriageReturnFromCrLf (string input, string expected) + { + string actual = TextFormatter.StripCRLF(input, keepNewLine: true); + Assert.Equal (expected, actual); + } + + [Theory] + [InlineData ("No crlf", "No crlf")] + // CRLF + [InlineData ("\r\nThis has crlf in the beginning", " This has crlf in the beginning")] + [InlineData ("This has crlf\r\nin the middle", "This has crlf in the middle")] + [InlineData ("This has crlf in the end\r\n", "This has crlf in the end ")] + // LFCR + [InlineData ("\n\rThis has lfcr in the beginning", " This has lfcr in the beginning")] + [InlineData ("This has lfcr\n\rin the middle", "This has lfcr in the middle")] + [InlineData ("This has lfcr in the end\n\r", "This has lfcr in the end ")] + // CR + [InlineData ("\rThis has cr in the beginning", " This has cr in the beginning")] + [InlineData ("This has cr\rin the middle", "This has cr in the middle")] + [InlineData ("This has cr in the end\r", "This has cr in the end ")] + // LF + [InlineData ("\nThis has lf in the beginning", " This has lf in the beginning")] + [InlineData ("This has lf\nin the middle", "This has lf in the middle")] + [InlineData ("This has lf in the end\n", "This has lf in the end ")] + public void ReplaceCRLFWithSpace_ReplacesCrLfWithSpace (string input, string expected) + { + string actual = TextFormatter.ReplaceCRLFWithSpace(input); + Assert.Equal (expected, actual); + } } From a741904093fccb405b410ebc30c39f3fe20525ec Mon Sep 17 00:00:00 2001 From: Tonttu <15074459+TheTonttu@users.noreply.github.com> Date: Fri, 14 Mar 2025 22:27:22 +0200 Subject: [PATCH 02/12] Rewrite TextFormatter.StripCRLF Uses StringBuilder and char span indexof search to reduce intermediate allocations. The new implementation behaves slightly different compared to old implementation. In synthetic LFCR scenario it is correctly removed while the old implementation left the CR, which seems like an off-by-one error. --- Benchmarks/Text/TextFormatter/StripCRLF.cs | 102 ++++++++++++++++++ Terminal.Gui/Terminal.Gui.csproj | 3 +- Terminal.Gui/Text/TextFormatter.cs | 74 ++++++++----- .../Text/TextFormatterTests.cs | 6 +- 4 files changed, 153 insertions(+), 32 deletions(-) create mode 100644 Benchmarks/Text/TextFormatter/StripCRLF.cs diff --git a/Benchmarks/Text/TextFormatter/StripCRLF.cs b/Benchmarks/Text/TextFormatter/StripCRLF.cs new file mode 100644 index 0000000000..069a23d926 --- /dev/null +++ b/Benchmarks/Text/TextFormatter/StripCRLF.cs @@ -0,0 +1,102 @@ +using System.Text; +using BenchmarkDotNet.Attributes; +using Tui = Terminal.Gui; + +namespace Terminal.Gui.Benchmarks.Text.TextFormatter; + +[MemoryDiagnoser] +public class StripCRLF +{ + /// + /// Benchmark for previous implementation. + /// + /// + /// + /// + [Benchmark] + [ArgumentsSource (nameof (DataSource))] + public string Previous (string str, bool keepNewLine) + { + return RuneListToString (str, keepNewLine); + } + + /// + /// Benchmark for current implementation with StringBuilder and char span index of search. + /// + [Benchmark (Baseline = true)] + [ArgumentsSource (nameof (DataSource))] + public string Current (string str, bool keepNewLine) + { + return Tui.TextFormatter.StripCRLF (str, keepNewLine); + } + + /// + /// Previous implementation with intermediate list allocation. + /// + private static string RuneListToString (string str, bool keepNewLine = false) + { + List runes = str.ToRuneList (); + + for (var i = 0; i < runes.Count; i++) + { + switch ((char)runes [i].Value) + { + case '\n': + if (!keepNewLine) + { + runes.RemoveAt (i); + } + + break; + + case '\r': + if (i + 1 < runes.Count && runes [i + 1].Value == '\n') + { + runes.RemoveAt (i); + + if (!keepNewLine) + { + runes.RemoveAt (i); + } + + i++; + } + else + { + if (!keepNewLine) + { + runes.RemoveAt (i); + } + } + + break; + } + } + + return StringExtensions.ToString (runes); + } + + public IEnumerable DataSource () + { + string textSource = + """ + Ĺόŕéḿ íṕśúḿ d́όĺόŕ śít́ áḿét́, ćόńśéćt́ét́úŕ ád́íṕíśćíńǵ éĺít́. Ṕŕáéśéńt́ q́úíś ĺúćt́úś éĺít́. Íńt́éǵéŕ út́ áŕćú éǵét́ d́όĺόŕ śćéĺéŕíśq́úé ḿát́t́íś áć ét́ d́íáḿ. + Ṕéĺĺéńt́éśq́úé śéd́ d́áṕíb́úś ḿáśśá, v́éĺ t́ŕíśt́íq́úé d́úí. Śéd́ v́ít́áé ńéq́úé éú v́éĺít́ όŕńáŕé áĺíq́úét́. Út́ q́úíś όŕćí t́éḿṕόŕ, t́éḿṕόŕ t́úŕṕíś íd́, t́éḿṕúś ńéq́úé. + Ṕŕáéśéńt́ śáṕíéń t́úŕṕíś, όŕńáŕé v́éĺ ḿáúŕíś át́, v́áŕíúś śúśćíṕít́ áńt́é. Út́ ṕúĺv́íńáŕ t́úŕṕíś ḿáśśá, q́úíś ćúŕśúś áŕćú f́áúćíb́úś íń. + Óŕćí v́áŕíúś ńát́όq́úé ṕéńát́íb́úś ét́ ḿáǵńíś d́íś ṕáŕt́úŕíéńt́ ḿόńt́éś, ńáśćét́úŕ ŕíd́íćúĺúś ḿúś. F́úśćé át́ éx́ b́ĺáńd́ít́, ćόńv́áĺĺíś q́úáḿ ét́, v́úĺṕút́át́é ĺáćúś. + Śúśṕéńd́íśśé śít́ áḿét́ áŕćú út́ áŕćú f́áúćíb́úś v́áŕíúś. V́ív́áḿúś śít́ áḿét́ ḿáx́íḿúś d́íáḿ. Ńáḿ éx́ ĺéό, ṕh́áŕét́ŕá éú ĺόb́όŕt́íś át́, t́ŕíśt́íq́úé út́ f́éĺíś. + """; + // Consistent line endings between systems keeps performance evaluation more consistent. + textSource = textSource.ReplaceLineEndings ("\r\n"); + + bool[] permutations = [true, false]; + foreach (bool keepNewLine in permutations) + { + yield return [textSource [..1], keepNewLine]; + yield return [textSource [..10], keepNewLine]; + yield return [textSource [..100], keepNewLine]; + yield return [textSource [..(textSource.Length / 2)], keepNewLine]; + yield return [textSource, keepNewLine]; + } + } +} diff --git a/Terminal.Gui/Terminal.Gui.csproj b/Terminal.Gui/Terminal.Gui.csproj index e19e3521f9..8fd3908817 100644 --- a/Terminal.Gui/Terminal.Gui.csproj +++ b/Terminal.Gui/Terminal.Gui.csproj @@ -78,9 +78,10 @@ - + + diff --git a/Terminal.Gui/Text/TextFormatter.cs b/Terminal.Gui/Text/TextFormatter.cs index 82bef1f0dc..5e48c128cf 100644 --- a/Terminal.Gui/Text/TextFormatter.cs +++ b/Terminal.Gui/Text/TextFormatter.cs @@ -1,4 +1,5 @@ #nullable enable +using System.Buffers; using System.Diagnostics; namespace Terminal.Gui; @@ -9,6 +10,9 @@ namespace Terminal.Gui; /// public class TextFormatter { + // Utilized in CRLF related helper methods for faster newline char index search. + private static readonly SearchValues NewLineSearchValues = SearchValues.Create(['\r', '\n']); + private Key _hotKey = new (); private int _hotKeyPos = -1; private List _lines = new (); @@ -1187,45 +1191,59 @@ public static bool IsTopToBottom (TextDirection textDirection) // TODO: Move to StringExtensions? internal static string StripCRLF (string str, bool keepNewLine = false) { - List runes = str.ToRuneList (); + StringBuilder stringBuilder = new(); - for (var i = 0; i < runes.Count; i++) + ReadOnlySpan remaining = str.AsSpan (); + while (remaining.Length > 0) { - switch ((char)runes [i].Value) + int nextLineBreakIndex = remaining.IndexOfAny (NewLineSearchValues); + if (nextLineBreakIndex == -1) { - case '\n': - if (!keepNewLine) - { - runes.RemoveAt (i); - } + if (str.Length == remaining.Length) + { + return str; + } + stringBuilder.Append (remaining); + break; + } - break; + ReadOnlySpan slice = remaining.Slice (0, nextLineBreakIndex); + stringBuilder.Append (slice); - case '\r': - if (i + 1 < runes.Count && runes [i + 1].Value == '\n') + // Evaluate how many line break characters to preserve. + int stride; + char lineBreakChar = remaining [nextLineBreakIndex]; + if (lineBreakChar == '\n') + { + stride = 1; + if (keepNewLine) + { + stringBuilder.Append ('\n'); + } + } + else // '\r' + { + bool crlf = (nextLineBreakIndex + 1) < remaining.Length && remaining [nextLineBreakIndex + 1] == '\n'; + if (crlf) + { + stride = 2; + if (keepNewLine) { - runes.RemoveAt (i); - - if (!keepNewLine) - { - runes.RemoveAt (i); - } - - i++; + stringBuilder.Append ('\n'); } - else + } + else + { + stride = 1; + if (keepNewLine) { - if (!keepNewLine) - { - runes.RemoveAt (i); - } + stringBuilder.Append ('\r'); } - - break; + } } + remaining = remaining.Slice (slice.Length + stride); } - - return StringExtensions.ToString (runes); + return stringBuilder.ToString (); } // TODO: Move to StringExtensions? diff --git a/Tests/UnitTestsParallelizable/Text/TextFormatterTests.cs b/Tests/UnitTestsParallelizable/Text/TextFormatterTests.cs index a4dbeafc4e..388714d003 100644 --- a/Tests/UnitTestsParallelizable/Text/TextFormatterTests.cs +++ b/Tests/UnitTestsParallelizable/Text/TextFormatterTests.cs @@ -2895,9 +2895,9 @@ public void WordWrap_WithNewLines (string text, int maxWidth, int widthOffset, I [InlineData ("This has crlf\r\nin the middle", "This has crlfin the middle")] [InlineData ("This has crlf in the end\r\n", "This has crlf in the end")] // LFCR - [InlineData ("\n\rThis has lfcr in the beginning", "\rThis has lfcr in the beginning")] - [InlineData ("This has lfcr\n\rin the middle", "This has lfcr\rin the middle")] - [InlineData ("This has lfcr in the end\n\r", "This has lfcr in the end\r")] + [InlineData ("\n\rThis has lfcr in the beginning", "This has lfcr in the beginning")] + [InlineData ("This has lfcr\n\rin the middle", "This has lfcrin the middle")] + [InlineData ("This has lfcr in the end\n\r", "This has lfcr in the end")] // CR [InlineData ("\rThis has cr in the beginning", "This has cr in the beginning")] [InlineData ("This has cr\rin the middle", "This has crin the middle")] From 4dc47765f8b73cea9ce79c8a5794b8bf0675ff77 Mon Sep 17 00:00:00 2001 From: Tonttu <15074459+TheTonttu@users.noreply.github.com> Date: Fri, 14 Mar 2025 23:10:32 +0200 Subject: [PATCH 03/12] More varied text permutations for the StripCRLF benchmark --- Benchmarks/Text/TextFormatter/StripCRLF.cs | 43 +++++++++++++--------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/Benchmarks/Text/TextFormatter/StripCRLF.cs b/Benchmarks/Text/TextFormatter/StripCRLF.cs index 069a23d926..0878083ce1 100644 --- a/Benchmarks/Text/TextFormatter/StripCRLF.cs +++ b/Benchmarks/Text/TextFormatter/StripCRLF.cs @@ -78,25 +78,34 @@ private static string RuneListToString (string str, bool keepNewLine = false) public IEnumerable DataSource () { - string textSource = - """ - Ĺόŕéḿ íṕśúḿ d́όĺόŕ śít́ áḿét́, ćόńśéćt́ét́úŕ ád́íṕíśćíńǵ éĺít́. Ṕŕáéśéńt́ q́úíś ĺúćt́úś éĺít́. Íńt́éǵéŕ út́ áŕćú éǵét́ d́όĺόŕ śćéĺéŕíśq́úé ḿát́t́íś áć ét́ d́íáḿ. - Ṕéĺĺéńt́éśq́úé śéd́ d́áṕíb́úś ḿáśśá, v́éĺ t́ŕíśt́íq́úé d́úí. Śéd́ v́ít́áé ńéq́úé éú v́éĺít́ όŕńáŕé áĺíq́úét́. Út́ q́úíś όŕćí t́éḿṕόŕ, t́éḿṕόŕ t́úŕṕíś íd́, t́éḿṕúś ńéq́úé. - Ṕŕáéśéńt́ śáṕíéń t́úŕṕíś, όŕńáŕé v́éĺ ḿáúŕíś át́, v́áŕíúś śúśćíṕít́ áńt́é. Út́ ṕúĺv́íńáŕ t́úŕṕíś ḿáśśá, q́úíś ćúŕśúś áŕćú f́áúćíb́úś íń. - Óŕćí v́áŕíúś ńát́όq́úé ṕéńát́íb́úś ét́ ḿáǵńíś d́íś ṕáŕt́úŕíéńt́ ḿόńt́éś, ńáśćét́úŕ ŕíd́íćúĺúś ḿúś. F́úśćé át́ éx́ b́ĺáńd́ít́, ćόńv́áĺĺíś q́úáḿ ét́, v́úĺṕút́át́é ĺáćúś. - Śúśṕéńd́íśśé śít́ áḿét́ áŕćú út́ áŕćú f́áúćíb́úś v́áŕíúś. V́ív́áḿúś śít́ áḿét́ ḿáx́íḿúś d́íáḿ. Ńáḿ éx́ ĺéό, ṕh́áŕét́ŕá éú ĺόb́όŕt́íś át́, t́ŕíśt́íq́úé út́ f́éĺíś. - """; - // Consistent line endings between systems keeps performance evaluation more consistent. - textSource = textSource.ReplaceLineEndings ("\r\n"); + string[] textPermutations = [ + // Extreme newline scenario + "E\r\nx\r\nt\r\nr\r\ne\r\nm\r\ne\r\nn\r\ne\r\nw\r\nl\r\ni\r\nn\r\ne\r\ns\r\nc\r\ne\r\nn\r\na\r\nr\r\ni\r\no\r\n", + // Long text with few line endings + """ + Ĺόŕéḿ íṕśúḿ d́όĺόŕ śít́ áḿét́, ćόńśéćt́ét́úŕ ád́íṕíśćíńǵ éĺít́. Ṕŕáéśéńt́ q́úíś ĺúćt́úś éĺít́. Íńt́éǵéŕ út́ áŕćú éǵét́ d́όĺόŕ śćéĺéŕíśq́úé ḿát́t́íś áć ét́ d́íáḿ. + Ṕéĺĺéńt́éśq́úé śéd́ d́áṕíb́úś ḿáśśá, v́éĺ t́ŕíśt́íq́úé d́úí. Śéd́ v́ít́áé ńéq́úé éú v́éĺít́ όŕńáŕé áĺíq́úét́. Út́ q́úíś όŕćí t́éḿṕόŕ, t́éḿṕόŕ t́úŕṕíś íd́, t́éḿṕúś ńéq́úé. + Ṕŕáéśéńt́ śáṕíéń t́úŕṕíś, όŕńáŕé v́éĺ ḿáúŕíś át́, v́áŕíúś śúśćíṕít́ áńt́é. Út́ ṕúĺv́íńáŕ t́úŕṕíś ḿáśśá, q́úíś ćúŕśúś áŕćú f́áúćíb́úś íń. + Óŕćí v́áŕíúś ńát́όq́úé ṕéńát́íb́úś ét́ ḿáǵńíś d́íś ṕáŕt́úŕíéńt́ ḿόńt́éś, ńáśćét́úŕ ŕíd́íćúĺúś ḿúś. F́úśćé át́ éx́ b́ĺáńd́ít́, ćόńv́áĺĺíś q́úáḿ ét́, v́úĺṕút́át́é ĺáćúś. + Śúśṕéńd́íśśé śít́ áḿét́ áŕćú út́ áŕćú f́áúćíb́úś v́áŕíúś. V́ív́áḿúś śít́ áḿét́ ḿáx́íḿúś d́íáḿ. Ńáḿ éx́ ĺéό, ṕh́áŕét́ŕá éú ĺόb́όŕt́íś át́, t́ŕíśt́íq́úé út́ f́éĺíś. + """ + // Consistent line endings between systems for more consistent performance evaluation. + .ReplaceLineEndings ("\r\n"), + // Long text without line endings + "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla sed euismod metus. Phasellus lectus metus, ultricies a commodo quis, facilisis vitae nulla. " + + "Curabitur mollis ex nisl, vitae mattis nisl consequat at. Aliquam dolor lectus, tincidunt ac nunc eu, elementum molestie lectus. Donec lacinia eget dolor a scelerisque. " + + "Aenean elementum molestie rhoncus. Duis id ornare lorem. Nam eget porta sapien. Etiam rhoncus dignissim leo, ac suscipit magna finibus eu. Curabitur hendrerit elit erat, sit amet suscipit felis condimentum ut. " + + "Nullam semper tempor mi, nec semper quam fringilla eu. Aenean sit amet pretium augue, in posuere ante. Aenean convallis porttitor purus, et posuere velit dictum eu." + ]; - bool[] permutations = [true, false]; - foreach (bool keepNewLine in permutations) + bool[] newLinePermutations = { true, false }; + + foreach (string text in textPermutations) { - yield return [textSource [..1], keepNewLine]; - yield return [textSource [..10], keepNewLine]; - yield return [textSource [..100], keepNewLine]; - yield return [textSource [..(textSource.Length / 2)], keepNewLine]; - yield return [textSource, keepNewLine]; + foreach (bool keepNewLine in newLinePermutations) + { + yield return [text, keepNewLine]; + } } } } From f5ccaa47535fbbd6ca327667c074bb8d570017e7 Mon Sep 17 00:00:00 2001 From: Tonttu <15074459+TheTonttu@users.noreply.github.com> Date: Fri, 14 Mar 2025 23:14:13 +0200 Subject: [PATCH 04/12] StripCRLF early exit when no newline to avoid StringBuilder allocation --- Benchmarks/Text/TextFormatter/StripCRLF.cs | 7 +++- Terminal.Gui/Text/TextFormatter.cs | 48 +++++++++++++--------- 2 files changed, 34 insertions(+), 21 deletions(-) diff --git a/Benchmarks/Text/TextFormatter/StripCRLF.cs b/Benchmarks/Text/TextFormatter/StripCRLF.cs index 0878083ce1..64e1d4ed85 100644 --- a/Benchmarks/Text/TextFormatter/StripCRLF.cs +++ b/Benchmarks/Text/TextFormatter/StripCRLF.cs @@ -4,6 +4,9 @@ namespace Terminal.Gui.Benchmarks.Text.TextFormatter; +/// +/// Benchmarks for performance fine-tuning. +/// [MemoryDiagnoser] public class StripCRLF { @@ -31,7 +34,7 @@ public string Current (string str, bool keepNewLine) } /// - /// Previous implementation with intermediate list allocation. + /// Previous implementation with intermediate rune list. /// private static string RuneListToString (string str, bool keepNewLine = false) { @@ -98,7 +101,7 @@ public IEnumerable DataSource () "Nullam semper tempor mi, nec semper quam fringilla eu. Aenean sit amet pretium augue, in posuere ante. Aenean convallis porttitor purus, et posuere velit dictum eu." ]; - bool[] newLinePermutations = { true, false }; + bool[] newLinePermutations = [true, false]; foreach (string text in textPermutations) { diff --git a/Terminal.Gui/Text/TextFormatter.cs b/Terminal.Gui/Text/TextFormatter.cs index 5e48c128cf..bd5da255e2 100644 --- a/Terminal.Gui/Text/TextFormatter.cs +++ b/Terminal.Gui/Text/TextFormatter.cs @@ -11,7 +11,7 @@ namespace Terminal.Gui; public class TextFormatter { // Utilized in CRLF related helper methods for faster newline char index search. - private static readonly SearchValues NewLineSearchValues = SearchValues.Create(['\r', '\n']); + private static readonly SearchValues NewlineSearchValues = SearchValues.Create(['\r', '\n']); private Key _hotKey = new (); private int _hotKeyPos = -1; @@ -1191,31 +1191,39 @@ public static bool IsTopToBottom (TextDirection textDirection) // TODO: Move to StringExtensions? internal static string StripCRLF (string str, bool keepNewLine = false) { + ReadOnlySpan remaining = str.AsSpan (); + int firstNewlineCharIndex = remaining.IndexOfAny (NewlineSearchValues); + // Early exit to avoid StringBuilder allocation if there are no newline characters. + if (firstNewlineCharIndex < 0) + { + return str; + } + StringBuilder stringBuilder = new(); + ReadOnlySpan firstSegment = remaining[..firstNewlineCharIndex]; + stringBuilder.Append (firstSegment); + + // The first newline is not yet skipped because the "keepNewLine" condition has not been evaluated. + // This means there will be 1 extra iteration because the same newline index is checked again in the loop. + remaining = remaining [firstNewlineCharIndex..]; - ReadOnlySpan remaining = str.AsSpan (); while (remaining.Length > 0) { - int nextLineBreakIndex = remaining.IndexOfAny (NewLineSearchValues); - if (nextLineBreakIndex == -1) + int newlineCharIndex = remaining.IndexOfAny (NewlineSearchValues); + if (newlineCharIndex == -1) { - if (str.Length == remaining.Length) - { - return str; - } - stringBuilder.Append (remaining); break; } - ReadOnlySpan slice = remaining.Slice (0, nextLineBreakIndex); - stringBuilder.Append (slice); + ReadOnlySpan segment = remaining[..newlineCharIndex]; + stringBuilder.Append (segment); + int stride = segment.Length; // Evaluate how many line break characters to preserve. - int stride; - char lineBreakChar = remaining [nextLineBreakIndex]; - if (lineBreakChar == '\n') + char newlineChar = remaining [newlineCharIndex]; + if (newlineChar == '\n') { - stride = 1; + stride++; if (keepNewLine) { stringBuilder.Append ('\n'); @@ -1223,10 +1231,11 @@ internal static string StripCRLF (string str, bool keepNewLine = false) } else // '\r' { - bool crlf = (nextLineBreakIndex + 1) < remaining.Length && remaining [nextLineBreakIndex + 1] == '\n'; + int nextCharIndex = newlineCharIndex + 1; + bool crlf = nextCharIndex < remaining.Length && remaining [nextCharIndex] == '\n'; if (crlf) { - stride = 2; + stride += 2; if (keepNewLine) { stringBuilder.Append ('\n'); @@ -1234,15 +1243,16 @@ internal static string StripCRLF (string str, bool keepNewLine = false) } else { - stride = 1; + stride++; if (keepNewLine) { stringBuilder.Append ('\r'); } } } - remaining = remaining.Slice (slice.Length + stride); + remaining = remaining [stride..]; } + stringBuilder.Append (remaining); return stringBuilder.ToString (); } From 658160d00188450c18035ad66f147c0a71d16f21 Mon Sep 17 00:00:00 2001 From: Tonttu <15074459+TheTonttu@users.noreply.github.com> Date: Sat, 15 Mar 2025 00:06:17 +0200 Subject: [PATCH 05/12] Rewrite TextFormatter.ReplaceCRLFWithSpace Almost identical to the StripCRLF implementation. --- .../TextFormatter/ReplaceCRLFWithSpace.cs | 89 ++++++ Terminal.Gui/Text/TextFormatter.cs | 292 ++++++++++-------- 2 files changed, 248 insertions(+), 133 deletions(-) create mode 100644 Benchmarks/Text/TextFormatter/ReplaceCRLFWithSpace.cs diff --git a/Benchmarks/Text/TextFormatter/ReplaceCRLFWithSpace.cs b/Benchmarks/Text/TextFormatter/ReplaceCRLFWithSpace.cs new file mode 100644 index 0000000000..4b49a04a8b --- /dev/null +++ b/Benchmarks/Text/TextFormatter/ReplaceCRLFWithSpace.cs @@ -0,0 +1,89 @@ +using System.Text; +using BenchmarkDotNet.Attributes; +using Tui = Terminal.Gui; + +namespace Terminal.Gui.Benchmarks.Text.TextFormatter; + +/// +/// Benchmarks for performance fine-tuning. +/// +[MemoryDiagnoser] +public class ReplaceCRLFWithSpace +{ + + /// + /// Benchmark for previous implementation. + /// + [Benchmark] + [ArgumentsSource (nameof (DataSource))] + public string Previous (string str) + { + return ToRuneListReplaceImplementation (str); + } + + /// + /// Benchmark for current implementation. + /// + [Benchmark (Baseline = true)] + [ArgumentsSource (nameof (DataSource))] + public string Current (string str) + { + return Tui.TextFormatter.ReplaceCRLFWithSpace (str); + } + + /// + /// Previous implementation with intermediate rune list. + /// + /// + /// + private static string ToRuneListReplaceImplementation (string str) + { + var runes = str.ToRuneList (); + for (int i = 0; i < runes.Count; i++) + { + switch (runes [i].Value) + { + case '\n': + runes [i] = (Rune)' '; + break; + + case '\r': + if ((i + 1) < runes.Count && runes [i + 1].Value == '\n') + { + runes [i] = (Rune)' '; + runes.RemoveAt (i + 1); + i++; + } + else + { + runes [i] = (Rune)' '; + } + break; + } + } + return Tui.StringExtensions.ToString (runes); + } + + public IEnumerable DataSource () + { + // Extreme newline scenario + yield return "E\r\nx\r\nt\r\nr\r\ne\r\nm\r\ne\r\nn\r\ne\r\nw\r\nl\r\ni\r\nn\r\ne\r\ns\r\nc\r\ne\r\nn\r\na\r\nr\r\ni\r\no\r\n"; + // Long text with few line endings + yield return + """ + Ĺόŕéḿ íṕśúḿ d́όĺόŕ śít́ áḿét́, ćόńśéćt́ét́úŕ ád́íṕíśćíńǵ éĺít́. Ṕŕáéśéńt́ q́úíś ĺúćt́úś éĺít́. Íńt́éǵéŕ út́ áŕćú éǵét́ d́όĺόŕ śćéĺéŕíśq́úé ḿát́t́íś áć ét́ d́íáḿ. + Ṕéĺĺéńt́éśq́úé śéd́ d́áṕíb́úś ḿáśśá, v́éĺ t́ŕíśt́íq́úé d́úí. Śéd́ v́ít́áé ńéq́úé éú v́éĺít́ όŕńáŕé áĺíq́úét́. Út́ q́úíś όŕćí t́éḿṕόŕ, t́éḿṕόŕ t́úŕṕíś íd́, t́éḿṕúś ńéq́úé. + Ṕŕáéśéńt́ śáṕíéń t́úŕṕíś, όŕńáŕé v́éĺ ḿáúŕíś át́, v́áŕíúś śúśćíṕít́ áńt́é. Út́ ṕúĺv́íńáŕ t́úŕṕíś ḿáśśá, q́úíś ćúŕśúś áŕćú f́áúćíb́úś íń. + Óŕćí v́áŕíúś ńát́όq́úé ṕéńát́íb́úś ét́ ḿáǵńíś d́íś ṕáŕt́úŕíéńt́ ḿόńt́éś, ńáśćét́úŕ ŕíd́íćúĺúś ḿúś. F́úśćé át́ éx́ b́ĺáńd́ít́, ćόńv́áĺĺíś q́úáḿ ét́, v́úĺṕút́át́é ĺáćúś. + Śúśṕéńd́íśśé śít́ áḿét́ áŕćú út́ áŕćú f́áúćíb́úś v́áŕíúś. V́ív́áḿúś śít́ áḿét́ ḿáx́íḿúś d́íáḿ. Ńáḿ éx́ ĺéό, ṕh́áŕét́ŕá éú ĺόb́όŕt́íś át́, t́ŕíśt́íq́úé út́ f́éĺíś. + """ + // Consistent line endings between systems for more consistent performance evaluation. + .ReplaceLineEndings ("\r\n"); + // Long text without line endings + yield return + "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla sed euismod metus. Phasellus lectus metus, ultricies a commodo quis, facilisis vitae nulla. " + + "Curabitur mollis ex nisl, vitae mattis nisl consequat at. Aliquam dolor lectus, tincidunt ac nunc eu, elementum molestie lectus. Donec lacinia eget dolor a scelerisque. " + + "Aenean elementum molestie rhoncus. Duis id ornare lorem. Nam eget porta sapien. Etiam rhoncus dignissim leo, ac suscipit magna finibus eu. Curabitur hendrerit elit erat, sit amet suscipit felis condimentum ut. " + + "Nullam semper tempor mi, nec semper quam fringilla eu. Aenean sit amet pretium augue, in posuere ante. Aenean convallis porttitor purus, et posuere velit dictum eu."; + } +} diff --git a/Terminal.Gui/Text/TextFormatter.cs b/Terminal.Gui/Text/TextFormatter.cs index bd5da255e2..4188080949 100644 --- a/Terminal.Gui/Text/TextFormatter.cs +++ b/Terminal.Gui/Text/TextFormatter.cs @@ -442,7 +442,7 @@ public void Draw ( } } } - + /// /// Determines if the viewport width will be used or only the text width will be used, /// If all the viewport area will be filled with whitespaces and the same background color @@ -942,67 +942,67 @@ public Region GetDrawRegion (Rectangle screen, Rectangle maximum = default) { // Horizontal Alignment case Alignment.End when isVertical: - { - int runesWidth = GetColumnsRequiredForVerticalText (linesFormatted, line, linesFormatted.Count - line, TabWidth); - x = screen.Right - runesWidth; + { + int runesWidth = GetColumnsRequiredForVerticalText (linesFormatted, line, linesFormatted.Count - line, TabWidth); + x = screen.Right - runesWidth; - break; - } + break; + } case Alignment.End: - { - int runesWidth = StringExtensions.ToString (runes).GetColumns (); - x = screen.Right - runesWidth; + { + int runesWidth = StringExtensions.ToString (runes).GetColumns (); + x = screen.Right - runesWidth; - break; - } + break; + } case Alignment.Start when isVertical: - { - int runesWidth = line > 0 + { + int runesWidth = line > 0 ? GetColumnsRequiredForVerticalText (linesFormatted, 0, line, TabWidth) : 0; - x = screen.Left + runesWidth; + x = screen.Left + runesWidth; - break; - } + break; + } case Alignment.Start: x = screen.Left; break; case Alignment.Fill when isVertical: - { - int runesWidth = GetColumnsRequiredForVerticalText (linesFormatted, 0, linesFormatted.Count, TabWidth); - int prevLineWidth = line > 0 ? GetColumnsRequiredForVerticalText (linesFormatted, line - 1, 1, TabWidth) : 0; - int firstLineWidth = GetColumnsRequiredForVerticalText (linesFormatted, 0, 1, TabWidth); - int lastLineWidth = GetColumnsRequiredForVerticalText (linesFormatted, linesFormatted.Count - 1, 1, TabWidth); - var interval = (int)Math.Round ((double)(screen.Width + firstLineWidth + lastLineWidth) / linesFormatted.Count); - - x = line == 0 - ? screen.Left - : line < linesFormatted.Count - 1 - ? screen.Width - runesWidth <= lastLineWidth ? screen.Left + prevLineWidth : screen.Left + line * interval - : screen.Right - lastLineWidth; + { + int runesWidth = GetColumnsRequiredForVerticalText (linesFormatted, 0, linesFormatted.Count, TabWidth); + int prevLineWidth = line > 0 ? GetColumnsRequiredForVerticalText (linesFormatted, line - 1, 1, TabWidth) : 0; + int firstLineWidth = GetColumnsRequiredForVerticalText (linesFormatted, 0, 1, TabWidth); + int lastLineWidth = GetColumnsRequiredForVerticalText (linesFormatted, linesFormatted.Count - 1, 1, TabWidth); + var interval = (int)Math.Round ((double)(screen.Width + firstLineWidth + lastLineWidth) / linesFormatted.Count); + + x = line == 0 + ? screen.Left + : line < linesFormatted.Count - 1 + ? screen.Width - runesWidth <= lastLineWidth ? screen.Left + prevLineWidth : screen.Left + line * interval + : screen.Right - lastLineWidth; - break; - } + break; + } case Alignment.Fill: x = screen.Left; break; case Alignment.Center when isVertical: - { - int runesWidth = GetColumnsRequiredForVerticalText (linesFormatted, 0, linesFormatted.Count, TabWidth); - int linesWidth = GetColumnsRequiredForVerticalText (linesFormatted, 0, line, TabWidth); - x = screen.Left + linesWidth + (screen.Width - runesWidth) / 2; + { + int runesWidth = GetColumnsRequiredForVerticalText (linesFormatted, 0, linesFormatted.Count, TabWidth); + int linesWidth = GetColumnsRequiredForVerticalText (linesFormatted, 0, line, TabWidth); + x = screen.Left + linesWidth + (screen.Width - runesWidth) / 2; - break; - } + break; + } case Alignment.Center: - { - int runesWidth = StringExtensions.ToString (runes).GetColumns (); - x = screen.Left + (screen.Width - runesWidth) / 2; + { + int runesWidth = StringExtensions.ToString (runes).GetColumns (); + x = screen.Left + (screen.Width - runesWidth) / 2; - break; - } + break; + } default: Debug.WriteLine ($"Unsupported Alignment: {nameof (VerticalAlignment)}"); @@ -1033,28 +1033,28 @@ public Region GetDrawRegion (Rectangle screen, Rectangle maximum = default) break; case Alignment.Fill: - { - var interval = (int)Math.Round ((double)(screen.Height + 2) / linesFormatted.Count); + { + var interval = (int)Math.Round ((double)(screen.Height + 2) / linesFormatted.Count); - y = line == 0 ? screen.Top : - line < linesFormatted.Count - 1 ? screen.Height - interval <= 1 ? screen.Top + 1 : screen.Top + line * interval : screen.Bottom - 1; + y = line == 0 ? screen.Top : + line < linesFormatted.Count - 1 ? screen.Height - interval <= 1 ? screen.Top + 1 : screen.Top + line * interval : screen.Bottom - 1; - break; - } + break; + } case Alignment.Center when isVertical: - { - int s = (screen.Height - runes.Length) / 2; - y = screen.Top + s; + { + int s = (screen.Height - runes.Length) / 2; + y = screen.Top + s; - break; - } + break; + } case Alignment.Center: - { - int s = (screen.Height - linesFormatted.Count) / 2; - y = screen.Top + line + s; + { + int s = (screen.Height - linesFormatted.Count) / 2; + y = screen.Top + line + s; - break; - } + break; + } default: Debug.WriteLine ($"Unsupported Alignment: {nameof (VerticalAlignment)}"); @@ -1144,48 +1144,48 @@ public Region GetDrawRegion (Rectangle screen, Rectangle maximum = default) public static bool IsHorizontalDirection (TextDirection textDirection) { return textDirection switch - { - TextDirection.LeftRight_TopBottom => true, - TextDirection.LeftRight_BottomTop => true, - TextDirection.RightLeft_TopBottom => true, - TextDirection.RightLeft_BottomTop => true, - _ => false - }; + { + TextDirection.LeftRight_TopBottom => true, + TextDirection.LeftRight_BottomTop => true, + TextDirection.RightLeft_TopBottom => true, + TextDirection.RightLeft_BottomTop => true, + _ => false + }; } /// Check if it is a vertical direction public static bool IsVerticalDirection (TextDirection textDirection) { return textDirection switch - { - TextDirection.TopBottom_LeftRight => true, - TextDirection.TopBottom_RightLeft => true, - TextDirection.BottomTop_LeftRight => true, - TextDirection.BottomTop_RightLeft => true, - _ => false - }; + { + TextDirection.TopBottom_LeftRight => true, + TextDirection.TopBottom_RightLeft => true, + TextDirection.BottomTop_LeftRight => true, + TextDirection.BottomTop_RightLeft => true, + _ => false + }; } /// Check if it is Left to Right direction public static bool IsLeftToRight (TextDirection textDirection) { return textDirection switch - { - TextDirection.LeftRight_TopBottom => true, - TextDirection.LeftRight_BottomTop => true, - _ => false - }; + { + TextDirection.LeftRight_TopBottom => true, + TextDirection.LeftRight_BottomTop => true, + _ => false + }; } /// Check if it is Top to Bottom direction public static bool IsTopToBottom (TextDirection textDirection) { return textDirection switch - { - TextDirection.TopBottom_LeftRight => true, - TextDirection.TopBottom_RightLeft => true, - _ => false - }; + { + TextDirection.TopBottom_LeftRight => true, + TextDirection.TopBottom_RightLeft => true, + _ => false + }; } // TODO: Move to StringExtensions? @@ -1259,34 +1259,60 @@ internal static string StripCRLF (string str, bool keepNewLine = false) // TODO: Move to StringExtensions? internal static string ReplaceCRLFWithSpace (string str) { - List runes = str.ToRuneList (); + ReadOnlySpan remaining = str.AsSpan (); + int firstNewlineCharIndex = remaining.IndexOfAny (NewlineSearchValues); + // Early exit to avoid StringBuilder allocation if there are no newline characters. + if (firstNewlineCharIndex < 0) + { + return str; + } - for (var i = 0; i < runes.Count; i++) + StringBuilder stringBuilder = new(); + ReadOnlySpan firstSegment = remaining[..firstNewlineCharIndex]; + stringBuilder.Append (firstSegment); + + // The first newline is not yet skipped because the newline type has not been evaluated. + // This means there will be 1 extra iteration because the same newline index is checked again in the loop. + remaining = remaining [firstNewlineCharIndex..]; + + while (remaining.Length > 0) { - switch (runes [i].Value) + int newlineCharIndex = remaining.IndexOfAny (NewlineSearchValues); + if (newlineCharIndex == -1) { - case '\n': - runes [i] = (Rune)' '; - - break; + break; + } - case '\r': - if (i + 1 < runes.Count && runes [i + 1].Value == '\n') - { - runes [i] = (Rune)' '; - runes.RemoveAt (i + 1); - i++; - } - else - { - runes [i] = (Rune)' '; - } + ReadOnlySpan segment = remaining[..newlineCharIndex]; + stringBuilder.Append (segment); - break; + int stride = segment.Length; + // Replace newlines + char newlineChar = remaining [newlineCharIndex]; + if (newlineChar == '\n') + { + stride++; + stringBuilder.Append (' '); + } + else // '\r' + { + int nextCharIndex = newlineCharIndex + 1; + bool crlf = nextCharIndex < remaining.Length && remaining [nextCharIndex] == '\n'; + if (crlf) + { + stride += 2; + stringBuilder.Append (' '); + } + else + { + stride++; + stringBuilder.Append (' '); + } } + remaining = remaining [stride..]; } - - return StringExtensions.ToString (runes); + stringBuilder.Append (remaining); + return stringBuilder.ToString (); } // TODO: Move to StringExtensions? @@ -1598,21 +1624,21 @@ int GetNextWhiteSpace (int from, int cWidth, out bool incomplete, int cLength = case ' ': return GetNextWhiteSpace (to + 1, cWidth, out incomplete, length); case '\t': - { - length += tabWidth + 1; - - if (length == tabWidth && tabWidth > cWidth) { - return to + 1; - } + length += tabWidth + 1; - if (length > cWidth && tabWidth > cWidth) - { - return to; - } + if (length == tabWidth && tabWidth > cWidth) + { + return to + 1; + } - return GetNextWhiteSpace (to + 1, cWidth, out incomplete, length); - } + if (length > cWidth && tabWidth > cWidth) + { + return to; + } + + return GetNextWhiteSpace (to + 1, cWidth, out incomplete, length); + } default: to++; @@ -1621,11 +1647,11 @@ int GetNextWhiteSpace (int from, int cWidth, out bool incomplete, int cLength = } return cLength switch - { - > 0 when to < runes.Count && runes [to].Value != ' ' && runes [to].Value != '\t' => from, - > 0 when to < runes.Count && (runes [to].Value == ' ' || runes [to].Value == '\t') => from, - _ => to - }; + { + > 0 when to < runes.Count && runes [to].Value != ' ' && runes [to].Value != '\t' => from, + > 0 when to < runes.Count && (runes [to].Value == ' ' || runes [to].Value == '\t') => from, + _ => to + }; } if (start < text.GetRuneCount ()) @@ -2061,13 +2087,13 @@ public static List Format ( private static string PerformCorrectFormatDirection (TextDirection textDirection, string line) { return textDirection switch - { - TextDirection.RightLeft_BottomTop - or TextDirection.RightLeft_TopBottom - or TextDirection.BottomTop_LeftRight - or TextDirection.BottomTop_RightLeft => StringExtensions.ToString (line.EnumerateRunes ().Reverse ()), - _ => line - }; + { + TextDirection.RightLeft_BottomTop + or TextDirection.RightLeft_TopBottom + or TextDirection.BottomTop_LeftRight + or TextDirection.BottomTop_RightLeft => StringExtensions.ToString (line.EnumerateRunes ().Reverse ()), + _ => line + }; } private static List PerformCorrectFormatDirection (TextDirection textDirection, List runes) @@ -2078,13 +2104,13 @@ private static List PerformCorrectFormatDirection (TextDirection textDirec private static List PerformCorrectFormatDirection (TextDirection textDirection, List lines) { return textDirection switch - { - TextDirection.TopBottom_RightLeft - or TextDirection.LeftRight_BottomTop - or TextDirection.RightLeft_BottomTop - or TextDirection.BottomTop_RightLeft => lines.ToArray ().Reverse ().ToList (), - _ => lines - }; + { + TextDirection.TopBottom_RightLeft + or TextDirection.LeftRight_BottomTop + or TextDirection.RightLeft_BottomTop + or TextDirection.BottomTop_RightLeft => lines.ToArray ().Reverse ().ToList (), + _ => lines + }; } /// From cb255226e10c346eee59b7968441e4aae147b58f Mon Sep 17 00:00:00 2001 From: Tonttu <15074459+TheTonttu@users.noreply.github.com> Date: Sat, 15 Mar 2025 11:21:08 +0200 Subject: [PATCH 06/12] Add benchmark category to the TextFormatter benchmarks --- Benchmarks/Text/TextFormatter/ReplaceCRLFWithSpace.cs | 1 + Benchmarks/Text/TextFormatter/StripCRLF.cs | 1 + 2 files changed, 2 insertions(+) diff --git a/Benchmarks/Text/TextFormatter/ReplaceCRLFWithSpace.cs b/Benchmarks/Text/TextFormatter/ReplaceCRLFWithSpace.cs index 4b49a04a8b..ebfeb05a43 100644 --- a/Benchmarks/Text/TextFormatter/ReplaceCRLFWithSpace.cs +++ b/Benchmarks/Text/TextFormatter/ReplaceCRLFWithSpace.cs @@ -8,6 +8,7 @@ namespace Terminal.Gui.Benchmarks.Text.TextFormatter; /// Benchmarks for performance fine-tuning. /// [MemoryDiagnoser] +[BenchmarkCategory (nameof (Tui.TextFormatter))] public class ReplaceCRLFWithSpace { diff --git a/Benchmarks/Text/TextFormatter/StripCRLF.cs b/Benchmarks/Text/TextFormatter/StripCRLF.cs index 64e1d4ed85..055348e578 100644 --- a/Benchmarks/Text/TextFormatter/StripCRLF.cs +++ b/Benchmarks/Text/TextFormatter/StripCRLF.cs @@ -8,6 +8,7 @@ namespace Terminal.Gui.Benchmarks.Text.TextFormatter; /// Benchmarks for performance fine-tuning. /// [MemoryDiagnoser] +[BenchmarkCategory (nameof (Tui.TextFormatter))] public class StripCRLF { /// From 924dc11baff40851cc5f95a73dcfbc2511e2f347 Mon Sep 17 00:00:00 2001 From: Tonttu <15074459+TheTonttu@users.noreply.github.com> Date: Sat, 15 Mar 2025 12:19:30 +0200 Subject: [PATCH 07/12] Rewrite StringExtensions.ToString(IEnumerable) Appends rune chars to StringBuilder avoiding intermediate string allocation for each rune append. --- .../StringExtensions/ToStringEnumerable.cs | 70 +++++++++++++++++++ Benchmarks/Text/TextFormatter/StripCRLF.cs | 2 +- Terminal.Gui/Text/StringExtensions.cs | 12 ++-- 3 files changed, 78 insertions(+), 6 deletions(-) create mode 100644 Benchmarks/Text/StringExtensions/ToStringEnumerable.cs diff --git a/Benchmarks/Text/StringExtensions/ToStringEnumerable.cs b/Benchmarks/Text/StringExtensions/ToStringEnumerable.cs new file mode 100644 index 0000000000..43eef23cee --- /dev/null +++ b/Benchmarks/Text/StringExtensions/ToStringEnumerable.cs @@ -0,0 +1,70 @@ +using System.Text; +using BenchmarkDotNet.Attributes; +using Tui = Terminal.Gui; + +namespace Terminal.Gui.Benchmarks.Text.StringExtensions; + +/// +/// Benchmarks for performance fine-tuning. +/// +[MemoryDiagnoser] +public class ToStringEnumerable +{ + + /// + /// Benchmark for previous implementation. + /// + [Benchmark] + [ArgumentsSource (nameof (DataSource))] + public string Previous (IEnumerable runes, int size) + { + return StringAppendInLoop (runes); + } + + /// + /// Benchmark for current implementation with rune chars appending to StringBuilder. + /// + /// + /// + [Benchmark (Baseline = true)] + [ArgumentsSource (nameof (DataSource))] + public string Current (IEnumerable runes, int size) + { + return Tui.StringExtensions.ToString (runes); + } + + /// + /// Previous implementation with string append in a loop. + /// + private static string StringAppendInLoop (IEnumerable runes) + { + var str = string.Empty; + + foreach (Rune rune in runes) + { + str += rune.ToString (); + } + + return str; + } + + public IEnumerable DataSource () + { + string textSource = + """ + Ĺόŕéḿ íṕśúḿ d́όĺόŕ śít́ áḿét́, ćόńśéćt́ét́úŕ ád́íṕíśćíńǵ éĺít́. Ṕŕáéśéńt́ q́úíś ĺúćt́úś éĺít́. Íńt́éǵéŕ út́ áŕćú éǵét́ d́όĺόŕ śćéĺéŕíśq́úé ḿát́t́íś áć ét́ d́íáḿ. + Ṕéĺĺéńt́éśq́úé śéd́ d́áṕíb́úś ḿáśśá, v́éĺ t́ŕíśt́íq́úé d́úí. Śéd́ v́ít́áé ńéq́úé éú v́éĺít́ όŕńáŕé áĺíq́úét́. Út́ q́úíś όŕćí t́éḿṕόŕ, t́éḿṕόŕ t́úŕṕíś íd́, t́éḿṕúś ńéq́úé. + Ṕŕáéśéńt́ śáṕíéń t́úŕṕíś, όŕńáŕé v́éĺ ḿáúŕíś át́, v́áŕíúś śúśćíṕít́ áńt́é. Út́ ṕúĺv́íńáŕ t́úŕṕíś ḿáśśá, q́úíś ćúŕśúś áŕćú f́áúćíb́úś íń. + Óŕćí v́áŕíúś ńát́όq́úé ṕéńát́íb́úś ét́ ḿáǵńíś d́íś ṕáŕt́úŕíéńt́ ḿόńt́éś, ńáśćét́úŕ ŕíd́íćúĺúś ḿúś. F́úśćé át́ éx́ b́ĺáńd́ít́, ćόńv́áĺĺíś q́úáḿ ét́, v́úĺṕút́át́é ĺáćúś. + Śúśṕéńd́íśśé śít́ áḿét́ áŕćú út́ áŕćú f́áúćíb́úś v́áŕíúś. V́ív́áḿúś śít́ áḿét́ ḿáx́íḿúś d́íáḿ. Ńáḿ éx́ ĺéό, ṕh́áŕét́ŕá éú ĺόb́όŕt́íś át́, t́ŕíśt́íq́úé út́ f́éĺíś. + """; + + // Extra argument as workaround for the summary grouping different length collections to same baseline making comparison difficult. + int[] sizes = [1, 10, 100, textSource.Length / 2, textSource.Length]; + + foreach (int size in sizes) + { + yield return [textSource.EnumerateRunes ().Take (size).ToArray (), size]; + } + } +} diff --git a/Benchmarks/Text/TextFormatter/StripCRLF.cs b/Benchmarks/Text/TextFormatter/StripCRLF.cs index 055348e578..f12dd2831d 100644 --- a/Benchmarks/Text/TextFormatter/StripCRLF.cs +++ b/Benchmarks/Text/TextFormatter/StripCRLF.cs @@ -77,7 +77,7 @@ private static string RuneListToString (string str, bool keepNewLine = false) } } - return StringExtensions.ToString (runes); + return Tui.StringExtensions.ToString (runes); } public IEnumerable DataSource () diff --git a/Terminal.Gui/Text/StringExtensions.cs b/Terminal.Gui/Text/StringExtensions.cs index 538b63975c..4ad58912b2 100644 --- a/Terminal.Gui/Text/StringExtensions.cs +++ b/Terminal.Gui/Text/StringExtensions.cs @@ -124,14 +124,16 @@ public static (Rune Rune, int Size) DecodeRune (this string str, int start = 0, /// public static string ToString (IEnumerable runes) { - var str = string.Empty; - + StringBuilder stringBuilder = new(); + const int maxCharsPerRune = 2; + Span charBuffer = stackalloc char[maxCharsPerRune]; foreach (Rune rune in runes) { - str += rune.ToString (); + int charsWritten = rune.EncodeToUtf16 (charBuffer); + ReadOnlySpan runeChars = charBuffer [..charsWritten]; + stringBuilder.Append (runeChars); } - - return str; + return stringBuilder.ToString (); } /// Converts a byte generic collection into a string in the provided encoding (default is UTF8) From 196169d5bf5de142347a74c826fef71898da8176 Mon Sep 17 00:00:00 2001 From: Tonttu <15074459+TheTonttu@users.noreply.github.com> Date: Sat, 15 Mar 2025 14:15:55 +0200 Subject: [PATCH 08/12] StringExtensions.ToString(IEnumerable) stackalloc char buffer with StringBuilder fallback --- .../StringExtensions/ToStringEnumerable.cs | 46 ++++++++++++------- Terminal.Gui/Text/StringExtensions.cs | 38 +++++++++++++-- 2 files changed, 64 insertions(+), 20 deletions(-) diff --git a/Benchmarks/Text/StringExtensions/ToStringEnumerable.cs b/Benchmarks/Text/StringExtensions/ToStringEnumerable.cs index 43eef23cee..8d5d55f41c 100644 --- a/Benchmarks/Text/StringExtensions/ToStringEnumerable.cs +++ b/Benchmarks/Text/StringExtensions/ToStringEnumerable.cs @@ -16,27 +16,28 @@ public class ToStringEnumerable /// [Benchmark] [ArgumentsSource (nameof (DataSource))] - public string Previous (IEnumerable runes, int size) + public string Previous (IEnumerable runes, int len) { - return StringAppendInLoop (runes); + return StringConcatInLoop (runes); } /// - /// Benchmark for current implementation with rune chars appending to StringBuilder. + /// Benchmark for current implementation with stackalloc char buffer and + /// fallback to rune chars appending to StringBuilder. /// /// /// [Benchmark (Baseline = true)] [ArgumentsSource (nameof (DataSource))] - public string Current (IEnumerable runes, int size) + public string Current (IEnumerable runes, int len) { return Tui.StringExtensions.ToString (runes); } /// - /// Previous implementation with string append in a loop. + /// Previous implementation with string concatenation in a loop. /// - private static string StringAppendInLoop (IEnumerable runes) + private static string StringConcatInLoop (IEnumerable runes) { var str = string.Empty; @@ -49,22 +50,35 @@ private static string StringAppendInLoop (IEnumerable runes) } public IEnumerable DataSource () + { + // Extra length argument as workaround for the summary grouping + // different length collections to same baseline making comparison difficult. + foreach (string text in GetTextData ()) + { + Rune [] runes = [..text.EnumerateRunes ()]; + yield return [runes, runes.Length]; + } + } + + private IEnumerable GetTextData () { string textSource = """ - Ĺόŕéḿ íṕśúḿ d́όĺόŕ śít́ áḿét́, ćόńśéćt́ét́úŕ ád́íṕíśćíńǵ éĺít́. Ṕŕáéśéńt́ q́úíś ĺúćt́úś éĺít́. Íńt́éǵéŕ út́ áŕćú éǵét́ d́όĺόŕ śćéĺéŕíśq́úé ḿát́t́íś áć ét́ d́íáḿ. - Ṕéĺĺéńt́éśq́úé śéd́ d́áṕíb́úś ḿáśśá, v́éĺ t́ŕíśt́íq́úé d́úí. Śéd́ v́ít́áé ńéq́úé éú v́éĺít́ όŕńáŕé áĺíq́úét́. Út́ q́úíś όŕćí t́éḿṕόŕ, t́éḿṕόŕ t́úŕṕíś íd́, t́éḿṕúś ńéq́úé. - Ṕŕáéśéńt́ śáṕíéń t́úŕṕíś, όŕńáŕé v́éĺ ḿáúŕíś át́, v́áŕíúś śúśćíṕít́ áńt́é. Út́ ṕúĺv́íńáŕ t́úŕṕíś ḿáśśá, q́úíś ćúŕśúś áŕćú f́áúćíb́úś íń. - Óŕćí v́áŕíúś ńát́όq́úé ṕéńát́íb́úś ét́ ḿáǵńíś d́íś ṕáŕt́úŕíéńt́ ḿόńt́éś, ńáśćét́úŕ ŕíd́íćúĺúś ḿúś. F́úśćé át́ éx́ b́ĺáńd́ít́, ćόńv́áĺĺíś q́úáḿ ét́, v́úĺṕút́át́é ĺáćúś. - Śúśṕéńd́íśśé śít́ áḿét́ áŕćú út́ áŕćú f́áúćíb́úś v́áŕíúś. V́ív́áḿúś śít́ áḿét́ ḿáx́íḿúś d́íáḿ. Ńáḿ éx́ ĺéό, ṕh́áŕét́ŕá éú ĺόb́όŕt́íś át́, t́ŕíśt́íq́úé út́ f́éĺíś. - """; + Ĺόŕéḿ íṕśúḿ d́όĺόŕ śít́ áḿét́, ćόńśéćt́ét́úŕ ád́íṕíśćíńǵ éĺít́. Ṕŕáéśéńt́ q́úíś ĺúćt́úś éĺít́. Íńt́éǵéŕ út́ áŕćú éǵét́ d́όĺόŕ śćéĺéŕíśq́úé ḿát́t́íś áć ét́ d́íáḿ. + Ṕéĺĺéńt́éśq́úé śéd́ d́áṕíb́úś ḿáśśá, v́éĺ t́ŕíśt́íq́úé d́úí. Śéd́ v́ít́áé ńéq́úé éú v́éĺít́ όŕńáŕé áĺíq́úét́. Út́ q́úíś όŕćí t́éḿṕόŕ, t́éḿṕόŕ t́úŕṕíś íd́, t́éḿṕúś ńéq́úé. + Ṕŕáéśéńt́ śáṕíéń t́úŕṕíś, όŕńáŕé v́éĺ ḿáúŕíś át́, v́áŕíúś śúśćíṕít́ áńt́é. Út́ ṕúĺv́íńáŕ t́úŕṕíś ḿáśśá, q́úíś ćúŕśúś áŕćú f́áúćíb́úś íń. + Óŕćí v́áŕíúś ńát́όq́úé ṕéńát́íb́úś ét́ ḿáǵńíś d́íś ṕáŕt́úŕíéńt́ ḿόńt́éś, ńáśćét́úŕ ŕíd́íćúĺúś ḿúś. F́úśćé át́ éx́ b́ĺáńd́ít́, ćόńv́áĺĺíś q́úáḿ ét́, v́úĺṕút́át́é ĺáćúś. + Śúśṕéńd́íśśé śít́ áḿét́ áŕćú út́ áŕćú f́áúćíb́úś v́áŕíúś. V́ív́áḿúś śít́ áḿét́ ḿáx́íḿúś d́íáḿ. Ńáḿ éx́ ĺéό, ṕh́áŕét́ŕá éú ĺόb́όŕt́íś át́, t́ŕíśt́íq́úé út́ f́éĺíś. + """; - // Extra argument as workaround for the summary grouping different length collections to same baseline making comparison difficult. - int[] sizes = [1, 10, 100, textSource.Length / 2, textSource.Length]; + int[] lengths = [1, 10, 100, textSource.Length / 2, textSource.Length]; - foreach (int size in sizes) + foreach (int length in lengths) { - yield return [textSource.EnumerateRunes ().Take (size).ToArray (), size]; + yield return textSource [..length]; } + + string textLongerThanStackallocThreshold = string.Concat(Enumerable.Repeat(textSource, 10)); + yield return textLongerThanStackallocThreshold; } } diff --git a/Terminal.Gui/Text/StringExtensions.cs b/Terminal.Gui/Text/StringExtensions.cs index 4ad58912b2..a40143af83 100644 --- a/Terminal.Gui/Text/StringExtensions.cs +++ b/Terminal.Gui/Text/StringExtensions.cs @@ -124,13 +124,43 @@ public static (Rune Rune, int Size) DecodeRune (this string str, int start = 0, /// public static string ToString (IEnumerable runes) { - StringBuilder stringBuilder = new(); const int maxCharsPerRune = 2; - Span charBuffer = stackalloc char[maxCharsPerRune]; + // Max stackalloc ~2 kB + const int maxStackallocTextBufferSize = 1048; + + Span runeBuffer = stackalloc char[maxCharsPerRune]; + // Use stackalloc buffer if rune count is easily available and the count is reasonable. + if (runes.TryGetNonEnumeratedCount (out int count)) + { + if (count == 0) + { + return string.Empty; + } + + int maxRequiredTextBufferSize = count * maxCharsPerRune; + if (maxRequiredTextBufferSize <= maxStackallocTextBufferSize) + { + Span textBuffer = stackalloc char[maxRequiredTextBufferSize]; + Span remainingBuffer = textBuffer; + foreach (Rune rune in runes) + { + int charsWritten = rune.EncodeToUtf16 (runeBuffer); + ReadOnlySpan runeChars = runeBuffer [..charsWritten]; + runeChars.CopyTo (remainingBuffer); + remainingBuffer = remainingBuffer [runeChars.Length..]; + } + + ReadOnlySpan text = textBuffer[..^remainingBuffer.Length]; + return text.ToString (); + } + } + + // Fallback to StringBuilder append. + StringBuilder stringBuilder = new(); foreach (Rune rune in runes) { - int charsWritten = rune.EncodeToUtf16 (charBuffer); - ReadOnlySpan runeChars = charBuffer [..charsWritten]; + int charsWritten = rune.EncodeToUtf16 (runeBuffer); + ReadOnlySpan runeChars = runeBuffer [..charsWritten]; stringBuilder.Append (runeChars); } return stringBuilder.ToString (); From 77a0296efa58034eb758ab95139f6d5f3d6d7d2d Mon Sep 17 00:00:00 2001 From: Tonttu <15074459+TheTonttu@users.noreply.github.com> Date: Sun, 16 Mar 2025 00:19:06 +0200 Subject: [PATCH 09/12] Rewrite TextFormatter.RemoveHotKeySpecifier Uses stackalloc char buffer with fallback to rented array. --- .../TextFormatter/RemoveHotKeySpecifier.cs | 97 +++++++++++++++++++ Terminal.Gui/Text/TextFormatter.cs | 42 +++++--- 2 files changed, 128 insertions(+), 11 deletions(-) create mode 100644 Benchmarks/Text/TextFormatter/RemoveHotKeySpecifier.cs diff --git a/Benchmarks/Text/TextFormatter/RemoveHotKeySpecifier.cs b/Benchmarks/Text/TextFormatter/RemoveHotKeySpecifier.cs new file mode 100644 index 0000000000..e72dc0e1e2 --- /dev/null +++ b/Benchmarks/Text/TextFormatter/RemoveHotKeySpecifier.cs @@ -0,0 +1,97 @@ +using System.Text; +using BenchmarkDotNet.Attributes; +using Tui = Terminal.Gui; + +namespace Terminal.Gui.Benchmarks.Text.TextFormatter; + +/// +/// Benchmarks for performance fine-tuning. +/// +[MemoryDiagnoser] +[BenchmarkCategory (nameof(Tui.TextFormatter))] +public class RemoveHotKeySpecifier +{ + // Omit from summary table. + private static readonly Rune HotkeySpecifier = (Rune)'_'; + + /// + /// Benchmark for previous implementation. + /// + [Benchmark] + [ArgumentsSource (nameof (DataSource))] + public string Previous (string text, int hotPos) + { + return StringConcatLoop (text, hotPos, HotkeySpecifier); + } + + /// + /// Benchmark for current implementation with stackalloc char buffer and fallback to rented array. + /// + [Benchmark (Baseline = true)] + [ArgumentsSource (nameof (DataSource))] + public string Current (string text, int hotPos) + { + return Tui.TextFormatter.RemoveHotKeySpecifier (text, hotPos, HotkeySpecifier); + } + + /// + /// Previous implementation with string concatenation in a loop. + /// + public static string StringConcatLoop (string text, int hotPos, Rune hotKeySpecifier) + { + if (string.IsNullOrEmpty (text)) + { + return text; + } + + // Scan + var start = string.Empty; + var i = 0; + + foreach (Rune c in text.EnumerateRunes ()) + { + if (c == hotKeySpecifier && i == hotPos) + { + i++; + + continue; + } + + start += c; + i++; + } + + return start; + } + + public IEnumerable DataSource () + { + string[] texts = [ + "", + // Typical scenario. + "_Save file (Ctrl+S)", + // Medium text, hotkey specifier somewhere in the middle. + "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla sed euismod metus. _Phasellus lectus metus, ultricies a commodo quis, facilisis vitae nulla.", + // Long text, hotkey specifier almost at the beginning. + "Ĺόŕéḿ íṕśúḿ d́όĺόŕ śít́ áḿét́, ćόńśéćt́ét́úŕ ád́íṕíśćíńǵ éĺít́. _Ṕŕáéśéńt́ q́úíś ĺúćt́úś éĺít́. Íńt́éǵéŕ út́ áŕćú éǵét́ d́όĺόŕ śćéĺéŕíśq́úé ḿát́t́íś áć ét́ d́íáḿ. " + + "Ṕéĺĺéńt́éśq́úé śéd́ d́áṕíb́úś ḿáśśá, v́éĺ t́ŕíśt́íq́úé d́úí. Śéd́ v́ít́áé ńéq́úé éú v́éĺít́ όŕńáŕé áĺíq́úét́. Út́ q́úíś όŕćí t́éḿṕόŕ, t́éḿṕόŕ t́úŕṕíś íd́, t́éḿṕúś ńéq́úé. " + + "Ṕŕáéśéńt́ śáṕíéń t́úŕṕíś, όŕńáŕé v́éĺ ḿáúŕíś át́, v́áŕíúś śúśćíṕít́ áńt́é. Út́ ṕúĺv́íńáŕ t́úŕṕíś ḿáśśá, q́úíś ćúŕśúś áŕćú f́áúćíb́úś íń.", + // Long text, hotkey specifier almost at the end. + "Ĺόŕéḿ íṕśúḿ d́όĺόŕ śít́ áḿét́, ćόńśéćt́ét́úŕ ád́íṕíśćíńǵ éĺít́. Ṕŕáéśéńt́ q́úíś ĺúćt́úś éĺít́. Íńt́éǵéŕ út́ áŕćú éǵét́ d́όĺόŕ śćéĺéŕíśq́úé ḿát́t́íś áć ét́ d́íáḿ. " + + "Ṕéĺĺéńt́éśq́úé śéd́ d́áṕíb́úś ḿáśśá, v́éĺ t́ŕíśt́íq́úé d́úí. Śéd́ v́ít́áé ńéq́úé éú v́éĺít́ όŕńáŕé áĺíq́úét́. Út́ q́úíś όŕćí t́éḿṕόŕ, t́éḿṕόŕ t́úŕṕíś íd́, t́éḿṕúś ńéq́úé. " + + "Ṕŕáéśéńt́ śáṕíéń t́úŕṕíś, όŕńáŕé v́éĺ ḿáúŕíś át́, v́áŕíúś śúśćíṕít́ áńt́é. _Út́ ṕúĺv́íńáŕ t́úŕṕíś ḿáśśá, q́úíś ćúŕśúś áŕćú f́áúćíb́úś íń.", + ]; + + foreach (string text in texts) + { + int hotPos = text.EnumerateRunes() + .Select((r, i) => r == HotkeySpecifier ? i : -1) + .FirstOrDefault(i => i > -1, -1); + + yield return [text, hotPos]; + } + + // Typical scenario but without hotkey and with misleading position. + yield return ["Save file (Ctrl+S)", 3]; + } +} diff --git a/Terminal.Gui/Text/TextFormatter.cs b/Terminal.Gui/Text/TextFormatter.cs index 4188080949..ed49590e11 100644 --- a/Terminal.Gui/Text/TextFormatter.cs +++ b/Terminal.Gui/Text/TextFormatter.cs @@ -2444,24 +2444,44 @@ public static string RemoveHotKeySpecifier (string text, int hotPos, Rune hotKey return text; } - // Scan - var start = string.Empty; - var i = 0; - - foreach (Rune c in text.EnumerateRunes ()) + const int maxStackallocCharBufferSize = 512; // ~1 kiB + char[]? rentedBufferArray = null; + try { - if (c == hotKeySpecifier && i == hotPos) + Span buffer = text.Length <= maxStackallocCharBufferSize + ? stackalloc char[text.Length] + : (rentedBufferArray = ArrayPool.Shared.Rent(text.Length)); + + int i = 0; + var remainingBuffer = buffer; + foreach (Rune c in text.EnumerateRunes ()) { + if (c == hotKeySpecifier && i == hotPos) + { + i++; + continue; + } + int charsWritten = c.EncodeToUtf16 (remainingBuffer); + remainingBuffer = remainingBuffer [charsWritten..]; i++; + } - continue; + ReadOnlySpan newText = buffer [..^remainingBuffer.Length]; + // If the resulting string would be the same as original then just return the original. + if (newText.Equals(text, StringComparison.Ordinal)) + { + return text; } - start += c; - i++; + return new string (newText); + } + finally + { + if (rentedBufferArray != null) + { + ArrayPool.Shared.Return (rentedBufferArray); + } } - - return start; } #endregion // Static Members From e88351c20983a3bf232bf090e0a81926ca32622b Mon Sep 17 00:00:00 2001 From: Tonttu <15074459+TheTonttu@users.noreply.github.com> Date: Sun, 16 Mar 2025 00:37:59 +0200 Subject: [PATCH 10/12] StringExtensions.ToString(IEnumerable) remove extra rune chars copy --- Terminal.Gui/Text/StringExtensions.cs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/Terminal.Gui/Text/StringExtensions.cs b/Terminal.Gui/Text/StringExtensions.cs index a40143af83..3167bcf306 100644 --- a/Terminal.Gui/Text/StringExtensions.cs +++ b/Terminal.Gui/Text/StringExtensions.cs @@ -125,10 +125,8 @@ public static (Rune Rune, int Size) DecodeRune (this string str, int start = 0, public static string ToString (IEnumerable runes) { const int maxCharsPerRune = 2; - // Max stackalloc ~2 kB - const int maxStackallocTextBufferSize = 1048; + const int maxStackallocTextBufferSize = 1048; // ~2 kB - Span runeBuffer = stackalloc char[maxCharsPerRune]; // Use stackalloc buffer if rune count is easily available and the count is reasonable. if (runes.TryGetNonEnumeratedCount (out int count)) { @@ -144,10 +142,8 @@ public static string ToString (IEnumerable runes) Span remainingBuffer = textBuffer; foreach (Rune rune in runes) { - int charsWritten = rune.EncodeToUtf16 (runeBuffer); - ReadOnlySpan runeChars = runeBuffer [..charsWritten]; - runeChars.CopyTo (remainingBuffer); - remainingBuffer = remainingBuffer [runeChars.Length..]; + int charsWritten = rune.EncodeToUtf16 (remainingBuffer); + remainingBuffer = remainingBuffer [charsWritten..]; } ReadOnlySpan text = textBuffer[..^remainingBuffer.Length]; @@ -157,6 +153,7 @@ public static string ToString (IEnumerable runes) // Fallback to StringBuilder append. StringBuilder stringBuilder = new(); + Span runeBuffer = stackalloc char[maxCharsPerRune]; foreach (Rune rune in runes) { int charsWritten = rune.EncodeToUtf16 (runeBuffer); From 076b2ca1fe86b51610408796c744b87721537908 Mon Sep 17 00:00:00 2001 From: Tonttu <15074459+TheTonttu@users.noreply.github.com> Date: Sun, 16 Mar 2025 07:31:33 +0200 Subject: [PATCH 11/12] StringExtensions.ToString(IEnumerable) use rented array as alternative buffer --- .../StringExtensions/ToStringEnumerable.cs | 2 +- Terminal.Gui/Text/StringExtensions.cs | 19 +++++++++++++++---- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/Benchmarks/Text/StringExtensions/ToStringEnumerable.cs b/Benchmarks/Text/StringExtensions/ToStringEnumerable.cs index 8d5d55f41c..b44f33ad5e 100644 --- a/Benchmarks/Text/StringExtensions/ToStringEnumerable.cs +++ b/Benchmarks/Text/StringExtensions/ToStringEnumerable.cs @@ -22,7 +22,7 @@ public string Previous (IEnumerable runes, int len) } /// - /// Benchmark for current implementation with stackalloc char buffer and + /// Benchmark for current implementation with char buffer and /// fallback to rune chars appending to StringBuilder. /// /// diff --git a/Terminal.Gui/Text/StringExtensions.cs b/Terminal.Gui/Text/StringExtensions.cs index 3167bcf306..0dc1acdec3 100644 --- a/Terminal.Gui/Text/StringExtensions.cs +++ b/Terminal.Gui/Text/StringExtensions.cs @@ -127,7 +127,7 @@ public static string ToString (IEnumerable runes) const int maxCharsPerRune = 2; const int maxStackallocTextBufferSize = 1048; // ~2 kB - // Use stackalloc buffer if rune count is easily available and the count is reasonable. + // If rune count is easily available use stackalloc buffer or alternatively rented array. if (runes.TryGetNonEnumeratedCount (out int count)) { if (count == 0) @@ -135,10 +135,14 @@ public static string ToString (IEnumerable runes) return string.Empty; } - int maxRequiredTextBufferSize = count * maxCharsPerRune; - if (maxRequiredTextBufferSize <= maxStackallocTextBufferSize) + char[]? rentedBufferArray = null; + try { - Span textBuffer = stackalloc char[maxRequiredTextBufferSize]; + int maxRequiredTextBufferSize = count * maxCharsPerRune; + Span textBuffer = maxRequiredTextBufferSize <= maxStackallocTextBufferSize + ? stackalloc char[maxRequiredTextBufferSize] + : (rentedBufferArray = ArrayPool.Shared.Rent(maxRequiredTextBufferSize)); + Span remainingBuffer = textBuffer; foreach (Rune rune in runes) { @@ -149,6 +153,13 @@ public static string ToString (IEnumerable runes) ReadOnlySpan text = textBuffer[..^remainingBuffer.Length]; return text.ToString (); } + finally + { + if (rentedBufferArray != null) + { + ArrayPool.Shared.Return (rentedBufferArray); + } + } } // Fallback to StringBuilder append. From 601bc970835ed60489ad0d16027a55275d5964bd Mon Sep 17 00:00:00 2001 From: Tonttu <15074459+TheTonttu@users.noreply.github.com> Date: Sun, 16 Mar 2025 07:43:41 +0200 Subject: [PATCH 12/12] Consistent kilobyte unit in comments --- Terminal.Gui/Text/TextFormatter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Terminal.Gui/Text/TextFormatter.cs b/Terminal.Gui/Text/TextFormatter.cs index ed49590e11..2478ddc3b8 100644 --- a/Terminal.Gui/Text/TextFormatter.cs +++ b/Terminal.Gui/Text/TextFormatter.cs @@ -2444,7 +2444,7 @@ public static string RemoveHotKeySpecifier (string text, int hotPos, Rune hotKey return text; } - const int maxStackallocCharBufferSize = 512; // ~1 kiB + const int maxStackallocCharBufferSize = 512; // ~1 kB char[]? rentedBufferArray = null; try {