Scan only new data in LineOutputHandler so long lines cost linear time

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<string> 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<string> 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}");
+	}
 }

RunCommand/LineOutputHandler.cs

@@ -13,12 +13,12 @@ public class LineOutputHandler : OutputHandler
 	/// <summary>
 	/// Buffer to store incomplete lines from standard output.
 	/// </summary>
-	internal string outputBuffer = "";
+	internal readonly StringBuilder outputBuffer = new();
 
 	/// <summary>
 	/// Buffer to store incomplete lines from standard error.
 	/// </summary>
-	internal string errorBuffer = "";
+	internal readonly StringBuilder errorBuffer = new();
 
 	/// <summary>
 	/// Initializes a new instance of the <see cref="LineOutputHandler"/> class.
@@ -38,7 +38,7 @@ public LineOutputHandler(Action<string>? onStandardOutput = null, Action<string>
 	internal override void HandleStandardOutputData(string data)
 	{
 		Ensure.NotNull(data);
-		ProcessDataByLine(data, ref outputBuffer, OnStandardOutput);
+		ProcessDataByLine(data, outputBuffer, OnStandardOutput);
 	}
 
 	/// <summary>
@@ -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);
 	}
 
 	/// <summary>
@@ -58,8 +58,8 @@ internal override void HandleStandardErrorData(string data)
 	/// </summary>
 	internal override void Complete()
 	{
-		FlushBuffer(ref outputBuffer, OnStandardOutput);
-		FlushBuffer(ref errorBuffer, OnStandardError);
+		FlushBuffer(outputBuffer, OnStandardOutput);
+		FlushBuffer(errorBuffer, OnStandardError);
 	}
 
 	/// <summary>
@@ -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.
 	/// </remarks>
-	private static void FlushBuffer(ref string buffer, Action<string>? onLineReceived)
+	private static void FlushBuffer(StringBuilder buffer, Action<string>? 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<string>? onLineReceive
 	/// <param name="buffer">The buffer to store incomplete lines.</param>
 	/// <param name="onLineReceived">The action to be invoked for each complete line received.</param>
 	/// <remarks>
-	/// 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 <paramref name="data"/> 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.
 	/// </remarks>
-	private static void ProcessDataByLine(string data, ref string buffer, Action<string>? onLineReceived)
+	private static void ProcessDataByLine(string data, StringBuilder buffer, Action<string>? 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<str
 			}
 		}
 
-		buffer = buffer[lineStart..];
+		buffer.Append(data, lineStart, data.Length - lineStart);
+	}
+
+	/// <summary>
+	/// Ends the buffered line when the previous read finished on a CR, now that <paramref name="data"/> shows whether
+	/// an LF follows it.
+	/// </summary>
+	/// <param name="data">The newly arrived data, which must not be empty.</param>
+	/// <param name="buffer">The buffer holding an incomplete line.</param>
+	/// <param name="onLineReceived">The action to be invoked for the completed line.</param>
+	/// <returns>The index in <paramref name="data"/> to resume scanning from: 1 to skip the LF of a CRLF, otherwise 0.</returns>
+	private static int CompletePendingCarriageReturn(string data, StringBuilder buffer, Action<string>? onLineReceived)
+	{
+		if (buffer.Length == 0 || buffer[^1] != '\r')
+		{
+			return 0;
+		}
+
+		buffer.Length--;
+		EmitLine(buffer, data, 0, 0, onLineReceived);
+		return data[0] == '\n' ? 1 : 0;
+	}
+
+	/// <summary>
+	/// Invokes <paramref name="onLineReceived"/> with the buffered partial line followed by
+	/// <paramref name="data"/> from <paramref name="start"/> up to <paramref name="end"/>, and clears the buffer.
+	/// </summary>
+	/// <param name="buffer">The buffer holding the start of the line from earlier reads.</param>
+	/// <param name="data">The data the rest of the line comes from.</param>
+	/// <param name="start">The index in <paramref name="data"/> where the rest of the line starts.</param>
+	/// <param name="end">The index in <paramref name="data"/> of the line break that ends the line.</param>
+	/// <param name="onLineReceived">The action to be invoked for the line.</param>
+	private static void EmitLine(StringBuilder buffer, string data, int start, int end, Action<string>? 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);
 	}
 
 	/// <summary>