From f617142aed4b97fb45638ebba8f47641b604ecc1 Mon Sep 17 00:00:00 2001 From: Will Fuqua Date: Sun, 31 May 2026 09:43:53 +0700 Subject: [PATCH 1/6] add more benchmarks and minor grapheme perf fix --- src/PrettyPrompt/Documents/Grapheme.cs | 38 +++- .../PerKeystrokeBenchmark.cs | 133 +++++++++++ tests/PrettyPrompt.Benchmarks/Program.cs | 14 +- .../SteadyStateRenderBenchmark.cs | 212 ++++++++++++++++++ 4 files changed, 385 insertions(+), 12 deletions(-) create mode 100644 tests/PrettyPrompt.Benchmarks/PerKeystrokeBenchmark.cs create mode 100644 tests/PrettyPrompt.Benchmarks/SteadyStateRenderBenchmark.cs diff --git a/src/PrettyPrompt/Documents/Grapheme.cs b/src/PrettyPrompt/Documents/Grapheme.cs index f9205a7..4492585 100644 --- a/src/PrettyPrompt/Documents/Grapheme.cs +++ b/src/PrettyPrompt/Documents/Grapheme.cs @@ -17,9 +17,20 @@ namespace PrettyPrompt.Documents; /// e.g. an emoji such as "🤦🏼‍♂️" (one cluster, seven s) or a base letter plus /// combining marks - rather than splitting them into halves of a surrogate pair or orphaned /// combining marks. See https://github.com/waf/PrettyPrompt/issues/270. +/// +/// +/// Characters below U+0300 (the first combining mark) are never surrogates, combining marks, joiners, +/// or any other cluster continuation, so they always form their own single-char cluster. The methods +/// here use that as an O(1) fast path (matching the fast path in ) +/// to avoid scanning - important because the caret can be deep into a long document. +/// /// internal static class Grapheme { + private const char FirstCombiningMark = '\u0300'; + + private static bool IsSimple(char c) => c < FirstCombiningMark; + /// /// The smallest cluster boundary strictly greater than /// (i.e. the caret position one grapheme to the right), clamped to the text length. @@ -28,6 +39,9 @@ public static int NextBoundary(string text, int index) { if (index < 0) index = 0; if (index >= text.Length) return text.Length; + // a simple character not followed by a continuation is a single-char cluster + if (IsSimple(text[index]) && (index + 1 == text.Length || IsSimple(text[index + 1]))) + return index + 1; return index + StringInfo.GetNextTextElementLength(text, index); } @@ -39,14 +53,9 @@ public static int PreviousBoundary(string text, int index) { if (index <= 0) return 0; if (index > text.Length) index = text.Length; - int i = 0; - while (i < text.Length) - { - int next = i + StringInfo.GetNextTextElementLength(text, i); - if (next >= index) return i; - i = next; - } - return i; + // if the character ending the previous cluster is simple, that cluster is exactly one char + if (IsSimple(text[index - 1])) return index - 1; + return ScanToBoundary(text, index, inclusive: false); } /// @@ -58,12 +67,21 @@ public static int RoundDownToBoundary(string text, int index) { if (index <= 0) return 0; if (index >= text.Length) return text.Length; + // a simple character at 'index' always starts a new cluster, so 'index' is already a boundary + if (IsSimple(text[index])) return index; + return ScanToBoundary(text, index, inclusive: true); + } + + // Walks clusters from the start of the text to find the boundary at (inclusive) or just below + // (exclusive) 'index'. Only reached for text containing surrogates/combining marks near the caret. + private static int ScanToBoundary(string text, int index, bool inclusive) + { int i = 0; while (i < text.Length) { int next = i + StringInfo.GetNextTextElementLength(text, i); - if (next == index) return index; // already on a boundary - if (next > index) return i; // index is inside [i, next): snap back to the cluster start + if (inclusive && next == index) return index; + if (next >= index) return i; i = next; } return i; diff --git a/tests/PrettyPrompt.Benchmarks/PerKeystrokeBenchmark.cs b/tests/PrettyPrompt.Benchmarks/PerKeystrokeBenchmark.cs new file mode 100644 index 0000000..d8376a5 --- /dev/null +++ b/tests/PrettyPrompt.Benchmarks/PerKeystrokeBenchmark.cs @@ -0,0 +1,133 @@ +#region License Header +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. +#endregion + +using System; +using System.Collections.Generic; +using System.Text; +using BenchmarkDotNet.Attributes; +using PrettyPrompt; +using PrettyPrompt.Documents; +using PrettyPrompt.Highlighting; + +/// +/// Isolates the per-keystroke work that scales with document size. The existing +/// only drives short input through full submits, so it can't see the O(n) re-wrap / cell-build that happens +/// on every keypress. These benchmarks hold a document of lines and measure +/// the three hot stages individually: +/// +/// WordWrap - the full re-wrap a caret-only move (arrow key) pays today. +/// WrapAndBuildCells - wrap + Row/Cell generation for the whole document. +/// FullRedraw - the whole output pipeline (wrap -> cells -> screen -> ANSI diff). +/// +/// Run with: dotnet run -c Release --project tests/PrettyPrompt.Benchmarks -- --filter *PerKeystroke* +/// +[MemoryDiagnoser] +[SimpleJob(launchCount: 1, warmupCount: 3, iterationCount: 5)] +public class PerKeystrokeBenchmark +{ + // a typical-to-wide terminal; lines are generated ~100 columns wide so each wraps once at this width. + private const int Width = 80; + + [Params(10, 100, 1000)] + public int DocumentLineCount; + + private string text = ""; + private StringBuilder stringBuilder = null!; + private int caret; + private IReadOnlyCollection highlights = Array.Empty(); + + [GlobalSetup] + public void Setup() + { + text = GenerateDocument(DocumentLineCount); + stringBuilder = new StringBuilder(text); + caret = text.Length / 2; // simulate editing / navigating in the middle of the document + highlights = GenerateHighlights(text); + } + + /// + /// The cost a caret-only move pays today: fires for cursor moves too, + /// so every arrow key re-wraps the entire document even though only the 2-D cursor coordinate changed. + /// This is the work the caret-only fast path (and incremental wrap) aims to eliminate. + /// + [Benchmark] + public int WordWrap() + { + var wrapped = WordWrapping.WrapEditableCharacters(stringBuilder, caret, Width); + return wrapped.WrappedLines.Count; // consume the result so it isn't optimized away + } + + /// + /// Wrap plus building a Row (and a Cell per character) for every wrapped line. Cells are disposed back + /// to the shared pool so this measures warm steady-state allocation, matching production where + /// Screen.Dispose recycles them each keystroke. Target of viewport-bounded cell generation. + /// + [Benchmark] + public int WrapAndBuildCells() + { + var rows = CellRenderer.ApplyColorToCharacters(highlights, text, Width); + int count = rows.Length; + foreach (var row in rows) row.Dispose(); + return count; + } + + /// + /// The whole output pipeline: wrap + cells + screen buffer + ANSI diff. This diffs against a blank screen + /// (a full redraw, e.g. after Ctrl+L or a resize), so its allocation numbers are the cold upper bound - + /// steady-state per-keystroke is cheaper thanks to cell pooling and the incremental diff against the + /// previous near-identical screen (captured end-to-end by PromptBenchmark). + /// + [Benchmark] + public int FullRedraw() + => Prompt.RenderAnsiOutput(text, highlights, Width).Length; + + private static readonly string[] Words = + { + "apple", "the", "banana", "quick", "mango", "brown", "grape", "fox", + "melon", "jumps", "orange", "over", "pear", "lazy", "peach", "dog", + }; + + // words we mark as highlighted, to give the cell renderer realistic syntax-highlight work. + private static readonly string[] HighlightedWords = + { + "apple", "banana", "mango", "grape", "melon", "orange", "pear", "peach", + }; + + private static string GenerateDocument(int lineCount) + { + var sb = new StringBuilder(); + int w = 0; + for (int line = 0; line < lineCount; line++) + { + int col = 0; + // build a ~100-column line so it wraps once at Width (80) + while (col < 100) + { + if (col > 0) { sb.Append(' '); col++; } + var word = Words[w++ % Words.Length]; + sb.Append(word); + col += word.Length; + } + if (line < lineCount - 1) sb.Append('\n'); + } + return sb.ToString(); + } + + private static IReadOnlyCollection GenerateHighlights(string text) + { + var spans = new List(); + foreach (var word in HighlightedWords) + { + int idx = 0; + while ((idx = text.IndexOf(word, idx, StringComparison.Ordinal)) >= 0) + { + spans.Add(new FormatSpan(idx, word.Length, AnsiColor.Green)); + idx += word.Length; + } + } + return spans; + } +} diff --git a/tests/PrettyPrompt.Benchmarks/Program.cs b/tests/PrettyPrompt.Benchmarks/Program.cs index 60306ce..0ad7a14 100644 --- a/tests/PrettyPrompt.Benchmarks/Program.cs +++ b/tests/PrettyPrompt.Benchmarks/Program.cs @@ -21,9 +21,19 @@ public static class Program { - public static void Main() + public static void Main(string[] args) { - BenchmarkRunner.Run(); + // With no args, run every benchmark non-interactively. Pass BenchmarkDotNet filters to scope a run, e.g. + // dotnet run -c Release --project tests/PrettyPrompt.Benchmarks -- --filter *PerKeystroke* + var switcher = BenchmarkSwitcher.FromAssembly(typeof(Program).Assembly); + if (args.Length == 0) + { + switcher.RunAll(); + } + else + { + switcher.Run(args); + } //For manual running or debugging: //var b = new PromptBenchmark(); diff --git a/tests/PrettyPrompt.Benchmarks/SteadyStateRenderBenchmark.cs b/tests/PrettyPrompt.Benchmarks/SteadyStateRenderBenchmark.cs new file mode 100644 index 0000000..5f8aa3a --- /dev/null +++ b/tests/PrettyPrompt.Benchmarks/SteadyStateRenderBenchmark.cs @@ -0,0 +1,212 @@ +#region License Header +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. +#endregion + +using System; +using System.Collections.Generic; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using BenchmarkDotNet.Attributes; +using PrettyPrompt; +using PrettyPrompt.Consoles; +using PrettyPrompt.Highlighting; +using PrettyPrompt.Panes; +using PrettyPrompt.Rendering; +using PrettyPrompt.TextSelection; +using TextCopy; + +/// +/// Honest steady-state per-keystroke cost in a large document. Unlike +/// (which measures the hot stages in isolation against a blank screen), this drives the real per-keystroke +/// path: it holds a seeded and a persistent whose previous +/// screen is kept across invocations, so each measured keystroke pays the realistic cost - re-wrap (via the +/// real Document.Changed event), the GetText O(n) copy, MeasureConsole, cell pooling (warm), and the +/// incremental ANSI diff against the near-identical previous screen (so the diff/write is cheap, as in +/// production). This is the baseline to measure the Tier A (allocation) and Tier E (screen-buffer pooling) +/// work against, and it captures the loop costs the isolated microbenchmarks miss. +/// +/// Each op is a single keystroke. State is kept net-neutral across invocations by toggling: Navigate moves +/// the caret one cluster right/left around the document midpoint; Type alternates inserting and deleting one +/// character there. The user's syntax-highlight callback cost is deliberately excluded (it's application +/// code, not library code) - a precomputed span list is returned so we still exercise the highlight cache +/// compare and the highlight-to-cell mapping without an O(n) re-scan per keystroke. +/// +/// Run with: dotnet run -c Release --project tests/PrettyPrompt.Benchmarks -- --filter *SteadyState* +/// +[MemoryDiagnoser] +[SimpleJob(launchCount: 1, warmupCount: 3, iterationCount: 5)] +public class SteadyStateRenderBenchmark +{ + private const int ConsoleWidth = 120; + private const int ConsoleHeight = 80; + + [Params(10, 100, 1000)] + public int DocumentLineCount; + + private Renderer renderer = null!; + private CodePane codePane = null!; + private OverloadPane overloadPane = null!; + private CompletionPane completionPane = null!; + private SyntaxHighlighter highlighter = null!; + private KeyPress key = null!; + private int caretMid; + private bool toggled; + + [GlobalSetup] + public void Setup() + { + var text = GenerateDocument(DocumentLineCount); + var highlights = GenerateHighlights(text); + + var console = new SilentConsole(ConsoleWidth, ConsoleHeight); + var configuration = new PromptConfiguration(); + var callbacks = new PrecomputedHighlightCallbacks(highlights); + + renderer = new Renderer(console, configuration); + codePane = new CodePane(console, configuration, callbacks, new WrappedClipboard()); + overloadPane = new OverloadPane(codePane, callbacks, configuration); + completionPane = new CompletionPane(codePane, overloadPane, callbacks, configuration); + codePane.Bind(completionPane, overloadPane); + highlighter = new SyntaxHighlighter(callbacks, hasUserOptedOutFromColor: false); + + // seed the document and park the caret in the middle (the realistic editing position) + codePane.MeasureConsole(); + codePane.Document.InsertAtCaret(codePane, text); + caretMid = text.Length / 2; + codePane.Document.Caret = caretMid; + + key = new KeyPress(new ConsoleKeyInfo('x', ConsoleKey.X, shift: false, alt: false, control: false)); + + // establish the previous screen so subsequent renders exercise the incremental diff, not a full redraw + RenderAfterChange(); + } + + /// One arrow keystroke: caret-only move (no text change) + render. Today this still triggers a full re-wrap. + [Benchmark(Description = "Navigate (caret move + render)")] + public void Navigate() + { + codePane.MeasureConsole(); + toggled = !toggled; + codePane.Document.Caret = toggled ? caretMid + 1 : caretMid; + RenderAfterChange(); + } + + /// One editing keystroke: insert or delete a character at the midpoint + render. Alternates so the document stays net-neutral. + [Benchmark(Description = "Type (insert/delete + render)")] + public void Type() + { + codePane.MeasureConsole(); + if (toggled) + { + codePane.Document.Remove(codePane, codePane.Document.Caret - 1, 1); + } + else + { + codePane.Document.InsertAtCaret(codePane, 'x'); + } + toggled = !toggled; + RenderAfterChange(); + } + + private void RenderAfterChange() + { + // mirrors the per-keystroke tail of Prompt.ReadLineAsync's render path + codePane.MeasureConsole(); + var text = codePane.Document.GetText(); + var highlights = highlighter.HighlightAsync(text, CancellationToken.None).GetAwaiter().GetResult(); + renderer.RenderOutput(result: null, codePane, overloadPane, completionPane, highlights, key); + } + + private static readonly string[] Words = + { + "apple", "the", "banana", "quick", "mango", "brown", "grape", "fox", + "melon", "jumps", "orange", "over", "pear", "lazy", "peach", "dog", + }; + + private static readonly string[] HighlightedWords = + { + "apple", "banana", "mango", "grape", "melon", "orange", "pear", "peach", + }; + + private static string GenerateDocument(int lineCount) + { + var sb = new StringBuilder(); + int w = 0; + for (int line = 0; line < lineCount; line++) + { + int col = 0; + // ~160-column lines so each wraps at the console width + while (col < 160) + { + if (col > 0) { sb.Append(' '); col++; } + var word = Words[w++ % Words.Length]; + sb.Append(word); + col += word.Length; + } + if (line < lineCount - 1) sb.Append('\n'); + } + return sb.ToString(); + } + + private static IReadOnlyCollection GenerateHighlights(string text) + { + var spans = new List(); + foreach (var word in HighlightedWords) + { + int idx = 0; + while ((idx = text.IndexOf(word, idx, StringComparison.Ordinal)) >= 0) + { + spans.Add(new FormatSpan(idx, word.Length, AnsiColor.Green)); + idx += word.Length; + } + } + return spans; + } + + /// Returns the same precomputed spans regardless of input, so the highlight callback is O(1) and we measure library cost, not user-callback cost. + private sealed class PrecomputedHighlightCallbacks : PromptCallbacks + { + private readonly IReadOnlyCollection highlights; + public PrecomputedHighlightCallbacks(IReadOnlyCollection highlights) => this.highlights = highlights; + + protected override Task> HighlightCallbackAsync(string text, CancellationToken cancellationToken) + => Task.FromResult(highlights); + } + + /// A no-op console with a fixed size; output is discarded so we measure work, not terminal I/O. + private sealed class SilentConsole : IConsole + { + public SilentConsole(int width, int height) + { + BufferWidth = width; + WindowHeight = height; + } + + public int CursorTop => 0; + public int BufferWidth { get; } + public int WindowHeight { get; } + public int WindowTop => 0; + public bool KeyAvailable => false; + public bool IsErrorRedirected => false; + public bool CaptureControlC { get => false; set { } } + + public event ConsoleCancelEventHandler? CancelKeyPress { add { } remove { } } + + public ConsoleKeyInfo ReadKey(bool intercept) => default; + public void Clear() { } + public void HideCursor() { } + public void ShowCursor() { } + public void InitVirtualTerminalProcessing() { } + public void Write(string? value) { } + public void WriteError(string? value) { } + public void WriteErrorLine(string? value) { } + public void WriteLine(string? value) { } + public void Write(ReadOnlySpan value) { } + public void WriteError(ReadOnlySpan value) { } + public void WriteErrorLine(ReadOnlySpan value) { } + public void WriteLine(ReadOnlySpan value) { } + } +} From d67dd80f80a31c3162c25b25157b415b5369dffe Mon Sep 17 00:00:00 2001 From: Will Fuqua Date: Sun, 31 May 2026 10:15:56 +0700 Subject: [PATCH 2/6] Performance fix for long documents: viewport-bound cell generation --- src/PrettyPrompt/Highlighting/CellRenderer.cs | 61 ++++++++++++++-- src/PrettyPrompt/Rendering/Renderer.cs | 69 +++++++++++++------ 2 files changed, 103 insertions(+), 27 deletions(-) diff --git a/src/PrettyPrompt/Highlighting/CellRenderer.cs b/src/PrettyPrompt/Highlighting/CellRenderer.cs index d03c4e5..2482b3d 100644 --- a/src/PrettyPrompt/Highlighting/CellRenderer.cs +++ b/src/PrettyPrompt/Highlighting/CellRenderer.cs @@ -6,6 +6,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Text; using PrettyPrompt.Consoles; using PrettyPrompt.Documents; @@ -19,7 +20,23 @@ namespace PrettyPrompt.Highlighting; internal static class CellRenderer { public static Row[] ApplyColorToCharacters(IReadOnlyCollection highlights, IReadOnlyList lines, SelectionSpan? selection, AnsiColor? selectedTextBackground) + => ApplyColorToCharacters(highlights, lines, selection, selectedTextBackground, startLine: 0, endLine: lines.Count); + + /// + /// Builds the /s for the wrapped lines in the half-open range + /// [, ). Building only the visible range (instead + /// of the whole document and then discarding off-screen rows) keeps per-keystroke cost and allocation + /// bounded by the viewport rather than the document size (see PERFORMANCE_PLAN.md Tier C). + /// + /// When > 0, the two pieces of state a full top-down pass would have + /// carried across the skipped lines are seeded explicitly: + /// - currentHighlight: a multi-line highlight span that began above the viewport and is still open. + /// - selectionHighlight: whether the text selection is already "open" at the top of the viewport. + /// + public static Row[] ApplyColorToCharacters(IReadOnlyCollection highlights, IReadOnlyList lines, SelectionSpan? selection, AnsiColor? selectedTextBackground, int startLine, int endLine) { + Debug.Assert(startLine >= 0 && startLine <= endLine && endLine <= lines.Count); + var selectionStart = new ConsoleCoordinate(int.MaxValue, int.MaxValue); //invalid var selectionEnd = new ConsoleCoordinate(int.MaxValue, int.MaxValue); //invalid if (selection.TryGet(out var selectionValue)) @@ -28,12 +45,13 @@ public static Row[] ApplyColorToCharacters(IReadOnlyCollection highl selectionEnd = selectionValue.End; } - bool selectionHighlight = false; + // If the selection began above the viewport and hasn't ended yet, it's already "open" at startLine. + bool selectionHighlight = selectionStart.Row < startLine && selectionEnd.Row >= startLine; var highlightsLookup = HighlightsGroupingPool.Shared.Get(highlights); - var highlightedRows = new Row[lines.Count]; - FormatSpan? currentHighlight = null; - for (int lineIndex = 0; lineIndex < lines.Count; lineIndex++) + var highlightedRows = new Row[endLine - startLine]; + FormatSpan? currentHighlight = SeedCurrentHighlight(highlights, lines, startLine); + for (int lineIndex = startLine; lineIndex < endLine; lineIndex++) { WrappedLine line = lines[lineIndex]; int lineFullWidthCharacterOffset = 0; @@ -83,11 +101,44 @@ public static Row[] ApplyColorToCharacters(IReadOnlyCollection highl } } } - highlightedRows[lineIndex] = row; + highlightedRows[lineIndex - startLine] = row; } return highlightedRows; } + /// + /// When rendering starts partway down the document ( > 0), find the + /// highlight span the top-down pass would have been carrying into : one that + /// began strictly before this line's first character and still covers it. Returns null when starting at + /// the top, or when no span straddles the viewport's top boundary. + /// + private static FormatSpan? SeedCurrentHighlight(IReadOnlyCollection highlights, IReadOnlyList lines, int startLine) + { + if (startLine == 0) + { + return null; + } + + int startCharIndex = lines[startLine].StartIndex; + FormatSpan? seed = null; + foreach (var span in highlights) + { + if (span.Start < startCharIndex && span.Contains(startCharIndex)) + { + // Prefer the span that began closest to the boundary (and, on a tie, the longest) so we + // match the single span the top-down carry would be holding. For the usual disjoint + // (non-overlapping) syntax-highlight spans there is at most one candidate. + if (seed is null + || span.Start > seed.Value.Start + || (span.Start == seed.Value.Start && span.Length > seed.Value.Length)) + { + seed = span; + } + } + } + return seed; + } + private static FormatSpan? HighlightSpan(FormatSpan currentHighlight, Row row, int cellIndex, int endPosition) { var highlightedFullWidthOffset = 0; diff --git a/src/PrettyPrompt/Rendering/Renderer.cs b/src/PrettyPrompt/Rendering/Renderer.cs index f559451..36dcc32 100644 --- a/src/PrettyPrompt/Rendering/Renderer.cs +++ b/src/PrettyPrompt/Rendering/Renderer.cs @@ -145,52 +145,77 @@ private static bool DidCodeAreaResize(Screen previousScreen, Screen currentScree private ScreenArea BuildCodeScreenArea(CodePane codePane, IReadOnlyCollection highlights) { - var highlightedLines = CellRenderer.ApplyColorToCharacters(highlights, codePane.WordWrappedLines, codePane.Selection, configuration.SelectedTextBackground); - - // if we've filled up the full line, add a new line at the end so we can render our cursor on this new line. - if (highlightedLines[^1].Length > 0 - && (highlightedLines[^1].Length >= codePane.CodeAreaWidth - || highlightedLines[^1][^1]?.Text == "\n")) + var wrappedLines = codePane.WordWrappedLines; + int wrappedCount = wrappedLines.Count; + + // If the last wrapped line is full (or ends in a newline), we render an extra blank line below it so + // the cursor can sit on a fresh line. This depends only on the last wrapped line, so we decide it up + // front (measuring a single throwaway row) without having to build every row. + bool trailingBlankLine = LastWrappedLineNeedsTrailingBlank(wrappedLines, codePane.CodeAreaWidth); + int renderedLineCount = wrappedCount + (trailingBlankLine ? 1 : 0); + + // Compute the visible line range FIRST, then build cells only for those lines. Previously every + // wrapped line was turned into a Row, and TrimLinesToViewPortSize then sliced out the off-screen + // ones - which were never placed in a Screen and so never Disposed, leaking their pooled cells on + // every keystroke for any document taller than the window (see PERFORMANCE_PLAN.md Tier C). + (int viewPortStart, int viewPortEnd) = GetViewPortRange(codePane.Cursor.Row, renderedLineCount); + + int wrappedEnd = Math.Min(viewPortEnd, wrappedCount); + var highlightedLines = CellRenderer.ApplyColorToCharacters( + highlights, wrappedLines, codePane.Selection, configuration.SelectedTextBackground, viewPortStart, wrappedEnd); + + // Append the trailing blank line only if it falls within the viewport (the bottom of the document is visible). + if (trailingBlankLine && viewPortEnd == renderedLineCount) { Array.Resize(ref highlightedLines, highlightedLines.Length + 1); highlightedLines[^1] = new Row(0); } - (highlightedLines, int bufferStart) = TrimLinesToViewPortSize(codePane, highlightedLines); - - var codeWidget = new ScreenArea(ConsoleCoordinate.Zero, highlightedLines, TruncateToScreenHeight: false, ViewPortStart: bufferStart); + var codeWidget = new ScreenArea(ConsoleCoordinate.Zero, highlightedLines, TruncateToScreenHeight: false, ViewPortStart: viewPortStart); return codeWidget; } /// - /// If there are too many lines of code to show in the current console window, return a subset - /// of the lines that fit in the console window ("viewport") along with the index of the line - /// where the viewport starts. - /// The lines returned will always contain the line that contains the cursor. + /// Whether an extra blank line should be rendered below the last wrapped line so the cursor can sit on a + /// fresh line. Measures the real cells of the last wrapped line (cell count and last-cell text are + /// independent of highlighting/selection) and returns the row to the pool. /// - private (Row[] rowsInViewPort, int viewPortStart) TrimLinesToViewPortSize(CodePane codePane, Row[] highlightedLines) + private static bool LastWrappedLineNeedsTrailingBlank(IReadOnlyList wrappedLines, int codeAreaWidth) + { + using var lastRow = new Row(wrappedLines[^1].Content); + return lastRow.Length > 0 + && (lastRow.Length >= codeAreaWidth || lastRow[lastRow.Length - 1].Text == "\n"); + } + + /// + /// If there are too many lines of code to show in the current console window, return the half-open range + /// [start, end) of lines that fit in the console window ("viewport"); otherwise the whole document. The + /// range always contains the cursor line. This replaces the previous build-everything-then-slice approach + /// (the old TrimLinesToViewPortSize) so callers can build cells for only the visible rows. + /// + private (int viewPortStart, int viewPortEnd) GetViewPortRange(int cursorRow, int renderedLineCount) { const int BlankBufferLines = 2; - if (highlightedLines.Length <= console.WindowHeight - BlankBufferLines) + int height = console.WindowHeight - BlankBufferLines; + if (renderedLineCount <= height) { - return (highlightedLines, 0); + return (0, renderedLineCount); } - int height = console.WindowHeight - BlankBufferLines; - int bufferStart = codePane.Cursor.Row - height / 2; + int bufferStart = cursorRow - height / 2; int bufferEnd = bufferStart + height; if (bufferStart < 0) { bufferStart = 0; bufferEnd = bufferStart + height; } - else if (bufferEnd > highlightedLines.Length) + else if (bufferEnd > renderedLineCount) { - bufferStart -= (bufferEnd - highlightedLines.Length); - bufferEnd = highlightedLines.Length; + bufferStart -= (bufferEnd - renderedLineCount); + bufferEnd = renderedLineCount; } - return (highlightedLines[bufferStart..bufferEnd], bufferStart); + return (bufferStart, bufferEnd); } private ScreenArea[] BuildCompletionScreenAreas( From c40d54454fe9293a942050dc4ceb984b34b8ef9b Mon Sep 17 00:00:00 2001 From: Will Fuqua Date: Sun, 31 May 2026 10:34:02 +0700 Subject: [PATCH 3/6] Don't rewrap all text for navigation / caret moves --- src/PrettyPrompt/Documents/Document.cs | 11 +++++ .../Documents/StringBuilderWithCaret.cs | 24 ++++++++-- src/PrettyPrompt/Documents/WordWrapping.cs | 47 +++++++++++++++++++ src/PrettyPrompt/Panes/CodePane.cs | 32 ++++++++++++- 4 files changed, 107 insertions(+), 7 deletions(-) diff --git a/src/PrettyPrompt/Documents/Document.cs b/src/PrettyPrompt/Documents/Document.cs index f437271..4cab16e 100644 --- a/src/PrettyPrompt/Documents/Document.cs +++ b/src/PrettyPrompt/Documents/Document.cs @@ -367,6 +367,17 @@ public event Action? Changed remove => stringBuilder.Changed -= value; } + /// + /// Fires only when the document text actually changes, not on caret-only moves (unlike ). + /// Used to trigger a full re-wrap; caret-only moves instead recompute the cursor cheaply from the existing + /// wrapped lines via (see PERFORMANCE_PLAN.md Tier B1). + /// + public event Action? TextChanged + { + add => stringBuilder.TextChanged += value; + remove => stringBuilder.TextChanged -= value; + } + /* * The following methods are forwarding along the StringBuilder APIs. */ diff --git a/src/PrettyPrompt/Documents/StringBuilderWithCaret.cs b/src/PrettyPrompt/Documents/StringBuilderWithCaret.cs index 8b59e08..ca4d82a 100644 --- a/src/PrettyPrompt/Documents/StringBuilderWithCaret.cs +++ b/src/PrettyPrompt/Documents/StringBuilderWithCaret.cs @@ -58,8 +58,12 @@ public void Clear() { if (sb.Length > 0) { - Caret = 0; sb.Clear(); + // Assign the caret field directly rather than via the Caret setter: the setter raises a transient + // caret-only Changed event mid-mutation (text already changed, but no TextChanged announced yet), + // which would drive consumers to recompute the cursor against now-stale state. The single + // InvokeChangedEvent below notifies once, with text and caret already consistent. + caret = 0; InvokeChangedEvent(); } } @@ -67,28 +71,38 @@ public void Clear() public void SetContents(string contents, int? caret = null) { sb.SetContents(contents); - Caret = caret ?? sb.Length; + // Assign the caret field directly (not via the Caret setter) to avoid a transient caret-only Changed + // event before the text change is announced - see the note in Clear(). + this.caret = caret ?? sb.Length; + Debug.Assert(this.caret >= 0 && this.caret <= sb.Length); InvokeChangedEvent(); } public void Insert(int index, char c) { sb.Insert(index, c); - ++Caret; + // Advance the caret via the field, not the Caret setter, so we don't raise a transient caret-only + // Changed event before the text change is announced (see the note in Clear()). The single + // InvokeChangedEvent below fires once, with text and caret already consistent - which matters for + // callers that update live without suspending events (e.g. streaming via InsertAtCaretAsync). + caret++; + Debug.Assert(caret >= 0 && caret <= sb.Length); InvokeChangedEvent(); } public void Insert(int index, ReadOnlySpan text) { sb.Insert(index, text); - Caret += text.Length; + caret += text.Length; // assign the field directly, not the Caret setter - see the note in Insert(int, char). + Debug.Assert(caret >= 0 && caret <= sb.Length); InvokeChangedEvent(); } public void Remove(int startIndex, int length) { sb.Remove(startIndex, length); - Caret = startIndex; + caret = startIndex; // assign the field directly, not the Caret setter - see the note in Insert(int, char). + Debug.Assert(caret >= 0 && caret <= sb.Length); InvokeChangedEvent(); } diff --git a/src/PrettyPrompt/Documents/WordWrapping.cs b/src/PrettyPrompt/Documents/WordWrapping.cs index c701773..2f609a0 100644 --- a/src/PrettyPrompt/Documents/WordWrapping.cs +++ b/src/PrettyPrompt/Documents/WordWrapping.cs @@ -228,6 +228,53 @@ public WordWrappedText(IReadOnlyList wrappedLines, ConsoleCoordinat this.cursor = default; Cursor = cursor; } + + /// + /// Recomputes the 2-D cursor coordinate for a 1-D index from the already-wrapped + /// lines, WITHOUT re-wrapping. Used on caret-only moves (see PERFORMANCE_PLAN.md Tier B1). This must produce + /// the identical coordinate that would compute for the same + /// caret, text and width - a DEBUG assertion in CodePane verifies this on every caret move. + /// + public ConsoleCoordinate GetCursorForCaret(int caret) + { + Debug.Assert(caret >= 0); + + // Find the wrapped line containing the caret: the last line whose StartIndex is <= caret. The boundary + // rule (a caret exactly at a line's StartIndex belongs to THAT line at column 0) mirrors the wrap's + // `isCursorPastCharacter = caret > textIndex` semantics at a line break (WordWrapping.cs). + int lo = 0, hi = WrappedLines.Count - 1, row = 0; + while (lo <= hi) + { + int mid = (lo + hi) / 2; + if (WrappedLines[mid].StartIndex <= caret) + { + row = mid; + lo = mid + 1; + } + else + { + hi = mid - 1; + } + } + + // Column = count of non-control chars on the line before the caret. The only control char in wrapped + // content is the line-terminating '\n', which is always the last char of its line and so never appears + // strictly before the caret within the caret's own line - so this loop matches the wrap's per-char + // `cursorColumn++` exactly (surrogate halves and combining marks each count as one, as they do there). + var content = WrappedLines[row].Content; + int offset = caret - WrappedLines[row].StartIndex; + Debug.Assert(offset >= 0 && offset <= content.Length); + int column = 0; + for (int i = 0; i < offset; i++) + { + if (!char.IsControl(content[i])) + { + column++; + } + } + + return new ConsoleCoordinate(row, column); + } } [DebuggerDisplay("{Content}")] diff --git a/src/PrettyPrompt/Panes/CodePane.cs b/src/PrettyPrompt/Panes/CodePane.cs index f56322a..c705af4 100644 --- a/src/PrettyPrompt/Panes/CodePane.cs +++ b/src/PrettyPrompt/Panes/CodePane.cs @@ -32,6 +32,7 @@ internal class CodePane : IKeyPressHandler private int codeAreaWidth; private int codeAreaHeight; private WordWrappedText wordWrappedText; + private int lastWordWrapWidth; private CompletionPane completionPane = null!; private OverloadPane overloadPane = null!; @@ -125,13 +126,40 @@ public CodePane(IConsole console, PromptConfiguration configuration, IPromptCall MeasureConsole(); Document = new Document(); - Document.Changed += WordWrap; + Document.TextChanged += WordWrap; // full re-wrap only when the text actually changes + Document.Changed += SyncCursor; // every change (incl. caret-only moves): keep the 2-D cursor in sync selectionHandler = new SelectionKeyPressHandler(this); TabSpaces = new string(' ', configuration.TabSize); WordWrap(); - void WordWrap() => wordWrappedText = Document.WrapEditableCharacters(CodeAreaWidth); + void WordWrap() + { + wordWrappedText = Document.WrapEditableCharacters(CodeAreaWidth); + lastWordWrapWidth = CodeAreaWidth; + } + + // On a caret-only move (arrow keys, Home/End, etc.) the text is unchanged, so we skip the O(n) re-wrap + // and recompute just the 2-D cursor from the existing wrapped lines (see PERFORMANCE_PLAN.md Tier B1). + // On a text edit, WordWrap (subscribed to TextChanged, which StringBuilderWithCaret fires *before* + // Changed) has already re-wrapped and set the cursor, so this is a cheap redundant no-op. If the + // code-area width changed (terminal resize) the existing wrapped lines are stale, so fall back to a full + // re-wrap - matching the pre-Tier-B1 behavior where every Document change re-wrapped at the current width. + void SyncCursor() + { + if (CodeAreaWidth != lastWordWrapWidth) + { + WordWrap(); + return; + } + + var recomputed = wordWrappedText.GetCursorForCaret(Document.Caret); +#if DEBUG + var fromFullWrap = Document.WrapEditableCharacters(CodeAreaWidth).Cursor; + Debug.Assert(recomputed.Equals(fromFullWrap), $"Caret-only cursor recompute ({recomputed}) disagrees with a full wrap ({fromFullWrap}) at caret {Document.Caret}."); +#endif + wordWrappedText.Cursor = recomputed; + } } internal void Bind(CompletionPane completionPane, OverloadPane overloadPane) From f255982f13373ce4655ee9a7181db449fe7bea09 Mon Sep 17 00:00:00 2001 From: Will Fuqua Date: Sun, 31 May 2026 10:45:38 +0700 Subject: [PATCH 4/6] Fix missing return to pool in CellRenderer --- src/PrettyPrompt/Highlighting/CellRenderer.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/PrettyPrompt/Highlighting/CellRenderer.cs b/src/PrettyPrompt/Highlighting/CellRenderer.cs index 2482b3d..18a4f3b 100644 --- a/src/PrettyPrompt/Highlighting/CellRenderer.cs +++ b/src/PrettyPrompt/Highlighting/CellRenderer.cs @@ -103,6 +103,11 @@ public static Row[] ApplyColorToCharacters(IReadOnlyCollection highl } highlightedRows[lineIndex - startLine] = row; } + + // Return the lookup to the pool. The dict is local and its values (FormatSpan/ConsoleFormat) are value + // types copied into the cells, so nothing outlives this call. Without this Put the pool stayed empty and + // every render allocated a fresh dictionary sized to ALL highlight spans (large in highlight-heavy docs). + HighlightsGroupingPool.Shared.Put(highlightsLookup); return highlightedRows; } From 38c22b98f21cd03f4ac9338ee60d20824aa598c0 Mon Sep 17 00:00:00 2001 From: Will Fuqua Date: Sun, 31 May 2026 11:16:02 +0700 Subject: [PATCH 5/6] Add ScreenBufferPool and improve object pool algorithm --- src/PrettyPrompt/Highlighting/CellRenderer.cs | 34 +--- src/PrettyPrompt/Pools.cs | 172 +++++++++++++----- src/PrettyPrompt/Rendering/Cell.cs | 21 +-- src/PrettyPrompt/Rendering/Renderer.cs | 1 + src/PrettyPrompt/Rendering/Screen.cs | 13 +- 5 files changed, 156 insertions(+), 85 deletions(-) diff --git a/src/PrettyPrompt/Highlighting/CellRenderer.cs b/src/PrettyPrompt/Highlighting/CellRenderer.cs index 18a4f3b..f834558 100644 --- a/src/PrettyPrompt/Highlighting/CellRenderer.cs +++ b/src/PrettyPrompt/Highlighting/CellRenderer.cs @@ -171,30 +171,17 @@ public static Row[] ApplyColorToCharacters(IReadOnlyCollection highl return ApplyColorToCharacters(highlights, wrapped.WrappedLines, selection: null, selectedTextBackground: null); } - private sealed class HighlightsGroupingPool + private sealed class HighlightsGroupingPool : LockFreePool> { - private readonly Stack> pool = new(); - public static readonly HighlightsGroupingPool Shared = new(); + // One lookup is in flight per render (occasionally two when panes render), so a small cap is plenty. + private HighlightsGroupingPool() : base(maxRetained: 8) { } + public Dictionary Get(IReadOnlyCollection highlights) { - Dictionary? result = null; - lock (pool) - { - if (pool.Count > 0) - { - result = pool.Pop(); - } - } - if (result is null) - { - result = new Dictionary(highlights.Count); - } - else - { - result.EnsureCapacity(highlights.Count); - } + var result = Rent() ?? new Dictionary(highlights.Count); + result.EnsureCapacity(highlights.Count); foreach (var highlight in highlights) { @@ -214,13 +201,10 @@ public Dictionary Get(IReadOnlyCollection highlight return result; } - public void Put(Dictionary list) + public void Put(Dictionary lookup) { - list.Clear(); - lock (pool) - { - pool.Push(list); - } + lookup.Clear(); + ReturnToPool(lookup); } } } \ No newline at end of file diff --git a/src/PrettyPrompt/Pools.cs b/src/PrettyPrompt/Pools.cs index 97c8936..cc27e71 100644 --- a/src/PrettyPrompt/Pools.cs +++ b/src/PrettyPrompt/Pools.cs @@ -1,90 +1,174 @@ -#region License Header +#region License Header // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. #endregion +using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Text; +using System.Threading; namespace PrettyPrompt; -internal sealed class ListPool +/// +/// Base for our small allocation-saving object pools. Lock-free, modelled on the techniques in ASP.NET Core's +/// DefaultObjectPool<T> (we deliberately don't take a dependency on Microsoft.Extensions.ObjectPool, +/// just borrow the approach): a single fastItem hot slot, claimed/released with an +/// compare-exchange, backed by a lock-free for overflow. +/// +/// Retention is bounded by maxRetained so a burst of returns can't grow the pool without limit; returns +/// beyond the cap are dropped and left to the GC. Pass for an effectively unbounded +/// pool (which also skips the counter bookkeeping) when the working set is large but naturally bounded by usage - +/// e.g. the per-frame cells/rows, where alternating rent/return on the render thread self-limits the pool size. +/// +internal abstract class LockFreePool where T : class { - private readonly Stack> pool = new(); + private readonly bool bounded; + private readonly int maxRetained; + private int numRetained; + private readonly ConcurrentQueue overflow = new(); + private T? fastItem; - public static readonly ListPool Shared = new(); + protected LockFreePool(int maxRetained) + { + bounded = maxRetained != int.MaxValue; + // reserve one logical slot for fastItem, mirroring DefaultObjectPool. + this.maxRetained = bounded ? maxRetained - 1 : int.MaxValue; + } - public List Get(int capacity) + /// Takes a pooled instance, or null when the pool is empty (the caller then creates one). + protected T? Rent() + { + // fast path: the single hot slot, claimed atomically. + var item = fastItem; + if (item is not null && Interlocked.CompareExchange(ref fastItem, null, item) == item) + { + return item; + } + // slow path: the overflow queue. + if (overflow.TryDequeue(out item)) + { + if (bounded) Interlocked.Decrement(ref numRetained); + return item; + } + return null; + } + + /// Returns an instance to the pool. Callers reset it (e.g. Clear) before returning. + protected void ReturnToPool(T item) { - List? result = null; - lock (pool) + // fast path: drop it into the hot slot if it's free. + if (fastItem is null && Interlocked.CompareExchange(ref fastItem, item, null) is null) { - if (pool.Count > 0) - { - result = pool.Pop(); - } + return; } - if (result is null) + // slow path: enqueue, unless we're already at the retention cap (then drop it for the GC). + if (!bounded) { - result = new List(capacity); + overflow.Enqueue(item); + } + else if (Interlocked.Increment(ref numRetained) <= maxRetained) + { + overflow.Enqueue(item); } else { - if (result.Capacity < capacity) - { - result.Capacity = capacity; - } + Interlocked.Decrement(ref numRetained); } - return result; } +} - public void Put(List list) +internal sealed class ListPool : LockFreePool> +{ + public static readonly ListPool Shared = new(); + + // Per-frame Rows each hold a pooled list; the working set is large and the render thread alternates + // rent/return, so keep this unbounded rather than risk thrashing with a too-small cap. + private ListPool() : base(maxRetained: int.MaxValue) { } + + public List Get(int capacity) { - list.Clear(); - lock (pool) + var list = Rent(); + if (list is null) { - pool.Push(list); + return new List(capacity); } + if (list.Capacity < capacity) + { + list.Capacity = capacity; + } + return list; + } + + public void Put(List list) + { + list.Clear(); + ReturnToPool(list); } } -internal sealed class StringBuilderPool +internal sealed class StringBuilderPool : LockFreePool { - private readonly Stack pool = new(); - public static readonly StringBuilderPool Shared = new(); + // Only a couple are ever in flight at once (the word-wrap working buffer and the diff buffer). + private StringBuilderPool() : base(maxRetained: 16) { } + public StringBuilder Get(int capacity) { - StringBuilder? result = null; - lock (pool) + var sb = Rent(); + if (sb is null) { - if (pool.Count > 0) - { - result = pool.Pop(); - } + return new StringBuilder(capacity); } - if (result is null) + if (sb.Capacity < capacity) { - result = new StringBuilder(capacity); + sb.Capacity = capacity; } - else + return sb; + } + + public void Put(StringBuilder builder) + { + builder.Clear(); + ReturnToPool(builder); + } +} + +/// +/// Pools the ?[] backing buffer of a . The renderer keeps the +/// current and previous screen alive, so two buffers ping-pong; a small cap covers that plus a little slack +/// across a resize (when the buffer size changes and stale-sized buffers are discarded). Buffers are sized +/// exactly to Width*Height, so callers can keep relying on CellBuffer.Length. +/// +internal sealed class ScreenBufferPool : LockFreePool +{ + public static readonly ScreenBufferPool Shared = new(); + + private ScreenBufferPool() : base(maxRetained: 8) { } + + public Cell?[] Get(int length) + { + if (length == 0) { - if (result.Capacity < capacity) - { - result.Capacity = capacity; - } + return Array.Empty(); } - return result; + var buffer = Rent(); + // Pooled buffers are cleared on return; a wrong-sized one (left over from before a resize) is dropped + // here (not re-pooled) and replaced, so the pool converges back to the current size. + return buffer is not null && buffer.Length == length ? buffer : new Cell?[length]; } - public void Put(StringBuilder list) + public void Put(Cell?[] buffer) { - list.Clear(); - lock (pool) + if (buffer.Length == 0) { - pool.Push(list); + return; } + // the buffer holds references to cells that are about to be recycled, so null them out before reuse. + Array.Clear(buffer, 0, buffer.Length); + ReturnToPool(buffer); } -} \ No newline at end of file +} diff --git a/src/PrettyPrompt/Rendering/Cell.cs b/src/PrettyPrompt/Rendering/Cell.cs index 3c53eba..db86cb6 100644 --- a/src/PrettyPrompt/Rendering/Cell.cs +++ b/src/PrettyPrompt/Rendering/Cell.cs @@ -115,21 +115,15 @@ other is not null && private string GetDebuggerDisplay() => text + " " + Formatting.ToString(); - internal class Pool + internal sealed class Pool : LockFreePool { - private readonly Stack pool = new(); + // Cells are fine-grained and numerous (a screenful per frame), and rent/return alternate on the render + // thread, so keep this unbounded rather than risk reallocating cells every frame with a too-small cap. + public Pool() : base(maxRetained: int.MaxValue) { } public Cell Get(string? text, in ConsoleFormat formatting, int elementWidth = 1, bool isContinuationOfPreviousCharacter = false) { - Cell? result = null; - lock (pool) - { - if (pool.Count > 0) - { - result = pool.Pop(); - } - } - result ??= new Cell(isPoolable: true); + var result = Rent() ?? new Cell(isPoolable: true); result.Initialize(text, in formatting, elementWidth, isContinuationOfPreviousCharacter); return result; } @@ -138,10 +132,7 @@ public void Put(Cell value) { if (value.isPoolable) { - lock (pool) - { - pool.Push(value); - } + ReturnToPool(value); } } } diff --git a/src/PrettyPrompt/Rendering/Renderer.cs b/src/PrettyPrompt/Rendering/Renderer.cs index 36dcc32..019ae04 100644 --- a/src/PrettyPrompt/Rendering/Renderer.cs +++ b/src/PrettyPrompt/Rendering/Renderer.cs @@ -92,6 +92,7 @@ public void RenderOutput( { if (key.ObjectPattern is (Control, L)) { + previouslyRenderedScreen.Dispose(); // return its pooled buffer before we drop it previouslyRenderedScreen = new Screen(0, 0, ConsoleCoordinate.Zero); console.Clear(); // for some reason, using escape codes (ClearEntireScreen and MoveCursorToPosition) leaves // CursorTop in an old (cached?) state. Using Console.Clear() works around this. diff --git a/src/PrettyPrompt/Rendering/Screen.cs b/src/PrettyPrompt/Rendering/Screen.cs index 1f17180..b5c4610 100644 --- a/src/PrettyPrompt/Rendering/Screen.cs +++ b/src/PrettyPrompt/Rendering/Screen.cs @@ -17,6 +17,7 @@ namespace PrettyPrompt.Rendering; internal sealed class Screen : IDisposable { private readonly ScreenArea[] screenAreas; + private bool disposed; public int Width { get; } public int Height { get; } @@ -37,7 +38,7 @@ public Screen(int width, int height, ConsoleCoordinate cursor, params ScreenArea ) .DefaultIfEmpty() .Max(); - this.CellBuffer = new Cell[Width * Height]; + this.CellBuffer = ScreenBufferPool.Shared.Get(Width * Height); this.MaxIndex = FillCharBuffer(screenAreas); this.Cursor = PositionCursor(this, cursor); } @@ -126,9 +127,19 @@ private ConsoleCoordinate PositionCursor(Screen screen, ConsoleCoordinate cursor public void Dispose() { + // Guard against double-dispose: returning the same buffer to the pool twice would let two screens + // rent and write the same buffer. (Row.Dispose is already self-guarded, so disposing the areas twice + // was previously harmless, but the buffer Put is not.) + if (disposed) + { + return; + } + disposed = true; + foreach (var screenArea in screenAreas) { screenArea.Dispose(); } + ScreenBufferPool.Shared.Put(CellBuffer); } } From abc4b93e5341057f6d0a4d301593aeeaf99fc128 Mon Sep 17 00:00:00 2001 From: Will Fuqua Date: Sun, 31 May 2026 11:33:17 +0700 Subject: [PATCH 6/6] Minor cleanup to remove duplicate allocation --- src/PrettyPrompt/Prompt.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/PrettyPrompt/Prompt.cs b/src/PrettyPrompt/Prompt.cs index c2aa0d5..d17cf67 100644 --- a/src/PrettyPrompt/Prompt.cs +++ b/src/PrettyPrompt/Prompt.cs @@ -83,6 +83,10 @@ public async Task ReadLineAsync() history.Track(codePane); cancellationManager.CaptureControlC(); + // The dispatch set is fixed for the lifetime of this prompt, so allocate it once rather than twice + // per keystroke (OnKeyDown + OnKeyUp). Order is significant - panes get first crack in this order. + var keyPressHandlers = new IKeyPressHandler[] { completionPane, overloadPane, history, codePane }; + foreach (var key in KeyPress.ReadForever(console)) { // grab the code area width every key press, so we rerender appropriately when the console is resized. @@ -137,10 +141,10 @@ async Task InterpretKeyPress(KeyPress key, CancellationToken cancellationToken) key = await promptCallbacks.TransformKeyPressAsync(codePane.Document.GetText(), codePane.Document.Caret, key, cancellationToken).ConfigureAwait(false); } - foreach (var panes in new IKeyPressHandler[] { completionPane, overloadPane, history, codePane }) + foreach (var panes in keyPressHandlers) await panes.OnKeyDown(key, cancellationToken).ConfigureAwait(false); - foreach (var panes in new IKeyPressHandler[] { completionPane, overloadPane, history, codePane }) + foreach (var panes in keyPressHandlers) await panes.OnKeyUp(key, cancellationToken).ConfigureAwait(false); await AutoFormatDocument(key, codePane, cancellationToken).ConfigureAwait(false);