diff --git a/RunCommand.Test/LineOutputHandlerTests.cs b/RunCommand.Test/LineOutputHandlerTests.cs index 3b397c8..b21b819 100644 --- a/RunCommand.Test/LineOutputHandlerTests.cs +++ b/RunCommand.Test/LineOutputHandlerTests.cs @@ -10,6 +10,7 @@ public class LineOutputHandlerTests private static readonly string[] ExpectedALast = ["a", "last"]; private static readonly string[] ExpectedALastNext = ["a", "last", "next"]; private static readonly string[] ExpectedOops = ["oops"]; + private static readonly string[] ExpectedCrlfAcrossReads = ["a", "b", "", "c"]; [TestMethod] public void HandleStandardOutputDataShouldProcessLinesCorrectly() @@ -75,7 +76,7 @@ public void HandleStandardOutputDataShouldBufferIncompleteLines() // Assert Assert.AreEqual(expectedLines.Length, index); - Assert.AreEqual("Incomplete", handler.outputBuffer); + Assert.AreEqual("Incomplete", handler.outputBuffer.ToString()); } [TestMethod] @@ -98,7 +99,7 @@ public void HandleStandardErrorDataShouldBufferIncompleteLines() // Assert Assert.AreEqual(expectedLines.Length, index); - Assert.AreEqual("Incomplete", handler.errorBuffer); + Assert.AreEqual("Incomplete", handler.errorBuffer.ToString()); } [TestMethod] @@ -114,7 +115,7 @@ public void HandleStandardOutputDataShouldTreatCrlfSplitAcrossChunksAsOneLineBre // Assert Assert.AreSequenceEqual(ExpectedXY, lines); - Assert.AreEqual(string.Empty, handler.outputBuffer); + Assert.AreEqual(string.Empty, handler.outputBuffer.ToString()); } [TestMethod] @@ -130,7 +131,7 @@ public void HandleStandardErrorDataShouldTreatCrlfSplitAcrossChunksAsOneLineBrea // Assert Assert.AreSequenceEqual(ExpectedXY, lines); - Assert.AreEqual(string.Empty, handler.errorBuffer); + Assert.AreEqual(string.Empty, handler.errorBuffer.ToString()); } [TestMethod] @@ -145,7 +146,7 @@ public void HandleStandardOutputDataShouldHoldBackTrailingCarriageReturn() // Assert Assert.IsEmpty(lines); - Assert.AreEqual("x\r", handler.outputBuffer); + Assert.AreEqual("x\r", handler.outputBuffer.ToString()); } [TestMethod] @@ -160,7 +161,7 @@ public void HandleStandardOutputDataShouldSplitOnEveryLineEndingKind() // Assert Assert.AreSequenceEqual(ExpectedEveryLineEndingKind, lines); - Assert.AreEqual("e", handler.outputBuffer); + Assert.AreEqual("e", handler.outputBuffer.ToString()); } [TestMethod] @@ -179,8 +180,8 @@ public void CompleteShouldDeliverTheFinalUnterminatedLine() // Assert Assert.AreSequenceEqual(ExpectedALast, output); Assert.AreSequenceEqual(ExpectedOops, error); - Assert.AreEqual("", handler.outputBuffer); - Assert.AreEqual("", handler.errorBuffer); + Assert.AreEqual("", handler.outputBuffer.ToString()); + Assert.AreEqual("", handler.errorBuffer.ToString()); } [TestMethod] @@ -228,4 +229,48 @@ public void CompleteShouldKeepAPartialLineFromLeakingIntoTheNextRun() // Assert Assert.AreSequenceEqual(ExpectedALastNext, lines); } + + [TestMethod] + public void HandleStandardOutputDataShouldTreatALineBreakSplitAcrossReadsAsOne() + { + // Arrange + List lines = []; + LineOutputHandler handler = new(onStandardOutput: lines.Add); + + // Act + handler.HandleStandardOutputData("a\r"); + handler.HandleStandardOutputData("\nb\r"); + handler.HandleStandardOutputData("\r"); + handler.HandleStandardOutputData("c\n"); + + // Assert + Assert.AreSequenceEqual(ExpectedCrlfAcrossReads, lines); + Assert.AreEqual("", handler.outputBuffer.ToString()); + } + + [TestMethod] + public void HandleStandardOutputDataShouldBeLinearInTheLengthOfALongLine() + { + // Arrange + const int chunkCount = 1024; + const int chunkLength = 4096; + string chunk = new('a', chunkLength); + List lines = []; + LineOutputHandler handler = new(onStandardOutput: lines.Add); + System.Diagnostics.Stopwatch stopwatch = System.Diagnostics.Stopwatch.StartNew(); + + // Act + for (int i = 0; i < chunkCount; i++) + { + handler.HandleStandardOutputData(chunk); + } + + handler.HandleStandardOutputData("\n"); + stopwatch.Stop(); + + // Assert + Assert.HasCount(1, lines); + Assert.AreEqual(chunkCount * chunkLength, lines[0].Length); + Assert.IsLessThan(TimeSpan.FromSeconds(2), stopwatch.Elapsed, $"4 MB in 4 KB reads took {stopwatch.Elapsed}"); + } } diff --git a/RunCommand/LineOutputHandler.cs b/RunCommand/LineOutputHandler.cs index d9b4aab..700b9f5 100644 --- a/RunCommand/LineOutputHandler.cs +++ b/RunCommand/LineOutputHandler.cs @@ -13,12 +13,12 @@ public class LineOutputHandler : OutputHandler /// /// Buffer to store incomplete lines from standard output. /// - internal string outputBuffer = ""; + internal readonly StringBuilder outputBuffer = new(); /// /// Buffer to store incomplete lines from standard error. /// - internal string errorBuffer = ""; + internal readonly StringBuilder errorBuffer = new(); /// /// Initializes a new instance of the class. @@ -38,7 +38,7 @@ public LineOutputHandler(Action? onStandardOutput = null, Action internal override void HandleStandardOutputData(string data) { Ensure.NotNull(data); - ProcessDataByLine(data, ref outputBuffer, OnStandardOutput); + ProcessDataByLine(data, outputBuffer, OnStandardOutput); } /// @@ -49,7 +49,7 @@ internal override void HandleStandardOutputData(string data) internal override void HandleStandardErrorData(string data) { Ensure.NotNull(data); - ProcessDataByLine(data, ref errorBuffer, OnStandardError); + ProcessDataByLine(data, errorBuffer, OnStandardError); } /// @@ -58,8 +58,8 @@ internal override void HandleStandardErrorData(string data) /// internal override void Complete() { - FlushBuffer(ref outputBuffer, OnStandardOutput); - FlushBuffer(ref errorBuffer, OnStandardError); + FlushBuffer(outputBuffer, OnStandardOutput); + FlushBuffer(errorBuffer, OnStandardError); } /// @@ -71,15 +71,20 @@ internal override void Complete() /// A buffer that ends in a CR is a line whose break had not yet been confirmed as CR or CRLF. /// At the end of the stream it is a CR on its own, so it ends the line rather than being part of it. /// - private static void FlushBuffer(ref string buffer, Action? onLineReceived) + private static void FlushBuffer(StringBuilder buffer, Action? onLineReceived) { if (buffer.Length == 0) { return; } - string line = buffer[^1] == '\r' ? buffer[..^1] : buffer; - buffer = ""; + if (buffer[^1] == '\r') + { + buffer.Length--; + } + + string line = buffer.ToString(); + buffer.Clear(); onLineReceived?.Invoke(line); } @@ -90,31 +95,37 @@ private static void FlushBuffer(ref string buffer, Action? onLineReceive /// The buffer to store incomplete lines. /// The action to be invoked for each complete line received. /// - /// Line endings are recognised on the buffered text rather than on each chunk, so a CRLF split across two - /// reads is still one line break. A trailing CR stays in the buffer until the next chunk shows whether an LF follows. + /// Line endings are recognised across reads, so a CRLF split across two reads is still one line break. + /// A trailing CR stays in the buffer until the next chunk shows whether an LF follows. + /// Only the newly arrived is scanned, and the partial line is appended to rather than + /// copied, so the cost is linear in the output size however long a line grows. /// - private static void ProcessDataByLine(string data, ref string buffer, Action? onLineReceived) + private static void ProcessDataByLine(string data, StringBuilder buffer, Action? onLineReceived) { - buffer += data; - int lineStart = 0; - int i = 0; - while (i < buffer.Length) + if (data.Length == 0) + { + return; + } + + int i = CompletePendingCarriageReturn(data, buffer, onLineReceived); + int lineStart = i; + while (i < data.Length) { - char c = buffer[i]; + char c = data[i]; if (c == '\r') { - if (i == buffer.Length - 1) + if (i == data.Length - 1) { break; } - onLineReceived?.Invoke(buffer[lineStart..i]); - i += buffer[i + 1] == '\n' ? 2 : 1; + EmitLine(buffer, data, lineStart, i, onLineReceived); + i += data[i + 1] == '\n' ? 2 : 1; lineStart = i; } else if (IsLineBreak(c)) { - onLineReceived?.Invoke(buffer[lineStart..i]); + EmitLine(buffer, data, lineStart, i, onLineReceived); i++; lineStart = i; } @@ -124,7 +135,53 @@ private static void ProcessDataByLine(string data, ref string buffer, Action + /// Ends the buffered line when the previous read finished on a CR, now that shows whether + /// an LF follows it. + /// + /// The newly arrived data, which must not be empty. + /// The buffer holding an incomplete line. + /// The action to be invoked for the completed line. + /// The index in to resume scanning from: 1 to skip the LF of a CRLF, otherwise 0. + private static int CompletePendingCarriageReturn(string data, StringBuilder buffer, Action? onLineReceived) + { + if (buffer.Length == 0 || buffer[^1] != '\r') + { + return 0; + } + + buffer.Length--; + EmitLine(buffer, data, 0, 0, onLineReceived); + return data[0] == '\n' ? 1 : 0; + } + + /// + /// Invokes with the buffered partial line followed by + /// from up to , and clears the buffer. + /// + /// The buffer holding the start of the line from earlier reads. + /// The data the rest of the line comes from. + /// The index in where the rest of the line starts. + /// The index in of the line break that ends the line. + /// The action to be invoked for the line. + private static void EmitLine(StringBuilder buffer, string data, int start, int end, Action? onLineReceived) + { + string line; + if (buffer.Length == 0) + { + line = data[start..end]; + } + else + { + buffer.Append(data, start, end - start); + line = buffer.ToString(); + buffer.Clear(); + } + + onLineReceived?.Invoke(line); } ///