From c4bfaeedd56c89fc6c5c148edc9e582a8210a0a8 Mon Sep 17 00:00:00 2001 From: Jeff Robison Date: Wed, 21 Jun 2023 17:46:24 -0700 Subject: [PATCH 01/10] Reduce runtime and allocations from BaseConsoleLogger.IndentString --- src/Build.UnitTests/ConsoleLogger_Tests.cs | 4 +- src/Build/Logging/BaseConsoleLogger.cs | 213 ++++++++++++++++++--- 2 files changed, 193 insertions(+), 24 deletions(-) diff --git a/src/Build.UnitTests/ConsoleLogger_Tests.cs b/src/Build.UnitTests/ConsoleLogger_Tests.cs index 70ed081e9fb..10859bb9ce5 100644 --- a/src/Build.UnitTests/ConsoleLogger_Tests.cs +++ b/src/Build.UnitTests/ConsoleLogger_Tests.cs @@ -1234,7 +1234,7 @@ public void MultilineFormatUnixLineEndings() [Fact] public void MultilineFormatMixedLineEndings() { - string s = "foo" + "\r\n\r\n" + "bar" + "\n" + "baz" + "\n\r\n\n" + + string s = "\n" + "foo" + "\r\n\r\n" + "bar" + "\n" + "baz" + "\n\r\n\n" + "jazz" + "\r\n" + "razz" + "\n\n" + "matazz" + "\n" + "end"; SerialConsoleLogger cl = new SerialConsoleLogger(); @@ -1242,7 +1242,7 @@ public void MultilineFormatMixedLineEndings() string ss = cl.IndentString(s, 0); // should convert lines to system format - ss.ShouldBe($"foo{Environment.NewLine}{Environment.NewLine}bar{Environment.NewLine}baz{Environment.NewLine}{Environment.NewLine}{Environment.NewLine}jazz{Environment.NewLine}razz{Environment.NewLine}{Environment.NewLine}matazz{Environment.NewLine}end{Environment.NewLine}"); + ss.ShouldBe($"{Environment.NewLine}foo{Environment.NewLine}{Environment.NewLine}bar{Environment.NewLine}baz{Environment.NewLine}{Environment.NewLine}{Environment.NewLine}jazz{Environment.NewLine}razz{Environment.NewLine}{Environment.NewLine}matazz{Environment.NewLine}end{Environment.NewLine}"); } [Fact] diff --git a/src/Build/Logging/BaseConsoleLogger.cs b/src/Build/Logging/BaseConsoleLogger.cs index ce386341186..a95e344d72a 100644 --- a/src/Build/Logging/BaseConsoleLogger.cs +++ b/src/Build/Logging/BaseConsoleLogger.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Buffers; using System.Collections; using System.Collections.Generic; using System.Globalization; @@ -130,28 +131,7 @@ public int Compare(Object a, Object b) /// Depth to indent. internal string IndentString(string s, int indent) { - // It's possible the event has a null message - if (s == null) - { - return string.Empty; - } - - // This will never return an empty array. The returned array will always - // have at least one non-null element, even if "s" is totally empty. - String[] subStrings = SplitStringOnNewLines(s); - - StringBuilder result = new StringBuilder( - (subStrings.Length * indent) + - (subStrings.Length * Environment.NewLine.Length) + - s.Length); - - for (int i = 0; i < subStrings.Length; i++) - { - result.Append(' ', indent).Append(subStrings[i]); - result.AppendLine(); - } - - return result.ToString(); + return OptimizedStringIndenter.IndentString(s, indent); } /// @@ -911,6 +891,195 @@ public int Compare(object o1, object o2) } } + /// + /// Helper class to indent all the lines of a potentially multi-line string with + /// minimal CPU and memory overhead. + /// + /// + /// is a functional replacement for the following code: + /// + /// string IndentString(string s, int indent) + /// { + /// string[] newLines = { "\r\n", "\n" }; + /// string[] subStrings = s.Split(newLines, StringSplitOptions.None); + /// + /// StringBuilder result = new StringBuilder( + /// (subStrings.Length * indent) + + /// (subStrings.Length * Environment.NewLine.Length) + + /// s.Length); + /// + /// for (int i = 0; i < subStrings.Length; i++) + /// { + /// result.Append(' ', indent).Append(subStrings[i]); + /// result.AppendLine(); + /// } + /// + /// return result.ToString(); + /// } + /// + /// On net472, benchmarks show that the optimized version runs in about 50-60% of the time + /// and has about 15% of the memory overhead of the code that it replaces. + /// + /// On net7.0 (which has more optimizations than net472), the optimized version runs in + /// about 70-75% of the time and has about 30% of the memory overhead of the code that it + /// replaces. + /// + /// + private static class OptimizedStringIndenter + { +#nullable enable + internal static string IndentString(string? s, int indent) + { + if (s is null) + { + return string.Empty; + } + + string newLine = Environment.NewLine; + + using PooledSpan segments = GetStringSegments(s); + int indentedStringLength = ComputeIndentedStringLength(segments, newLine, indent); + string indented = new string('\0', indentedStringLength); + + unsafe + { +#pragma warning disable SA1519 // Braces should not be omitted from multi-line child statement + fixed (char* pInput = s) + fixed (char* pIndented = indented) + { + char* pSegment = pInput; + char* pOutput = pIndented; + + foreach (var segment in segments) + { + // append indent + for (int i = 0; i < indent; i++) + { + *pOutput++ = ' '; + } + + // append string segment + int byteCount = segment.Length * sizeof(char); + Buffer.MemoryCopy(pSegment, pOutput, byteCount, byteCount); + pOutput += segment.Length; + + // append newLine + for (int i = 0; i < newLine.Length; i++) + { + *pOutput++ = newLine[i]; + } + + // move to next segment + pSegment += segment.TotalLength; + } + } +#pragma warning restore SA1519 // Braces should not be omitted from multi-line child statement + } + + return indented; + + // local method + static int ComputeIndentedStringLength(PooledSpan segments, string newLine, int indent) + { + int indentedLength = segments.Length * (newLine.Length + indent); + + foreach (var segment in segments) + { + indentedLength += segment.Length; + } + + return indentedLength; + } + } + + private static PooledSpan GetStringSegments(string input) + { + if (input.Length == 0) + { + PooledSpan emptyResult = new(1); + emptyResult.Span[0] = new StringSegment(0, 0); + return emptyResult; + } + + int segmentCount = 1; + for (int i = 0; i < input.Length; i++) + { + if (input[i] == '\n') + { + segmentCount++; + } + } + + PooledSpan segments = new(segmentCount); + int start = 0; + int index = 0; + + for (int i = 0; i < segmentCount; i++) + { + while (index < input.Length && input[index] != '\n') + { + index++; + } + + // the input string didn't end with a newline + if (index == input.Length) + { + segments[i] = new StringSegment(index - start, 0); + break; + } + + int newLineLength = 1; + bool endedWithReturnNewline = (index > 0) && (input[index - 1] == '\r'); + + if (endedWithReturnNewline) + { + newLineLength++; + index--; + } + + segments[i] = new StringSegment(index - start, newLineLength); + + start = index += newLineLength; + } + + return segments; + } + + private readonly record struct StringSegment(int Length, int NewLineLength) + { + public int TotalLength => Length + NewLineLength; + } + + private ref struct PooledSpan + { + private static readonly ArrayPool Pool = ArrayPool.Shared; + private readonly T[] _pooledArray; + + public PooledSpan(int length) + { + _pooledArray = Pool.Rent(length); + Array.Clear(_pooledArray, 0, length); + Span = _pooledArray.AsSpan(0, length); + } + + public void Dispose() + { + Pool.Return(_pooledArray); + } + + public Span Span { get; } + public int Length => Span.Length; + public Span.Enumerator GetEnumerator() => Span.GetEnumerator(); + + public T this[int index] + { + get => Span[index]; + set => Span[index] = value; + } + } +#nullable restore + } + #region eventHandlers public virtual void Shutdown() From 7259ca04f4cab49c7104ff15448505c8408af189 Mon Sep 17 00:00:00 2001 From: Jeff Robison Date: Mon, 26 Jun 2023 13:35:43 -0700 Subject: [PATCH 02/10] Prefer class over record for reduced code gen --- src/Build/Logging/BaseConsoleLogger.cs | 195 +++++++++++++------------ 1 file changed, 105 insertions(+), 90 deletions(-) diff --git a/src/Build/Logging/BaseConsoleLogger.cs b/src/Build/Logging/BaseConsoleLogger.cs index a95e344d72a..2c128f0cc66 100644 --- a/src/Build/Logging/BaseConsoleLogger.cs +++ b/src/Build/Logging/BaseConsoleLogger.cs @@ -8,7 +8,9 @@ using System.Globalization; using System.IO; using System.Linq; +using System.Runtime.CompilerServices; using System.Text; +using System.Threading; using Microsoft.Build.Evaluation; using Microsoft.Build.Framework; using Microsoft.Build.Internal; @@ -920,85 +922,83 @@ public int Compare(object o1, object o2) /// On net472, benchmarks show that the optimized version runs in about 50-60% of the time /// and has about 15% of the memory overhead of the code that it replaces. /// - /// On net7.0 (which has more optimizations than net472), the optimized version runs in - /// about 70-75% of the time and has about 30% of the memory overhead of the code that it - /// replaces. + /// On net7.0, the optimized version runs in about 45-55% of the time and has about 30% + /// of the memory overhead of the code that it replaces. /// /// private static class OptimizedStringIndenter { #nullable enable - internal static string IndentString(string? s, int indent) +#if NET7_0_OR_GREATER + [SkipLocalsInit] +#endif + internal static unsafe string IndentString(string? s, int indent) { if (s is null) { return string.Empty; } - string newLine = Environment.NewLine; + Span segments = GetStringSegments(s.AsSpan(), stackalloc StringSegment[128], out StringSegment[]? pooledArray); - using PooledSpan segments = GetStringSegments(s); - int indentedStringLength = ComputeIndentedStringLength(segments, newLine, indent); - string indented = new string('\0', indentedStringLength); + int indentedStringLength = segments.Length * (Environment.NewLine.Length + indent); + foreach (StringSegment segment in segments) + { + indentedStringLength += segment.Length; + } - unsafe +#if NET7_0_OR_GREATER +#pragma warning disable CS8500 + string result = string.Create(indentedStringLength, (s, (IntPtr)(&segments), indent), static (output, state) => { -#pragma warning disable SA1519 // Braces should not be omitted from multi-line child statement - fixed (char* pInput = s) - fixed (char* pIndented = indented) + ReadOnlySpan input = state.s; + foreach (StringSegment segment in *(Span*)state.Item2) { - char* pSegment = pInput; - char* pOutput = pIndented; - - foreach (var segment in segments) - { - // append indent - for (int i = 0; i < indent; i++) - { - *pOutput++ = ' '; - } - - // append string segment - int byteCount = segment.Length * sizeof(char); - Buffer.MemoryCopy(pSegment, pOutput, byteCount, byteCount); - pOutput += segment.Length; - - // append newLine - for (int i = 0; i < newLine.Length; i++) - { - *pOutput++ = newLine[i]; - } - - // move to next segment - pSegment += segment.TotalLength; - } + // Append indent + output.Slice(0, state.indent).Fill(' '); + output = output.Slice(state.indent); + + // Append string segment + input.Slice(0, segment.Length).CopyTo(output); + input = input.Slice(segment.TotalLength); + output = output.Slice(segment.Length); + + // Append newline + Environment.NewLine.CopyTo(output); + output = output.Slice(Environment.NewLine.Length); } -#pragma warning restore SA1519 // Braces should not be omitted from multi-line child statement - } - - return indented; + }); +#pragma warning restore CS8500 +#else + using RentedBuilder rental = RentBuilder(indentedStringLength); - // local method - static int ComputeIndentedStringLength(PooledSpan segments, string newLine, int indent) + foreach (StringSegment segment in segments) { - int indentedLength = segments.Length * (newLine.Length + indent); + rental.Builder + .Append(' ', indent) + .Append(s, segment.Start, segment.Length) + .AppendLine(); + } - foreach (var segment in segments) - { - indentedLength += segment.Length; - } + string result = rental.Builder.ToString(); +#endif - return indentedLength; + if (pooledArray is not null) + { + ArrayPool.Shared.Return(pooledArray); } + + return result; } - private static PooledSpan GetStringSegments(string input) + private static Span GetStringSegments(ReadOnlySpan input, Span segments, out StringSegment[]? pooledArray) { - if (input.Length == 0) + if (input.IsEmpty) { - PooledSpan emptyResult = new(1); - emptyResult.Span[0] = new StringSegment(0, 0); - return emptyResult; + segments = segments.Slice(0, 1); + segments[0] = new StringSegment(0, 0, 0); + pooledArray = null; + return segments; } int segmentCount = 1; @@ -1010,73 +1010,88 @@ private static PooledSpan GetStringSegments(string input) } } - PooledSpan segments = new(segmentCount); - int start = 0; - int index = 0; - - for (int i = 0; i < segmentCount; i++) + if (segmentCount <= segments.Length) { - while (index < input.Length && input[index] != '\n') - { - index++; - } + pooledArray = null; + segments = segments.Slice(0, segmentCount); + } + else + { + pooledArray = ArrayPool.Shared.Rent(segmentCount); + segments = pooledArray.AsSpan(0, segmentCount); + } - // the input string didn't end with a newline - if (index == input.Length) + int start = 0; + for (int i = 0; i < segments.Length; i++) + { + int index = input.IndexOf('\n'); + if (index < 0) { - segments[i] = new StringSegment(index - start, 0); + segments[i] = new StringSegment(start, input.Length, 0); break; } int newLineLength = 1; - bool endedWithReturnNewline = (index > 0) && (input[index - 1] == '\r'); - - if (endedWithReturnNewline) + if (index > 0 && input[index - 1] == '\r') { newLineLength++; index--; } - segments[i] = new StringSegment(index - start, newLineLength); + int totalLength = index + newLineLength; + segments[i] = new StringSegment(start, index, totalLength); - start = index += newLineLength; + start += totalLength; + input = input.Slice(totalLength); } return segments; } - private readonly record struct StringSegment(int Length, int NewLineLength) + private struct StringSegment { - public int TotalLength => Length + NewLineLength; + public StringSegment(int start, int length, int totalLength) + { + Start = start; + Length = length; + TotalLength = totalLength; + } + + public int Start { get; } + public int Length { get; } + public int TotalLength { get; } } - private ref struct PooledSpan +#if !NET7_0_OR_GREATER + private static RentedBuilder RentBuilder(int capacity) => new RentedBuilder(capacity); + + private ref struct RentedBuilder { - private static readonly ArrayPool Pool = ArrayPool.Shared; - private readonly T[] _pooledArray; + // The maximum capacity for a StringBuilder that we'll cache. StringBuilders with + // larger capacities will be allowed to be GC'd. + private const int MaxStringBuilderCapacity = 512; - public PooledSpan(int length) + private static StringBuilder? _cachedBuilder; + + public RentedBuilder(int capacity) { - _pooledArray = Pool.Rent(length); - Array.Clear(_pooledArray, 0, length); - Span = _pooledArray.AsSpan(0, length); + Builder = Interlocked.Exchange(ref _cachedBuilder, null) ?? new StringBuilder(capacity); + Builder.EnsureCapacity(capacity); } public void Dispose() { - Pool.Return(_pooledArray); + // if builder's capacity is within our limits, return it to the cache + if (Builder.Capacity <= MaxStringBuilderCapacity) + { + Builder.Clear(); + Interlocked.Exchange(ref _cachedBuilder, Builder); + } } - public Span Span { get; } - public int Length => Span.Length; - public Span.Enumerator GetEnumerator() => Span.GetEnumerator(); - - public T this[int index] - { - get => Span[index]; - set => Span[index] = value; - } + public StringBuilder Builder { get; } } +#endif #nullable restore } From 2f39b0840e156576ba73af6d0b103c68024c197d Mon Sep 17 00:00:00 2001 From: Jeff Robison Date: Fri, 30 Jun 2023 09:25:40 -0700 Subject: [PATCH 03/10] Disable nullable warnings after OptimizedStringIndenter --- src/Build/Logging/BaseConsoleLogger.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Build/Logging/BaseConsoleLogger.cs b/src/Build/Logging/BaseConsoleLogger.cs index 2c128f0cc66..452ba0f2d3e 100644 --- a/src/Build/Logging/BaseConsoleLogger.cs +++ b/src/Build/Logging/BaseConsoleLogger.cs @@ -19,6 +19,7 @@ using ColorSetter = Microsoft.Build.Logging.ColorSetter; using WriteHandler = Microsoft.Build.Logging.WriteHandler; +// if this is removed, also remove the "#nullable disable" in OptimizedStringIndenter #nullable disable namespace Microsoft.Build.BackEnd.Logging @@ -1092,7 +1093,7 @@ public void Dispose() public StringBuilder Builder { get; } } #endif -#nullable restore +#nullable disable } #region eventHandlers From c7bef4b524ef128692e4354397de6f5a30974497 Mon Sep 17 00:00:00 2001 From: Jeff Robison Date: Fri, 30 Jun 2023 10:38:33 -0700 Subject: [PATCH 04/10] Fix PR build validation --- src/Build/Logging/BaseConsoleLogger.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Build/Logging/BaseConsoleLogger.cs b/src/Build/Logging/BaseConsoleLogger.cs index 452ba0f2d3e..262ce028fdc 100644 --- a/src/Build/Logging/BaseConsoleLogger.cs +++ b/src/Build/Logging/BaseConsoleLogger.cs @@ -8,7 +8,9 @@ using System.Globalization; using System.IO; using System.Linq; +#if NET7_0_OR_GREATER using System.Runtime.CompilerServices; +#endif using System.Text; using System.Threading; using Microsoft.Build.Evaluation; From eae9b9fecf7531affb28b04ee4f9826deff4fc1e Mon Sep 17 00:00:00 2001 From: Rainer Sigwald Date: Fri, 30 Jun 2023 13:48:03 -0500 Subject: [PATCH 05/10] Separate file --- src/Build/Logging/BaseConsoleLogger.cs | 208 ------------------ src/Build/Logging/OptimizedStringIndenter.cs | 214 +++++++++++++++++++ src/Build/Microsoft.Build.csproj | 1 + 3 files changed, 215 insertions(+), 208 deletions(-) create mode 100644 src/Build/Logging/OptimizedStringIndenter.cs diff --git a/src/Build/Logging/BaseConsoleLogger.cs b/src/Build/Logging/BaseConsoleLogger.cs index 262ce028fdc..d0863f48af2 100644 --- a/src/Build/Logging/BaseConsoleLogger.cs +++ b/src/Build/Logging/BaseConsoleLogger.cs @@ -2,17 +2,12 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Buffers; using System.Collections; using System.Collections.Generic; using System.Globalization; using System.IO; using System.Linq; -#if NET7_0_OR_GREATER -using System.Runtime.CompilerServices; -#endif using System.Text; -using System.Threading; using Microsoft.Build.Evaluation; using Microsoft.Build.Framework; using Microsoft.Build.Internal; @@ -21,7 +16,6 @@ using ColorSetter = Microsoft.Build.Logging.ColorSetter; using WriteHandler = Microsoft.Build.Logging.WriteHandler; -// if this is removed, also remove the "#nullable disable" in OptimizedStringIndenter #nullable disable namespace Microsoft.Build.BackEnd.Logging @@ -896,208 +890,6 @@ public int Compare(object o1, object o2) } } - /// - /// Helper class to indent all the lines of a potentially multi-line string with - /// minimal CPU and memory overhead. - /// - /// - /// is a functional replacement for the following code: - /// - /// string IndentString(string s, int indent) - /// { - /// string[] newLines = { "\r\n", "\n" }; - /// string[] subStrings = s.Split(newLines, StringSplitOptions.None); - /// - /// StringBuilder result = new StringBuilder( - /// (subStrings.Length * indent) + - /// (subStrings.Length * Environment.NewLine.Length) + - /// s.Length); - /// - /// for (int i = 0; i < subStrings.Length; i++) - /// { - /// result.Append(' ', indent).Append(subStrings[i]); - /// result.AppendLine(); - /// } - /// - /// return result.ToString(); - /// } - /// - /// On net472, benchmarks show that the optimized version runs in about 50-60% of the time - /// and has about 15% of the memory overhead of the code that it replaces. - /// - /// On net7.0, the optimized version runs in about 45-55% of the time and has about 30% - /// of the memory overhead of the code that it replaces. - /// - /// - private static class OptimizedStringIndenter - { -#nullable enable -#if NET7_0_OR_GREATER - [SkipLocalsInit] -#endif - internal static unsafe string IndentString(string? s, int indent) - { - if (s is null) - { - return string.Empty; - } - - Span segments = GetStringSegments(s.AsSpan(), stackalloc StringSegment[128], out StringSegment[]? pooledArray); - - int indentedStringLength = segments.Length * (Environment.NewLine.Length + indent); - foreach (StringSegment segment in segments) - { - indentedStringLength += segment.Length; - } - -#if NET7_0_OR_GREATER -#pragma warning disable CS8500 - string result = string.Create(indentedStringLength, (s, (IntPtr)(&segments), indent), static (output, state) => - { - ReadOnlySpan input = state.s; - foreach (StringSegment segment in *(Span*)state.Item2) - { - // Append indent - output.Slice(0, state.indent).Fill(' '); - output = output.Slice(state.indent); - - // Append string segment - input.Slice(0, segment.Length).CopyTo(output); - input = input.Slice(segment.TotalLength); - output = output.Slice(segment.Length); - - // Append newline - Environment.NewLine.CopyTo(output); - output = output.Slice(Environment.NewLine.Length); - } - }); -#pragma warning restore CS8500 -#else - using RentedBuilder rental = RentBuilder(indentedStringLength); - - foreach (StringSegment segment in segments) - { - rental.Builder - .Append(' ', indent) - .Append(s, segment.Start, segment.Length) - .AppendLine(); - } - - string result = rental.Builder.ToString(); -#endif - - if (pooledArray is not null) - { - ArrayPool.Shared.Return(pooledArray); - } - - return result; - } - - private static Span GetStringSegments(ReadOnlySpan input, Span segments, out StringSegment[]? pooledArray) - { - if (input.IsEmpty) - { - segments = segments.Slice(0, 1); - segments[0] = new StringSegment(0, 0, 0); - pooledArray = null; - return segments; - } - - int segmentCount = 1; - for (int i = 0; i < input.Length; i++) - { - if (input[i] == '\n') - { - segmentCount++; - } - } - - if (segmentCount <= segments.Length) - { - pooledArray = null; - segments = segments.Slice(0, segmentCount); - } - else - { - pooledArray = ArrayPool.Shared.Rent(segmentCount); - segments = pooledArray.AsSpan(0, segmentCount); - } - - int start = 0; - for (int i = 0; i < segments.Length; i++) - { - int index = input.IndexOf('\n'); - if (index < 0) - { - segments[i] = new StringSegment(start, input.Length, 0); - break; - } - - int newLineLength = 1; - if (index > 0 && input[index - 1] == '\r') - { - newLineLength++; - index--; - } - - int totalLength = index + newLineLength; - segments[i] = new StringSegment(start, index, totalLength); - - start += totalLength; - input = input.Slice(totalLength); - } - - return segments; - } - - private struct StringSegment - { - public StringSegment(int start, int length, int totalLength) - { - Start = start; - Length = length; - TotalLength = totalLength; - } - - public int Start { get; } - public int Length { get; } - public int TotalLength { get; } - } - -#if !NET7_0_OR_GREATER - private static RentedBuilder RentBuilder(int capacity) => new RentedBuilder(capacity); - - private ref struct RentedBuilder - { - // The maximum capacity for a StringBuilder that we'll cache. StringBuilders with - // larger capacities will be allowed to be GC'd. - private const int MaxStringBuilderCapacity = 512; - - private static StringBuilder? _cachedBuilder; - - public RentedBuilder(int capacity) - { - Builder = Interlocked.Exchange(ref _cachedBuilder, null) ?? new StringBuilder(capacity); - Builder.EnsureCapacity(capacity); - } - - public void Dispose() - { - // if builder's capacity is within our limits, return it to the cache - if (Builder.Capacity <= MaxStringBuilderCapacity) - { - Builder.Clear(); - Interlocked.Exchange(ref _cachedBuilder, Builder); - } - } - - public StringBuilder Builder { get; } - } -#endif -#nullable disable - } - #region eventHandlers public virtual void Shutdown() diff --git a/src/Build/Logging/OptimizedStringIndenter.cs b/src/Build/Logging/OptimizedStringIndenter.cs new file mode 100644 index 00000000000..b444465ee82 --- /dev/null +++ b/src/Build/Logging/OptimizedStringIndenter.cs @@ -0,0 +1,214 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Buffers; +#if NET7_0_OR_GREATER +using System.Runtime.CompilerServices; +#else +using System.Text; +using System.Threading; +#endif + +namespace Microsoft.Build.BackEnd.Logging; + +/// +/// Helper class to indent all the lines of a potentially multi-line string with +/// minimal CPU and memory overhead. +/// +/// +/// is a functional replacement for the following code: +/// +/// string IndentString(string s, int indent) +/// { +/// string[] newLines = { "\r\n", "\n" }; +/// string[] subStrings = s.Split(newLines, StringSplitOptions.None); +/// +/// StringBuilder result = new StringBuilder( +/// (subStrings.Length * indent) + +/// (subStrings.Length * Environment.NewLine.Length) + +/// s.Length); +/// +/// for (int i = 0; i < subStrings.Length; i++) +/// { +/// result.Append(' ', indent).Append(subStrings[i]); +/// result.AppendLine(); +/// } +/// +/// return result.ToString(); +/// } +/// +/// On net472, benchmarks show that the optimized version runs in about 50-60% of the time +/// and has about 15% of the memory overhead of the code that it replaces. +/// +/// On net7.0, the optimized version runs in about 45-55% of the time and has about 30% +/// of the memory overhead of the code that it replaces. +/// +/// +internal static class OptimizedStringIndenter +{ +#nullable enable +#if NET7_0_OR_GREATER + [SkipLocalsInit] +#endif + internal static unsafe string IndentString(string? s, int indent) + { + if (s is null) + { + return string.Empty; + } + + Span segments = GetStringSegments(s.AsSpan(), stackalloc StringSegment[128], out StringSegment[]? pooledArray); + + int indentedStringLength = segments.Length * (Environment.NewLine.Length + indent); + foreach (StringSegment segment in segments) + { + indentedStringLength += segment.Length; + } + +#if NET7_0_OR_GREATER +#pragma warning disable CS8500 + string result = string.Create(indentedStringLength, (s, (IntPtr)(&segments), indent), static (output, state) => + { + ReadOnlySpan input = state.s; + foreach (StringSegment segment in *(Span*)state.Item2) + { + // Append indent + output.Slice(0, state.indent).Fill(' '); + output = output.Slice(state.indent); + + // Append string segment + input.Slice(0, segment.Length).CopyTo(output); + input = input.Slice(segment.TotalLength); + output = output.Slice(segment.Length); + + // Append newline + Environment.NewLine.CopyTo(output); + output = output.Slice(Environment.NewLine.Length); + } + }); +#pragma warning restore CS8500 +#else + using RentedBuilder rental = RentBuilder(indentedStringLength); + + foreach (StringSegment segment in segments) + { + rental.Builder + .Append(' ', indent) + .Append(s, segment.Start, segment.Length) + .AppendLine(); + } + + string result = rental.Builder.ToString(); +#endif + + if (pooledArray is not null) + { + ArrayPool.Shared.Return(pooledArray); + } + + return result; + } + + private static Span GetStringSegments(ReadOnlySpan input, Span segments, out StringSegment[]? pooledArray) + { + if (input.IsEmpty) + { + segments = segments.Slice(0, 1); + segments[0] = new StringSegment(0, 0, 0); + pooledArray = null; + return segments; + } + + int segmentCount = 1; + for (int i = 0; i < input.Length; i++) + { + if (input[i] == '\n') + { + segmentCount++; + } + } + + if (segmentCount <= segments.Length) + { + pooledArray = null; + segments = segments.Slice(0, segmentCount); + } + else + { + pooledArray = ArrayPool.Shared.Rent(segmentCount); + segments = pooledArray.AsSpan(0, segmentCount); + } + + int start = 0; + for (int i = 0; i < segments.Length; i++) + { + int index = input.IndexOf('\n'); + if (index < 0) + { + segments[i] = new StringSegment(start, input.Length, 0); + break; + } + + int newLineLength = 1; + if (index > 0 && input[index - 1] == '\r') + { + newLineLength++; + index--; + } + + int totalLength = index + newLineLength; + segments[i] = new StringSegment(start, index, totalLength); + + start += totalLength; + input = input.Slice(totalLength); + } + + return segments; + } + + private struct StringSegment + { + public StringSegment(int start, int length, int totalLength) + { + Start = start; + Length = length; + TotalLength = totalLength; + } + + public int Start { get; } + public int Length { get; } + public int TotalLength { get; } + } + +#if !NET7_0_OR_GREATER + private static RentedBuilder RentBuilder(int capacity) => new RentedBuilder(capacity); + + private ref struct RentedBuilder + { + // The maximum capacity for a StringBuilder that we'll cache. StringBuilders with + // larger capacities will be allowed to be GC'd. + private const int MaxStringBuilderCapacity = 512; + + private static StringBuilder? _cachedBuilder; + + public RentedBuilder(int capacity) + { + Builder = Interlocked.Exchange(ref _cachedBuilder, null) ?? new StringBuilder(capacity); + Builder.EnsureCapacity(capacity); + } + + public void Dispose() + { + // if builder's capacity is within our limits, return it to the cache + if (Builder.Capacity <= MaxStringBuilderCapacity) + { + Builder.Clear(); + Interlocked.Exchange(ref _cachedBuilder, Builder); + } + } + + public StringBuilder Builder { get; } + } +#endif +} diff --git a/src/Build/Microsoft.Build.csproj b/src/Build/Microsoft.Build.csproj index 1bc5e67a847..44bf8651ae7 100644 --- a/src/Build/Microsoft.Build.csproj +++ b/src/Build/Microsoft.Build.csproj @@ -617,6 +617,7 @@ true + true From 72733aa9751828ac4e8b7d2fb407139156ebf8b3 Mon Sep 17 00:00:00 2001 From: Rainer Sigwald Date: Fri, 30 Jun 2023 13:51:50 -0500 Subject: [PATCH 06/10] Use StringBuilderCache --- src/Build/Logging/OptimizedStringIndenter.cs | 40 ++------------------ 1 file changed, 3 insertions(+), 37 deletions(-) diff --git a/src/Build/Logging/OptimizedStringIndenter.cs b/src/Build/Logging/OptimizedStringIndenter.cs index b444465ee82..a01aa401b20 100644 --- a/src/Build/Logging/OptimizedStringIndenter.cs +++ b/src/Build/Logging/OptimizedStringIndenter.cs @@ -5,9 +5,6 @@ using System.Buffers; #if NET7_0_OR_GREATER using System.Runtime.CompilerServices; -#else -using System.Text; -using System.Threading; #endif namespace Microsoft.Build.BackEnd.Logging; @@ -89,17 +86,17 @@ internal static unsafe string IndentString(string? s, int indent) }); #pragma warning restore CS8500 #else - using RentedBuilder rental = RentBuilder(indentedStringLength); + StringBuilder builder = StringBuilderCache.Acquire(indentedStringLength); foreach (StringSegment segment in segments) { - rental.Builder + builder .Append(' ', indent) .Append(s, segment.Start, segment.Length) .AppendLine(); } - string result = rental.Builder.ToString(); + string result = StringBuilderCache.GetStringAndRelease(builder); #endif if (pooledArray is not null) @@ -180,35 +177,4 @@ public StringSegment(int start, int length, int totalLength) public int Length { get; } public int TotalLength { get; } } - -#if !NET7_0_OR_GREATER - private static RentedBuilder RentBuilder(int capacity) => new RentedBuilder(capacity); - - private ref struct RentedBuilder - { - // The maximum capacity for a StringBuilder that we'll cache. StringBuilders with - // larger capacities will be allowed to be GC'd. - private const int MaxStringBuilderCapacity = 512; - - private static StringBuilder? _cachedBuilder; - - public RentedBuilder(int capacity) - { - Builder = Interlocked.Exchange(ref _cachedBuilder, null) ?? new StringBuilder(capacity); - Builder.EnsureCapacity(capacity); - } - - public void Dispose() - { - // if builder's capacity is within our limits, return it to the cache - if (Builder.Capacity <= MaxStringBuilderCapacity) - { - Builder.Clear(); - Interlocked.Exchange(ref _cachedBuilder, Builder); - } - } - - public StringBuilder Builder { get; } - } -#endif } From 914d51b57e36a8cf408e16945845cd5609019403 Mon Sep 17 00:00:00 2001 From: Jeff Robison Date: Fri, 30 Jun 2023 13:09:46 -0700 Subject: [PATCH 07/10] Fix build breaks --- src/Build/Logging/OptimizedStringIndenter.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Build/Logging/OptimizedStringIndenter.cs b/src/Build/Logging/OptimizedStringIndenter.cs index a01aa401b20..3431e5710e8 100644 --- a/src/Build/Logging/OptimizedStringIndenter.cs +++ b/src/Build/Logging/OptimizedStringIndenter.cs @@ -3,8 +3,12 @@ using System; using System.Buffers; + #if NET7_0_OR_GREATER using System.Runtime.CompilerServices; +#else +using System.Text; +using Microsoft.Build.Framework; #endif namespace Microsoft.Build.BackEnd.Logging; From ebd1f1eb04f13fb3f1fcac6e0e71062789a2f58d Mon Sep 17 00:00:00 2001 From: Roman Konecny Date: Fri, 14 Jul 2023 10:29:32 +0200 Subject: [PATCH 08/10] Remove total length from segments --- src/Build/Logging/OptimizedStringIndenter.cs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/Build/Logging/OptimizedStringIndenter.cs b/src/Build/Logging/OptimizedStringIndenter.cs index 3431e5710e8..d7ce739f3de 100644 --- a/src/Build/Logging/OptimizedStringIndenter.cs +++ b/src/Build/Logging/OptimizedStringIndenter.cs @@ -79,8 +79,7 @@ internal static unsafe string IndentString(string? s, int indent) output = output.Slice(state.indent); // Append string segment - input.Slice(0, segment.Length).CopyTo(output); - input = input.Slice(segment.TotalLength); + input.Slice(segment.Start, segment.Length).CopyTo(output); output = output.Slice(segment.Length); // Append newline @@ -116,7 +115,7 @@ private static Span GetStringSegments(ReadOnlySpan input, S if (input.IsEmpty) { segments = segments.Slice(0, 1); - segments[0] = new StringSegment(0, 0, 0); + segments[0] = new StringSegment(0, 0); pooledArray = null; return segments; } @@ -147,7 +146,7 @@ private static Span GetStringSegments(ReadOnlySpan input, S int index = input.IndexOf('\n'); if (index < 0) { - segments[i] = new StringSegment(start, input.Length, 0); + segments[i] = new StringSegment(start, input.Length); break; } @@ -159,7 +158,7 @@ private static Span GetStringSegments(ReadOnlySpan input, S } int totalLength = index + newLineLength; - segments[i] = new StringSegment(start, index, totalLength); + segments[i] = new StringSegment(start, index); start += totalLength; input = input.Slice(totalLength); @@ -170,15 +169,13 @@ private static Span GetStringSegments(ReadOnlySpan input, S private struct StringSegment { - public StringSegment(int start, int length, int totalLength) + public StringSegment(int start, int length) { Start = start; Length = length; - TotalLength = totalLength; } public int Start { get; } public int Length { get; } - public int TotalLength { get; } } } From 9a3657f8696d7b871a10228acd0e6894054ef954 Mon Sep 17 00:00:00 2001 From: Roman Konecny Date: Fri, 14 Jul 2023 15:24:13 +0200 Subject: [PATCH 09/10] Share string builder in console loggers. --- .../ConsoleOutputAlignerTests.cs | 48 ++++++----- src/Build/Logging/BaseConsoleLogger.cs | 82 ++++++++++++++++++- src/Build/Logging/OptimizedStringIndenter.cs | 8 +- .../ParallelLogger/ConsoleOutputAligner.cs | 18 ++-- .../ParallelLogger/ParallelConsoleLogger.cs | 2 +- src/Framework/IStringBuilderProvider.cs | 38 +++++++++ 6 files changed, 162 insertions(+), 34 deletions(-) create mode 100644 src/Framework/IStringBuilderProvider.cs diff --git a/src/Build.UnitTests/ConsoleOutputAlignerTests.cs b/src/Build.UnitTests/ConsoleOutputAlignerTests.cs index 87dc12b963d..896837b46fd 100644 --- a/src/Build.UnitTests/ConsoleOutputAlignerTests.cs +++ b/src/Build.UnitTests/ConsoleOutputAlignerTests.cs @@ -2,7 +2,9 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Text; using Microsoft.Build.BackEnd.Logging; +using Microsoft.Build.Framework; using Shouldly; using Xunit; @@ -18,7 +20,7 @@ public class ConsoleOutputAlignerTests public void IndentBiggerThanBuffer_IndentedAndNotAligned(string input, bool aligned) { string indent = " "; - var aligner = new ConsoleOutputAligner(bufferWidth: 4, alignMessages: aligned); + var aligner = new ConsoleOutputAligner(bufferWidth: 4, alignMessages: aligned, stringBuilderProvider: new TestStringBuilderProvider()); string output = aligner.AlignConsoleOutput(message: input, prefixAlreadyWritten: false, prefixWidth: indent.Length); @@ -30,7 +32,7 @@ public void IndentBiggerThanBuffer_IndentedAndNotAligned(string input, bool alig [InlineData("12345")] public void NoAlignNoIndent_NotAlignedEvenIfBiggerThanBuffer(string input) { - var aligner = new ConsoleOutputAligner(bufferWidth: 4, alignMessages: false); + var aligner = new ConsoleOutputAligner(bufferWidth: 4, alignMessages: false, stringBuilderProvider: new TestStringBuilderProvider()); string output = aligner.AlignConsoleOutput(message: input, prefixAlreadyWritten: false, prefixWidth: 0); @@ -43,7 +45,7 @@ public void NoAlignNoIndent_NotAlignedEvenIfBiggerThanBuffer(string input) public void NoBufferWidthNoIndent_NotAligned(int sizeOfMessage) { string input = new string('.', sizeOfMessage); - var aligner = new ConsoleOutputAligner(bufferWidth: -1, alignMessages: false); + var aligner = new ConsoleOutputAligner(bufferWidth: -1, alignMessages: false, stringBuilderProvider: new TestStringBuilderProvider()); string output = aligner.AlignConsoleOutput(message: input, prefixAlreadyWritten: false, prefixWidth: 0); @@ -55,7 +57,7 @@ public void NoBufferWidthNoIndent_NotAligned(int sizeOfMessage) [InlineData("12345")] public void WithoutBufferWidthWithoutIndentWithAlign_NotIndentedAndNotAligned(string input) { - var aligner = new ConsoleOutputAligner(bufferWidth: -1, alignMessages: true); + var aligner = new ConsoleOutputAligner(bufferWidth: -1, alignMessages: true, stringBuilderProvider: new TestStringBuilderProvider()); string output = aligner.AlignConsoleOutput(message: input, prefixAlreadyWritten: false, prefixWidth: 0); @@ -67,7 +69,7 @@ public void WithoutBufferWidthWithoutIndentWithAlign_NotIndentedAndNotAligned(st [InlineData("12345")] public void NoAlignPrefixAlreadyWritten_NotChanged(string input) { - var aligner = new ConsoleOutputAligner(bufferWidth: 10, alignMessages: true); + var aligner = new ConsoleOutputAligner(bufferWidth: 10, alignMessages: true, stringBuilderProvider: new TestStringBuilderProvider()); string output = aligner.AlignConsoleOutput(message: input, prefixAlreadyWritten: true, prefixWidth: 0); @@ -80,7 +82,7 @@ public void NoAlignPrefixAlreadyWritten_NotChanged(string input) [InlineData(" ", "1")] public void SmallerThanBuffer_NotAligned(string indent, string input) { - var aligner = new ConsoleOutputAligner(bufferWidth: 4, alignMessages: true); + var aligner = new ConsoleOutputAligner(bufferWidth: 4, alignMessages: true, stringBuilderProvider: new TestStringBuilderProvider()); string output = aligner.AlignConsoleOutput(message: input, prefixAlreadyWritten: false, prefixWidth: indent.Length); @@ -93,7 +95,7 @@ public void SmallerThanBuffer_NotAligned(string indent, string input) [InlineData(" ", "12", " 1", " 2")] public void BiggerThanBuffer_AlignedWithIndent(string indent, string input, string expected1stLine, string expected2ndLine) { - var aligner = new ConsoleOutputAligner(bufferWidth: 4, alignMessages: true); + var aligner = new ConsoleOutputAligner(bufferWidth: 4, alignMessages: true, stringBuilderProvider: new TestStringBuilderProvider()); string output = aligner.AlignConsoleOutput(message: input, prefixAlreadyWritten: false, prefixWidth: indent.Length); @@ -114,7 +116,7 @@ public void BiggerThanBuffer_AlignedWithIndent(string indent, string input, stri " 4\n")] public void XTimesBiggerThanBuffer_AlignedToMultipleLines(string indent, string input, string expected) { - var aligner = new ConsoleOutputAligner(bufferWidth: 4, alignMessages: true); + var aligner = new ConsoleOutputAligner(bufferWidth: 4, alignMessages: true, stringBuilderProvider: new TestStringBuilderProvider()); string output = aligner.AlignConsoleOutput(message: input, prefixAlreadyWritten: false, prefixWidth: indent.Length); @@ -128,7 +130,7 @@ public void XTimesBiggerThanBuffer_AlignedToMultipleLines(string indent, string [InlineData(" ", "12", "1", " 2")] public void BiggerThanBufferWithPrefixAlreadyWritten_AlignedWithIndentFromSecondLine(string indent, string input, string expected1stLine, string expected2ndLine) { - var aligner = new ConsoleOutputAligner(bufferWidth: 4, alignMessages: true); + var aligner = new ConsoleOutputAligner(bufferWidth: 4, alignMessages: true, stringBuilderProvider: new TestStringBuilderProvider()); string output = aligner.AlignConsoleOutput(message: input, prefixAlreadyWritten: true, prefixWidth: indent.Length); @@ -142,7 +144,7 @@ public void BiggerThanBufferWithPrefixAlreadyWritten_AlignedWithIndentFromSecond public void MultiLineWithoutAlign_NotChanged(string input) { input = input.Replace("\n", Environment.NewLine); - var aligner = new ConsoleOutputAligner(bufferWidth: 4, alignMessages: false); + var aligner = new ConsoleOutputAligner(bufferWidth: 4, alignMessages: false, stringBuilderProvider: new TestStringBuilderProvider()); string output = aligner.AlignConsoleOutput(message: input, prefixAlreadyWritten: true, prefixWidth: 0); @@ -165,7 +167,7 @@ public void NonStandardNewLines_AlignAsExpected(string input, string expected) { expected = expected.Replace("\n", Environment.NewLine) + Environment.NewLine; - var aligner = new ConsoleOutputAligner(bufferWidth: 10, alignMessages: true); + var aligner = new ConsoleOutputAligner(bufferWidth: 10, alignMessages: true, stringBuilderProvider: new TestStringBuilderProvider()); string output = aligner.AlignConsoleOutput(message: input, prefixAlreadyWritten: true, prefixWidth: 2); @@ -179,7 +181,7 @@ public void NonStandardNewLines_AlignAsExpected(string input, string expected) public void ShortMultiLineWithAlign_NoChange(string input) { input = input.Replace("\n", Environment.NewLine); - var aligner = new ConsoleOutputAligner(bufferWidth: 10, alignMessages: true); + var aligner = new ConsoleOutputAligner(bufferWidth: 10, alignMessages: true, stringBuilderProvider: new TestStringBuilderProvider()); string output = aligner.AlignConsoleOutput(message: input, prefixAlreadyWritten: true, prefixWidth: 0); @@ -202,7 +204,7 @@ public void ShortMultiLineWithAlign_NoChange(string input) public void ShortMultiLineWithMixedNewLines_NewLinesReplacedByActualEnvironmentNewLines(string input) { string expected = input.Replace("\r", "").Replace("\n", Environment.NewLine) + Environment.NewLine; - var aligner = new ConsoleOutputAligner(bufferWidth: 10, alignMessages: true); + var aligner = new ConsoleOutputAligner(bufferWidth: 10, alignMessages: true, stringBuilderProvider: new TestStringBuilderProvider()); string output = aligner.AlignConsoleOutput(message: input, prefixAlreadyWritten: true, prefixWidth: 0); @@ -217,7 +219,7 @@ public void MultiLineWithPrefixAlreadyWritten(string prefix, string input, strin { input = input.Replace("\n", Environment.NewLine); expected = expected.Replace("\n", Environment.NewLine); - var aligner = new ConsoleOutputAligner(bufferWidth: 4, alignMessages: true); + var aligner = new ConsoleOutputAligner(bufferWidth: 4, alignMessages: true, stringBuilderProvider: new TestStringBuilderProvider()); string output = aligner.AlignConsoleOutput(message: input, prefixAlreadyWritten: true, prefixWidth: prefix.Length); @@ -231,7 +233,7 @@ public void MultiLineWithoutPrefixAlreadyWritten(string prefix, string input, st { input = input.Replace("\n", Environment.NewLine); expected = expected.Replace("\n", Environment.NewLine); - var aligner = new ConsoleOutputAligner(bufferWidth: 4, alignMessages: true); + var aligner = new ConsoleOutputAligner(bufferWidth: 4, alignMessages: true, stringBuilderProvider: new TestStringBuilderProvider()); string output = aligner.AlignConsoleOutput(message: input, prefixAlreadyWritten: false, prefixWidth: prefix.Length); @@ -244,7 +246,7 @@ public void MultiLineWithoutPrefixAlreadyWritten(string prefix, string input, st public void ShortTextWithTabs_NoChange(string input) { input = input.Replace("\n", Environment.NewLine); - var aligner = new ConsoleOutputAligner(bufferWidth: 50, alignMessages: true); + var aligner = new ConsoleOutputAligner(bufferWidth: 50, alignMessages: true, stringBuilderProvider: new TestStringBuilderProvider()); string output = aligner.AlignConsoleOutput(message: input, prefixAlreadyWritten: true, prefixWidth: 0); @@ -259,7 +261,7 @@ public void ShortTextWithTabs_NoChange(string input) public void LastTabOverLimit_NoChange(string prefix, string input, int bufferWidthWithoutNewLine, bool prefixAlreadyWritten) { input = input.Replace("\n", Environment.NewLine); - var aligner = new ConsoleOutputAligner(bufferWidth: bufferWidthWithoutNewLine + 1, alignMessages: true); + var aligner = new ConsoleOutputAligner(bufferWidth: bufferWidthWithoutNewLine + 1, alignMessages: true, stringBuilderProvider: new TestStringBuilderProvider()); string output = aligner.AlignConsoleOutput(message: input, prefixAlreadyWritten: prefixAlreadyWritten, prefixWidth: prefix.Length); @@ -274,7 +276,7 @@ public void LastTabOverLimit_NoChange(string prefix, string input, int bufferWid public void LastTabAtLimit_NoChange(string prefix, string input, int bufferWidthWithoutNewLine, bool prefixAlreadyWritten) { input = input.Replace("\n", Environment.NewLine); - var aligner = new ConsoleOutputAligner(bufferWidth: bufferWidthWithoutNewLine + 1, alignMessages: true); + var aligner = new ConsoleOutputAligner(bufferWidth: bufferWidthWithoutNewLine + 1, alignMessages: true, stringBuilderProvider: new TestStringBuilderProvider()); string output = aligner.AlignConsoleOutput(message: input, prefixAlreadyWritten: prefixAlreadyWritten, prefixWidth: prefix.Length); @@ -289,7 +291,7 @@ public void LastTabAtLimit_NoChange(string prefix, string input, int bufferWidth public void TabsMakesItJustOverLimit_IndentAndAlign(string prefix, string input, int bufferWidthWithoutNewLine, bool prefixAlreadyWritten) { input = input.Replace("\n", Environment.NewLine); - var aligner = new ConsoleOutputAligner(bufferWidth: bufferWidthWithoutNewLine + 1, alignMessages: true); + var aligner = new ConsoleOutputAligner(bufferWidth: bufferWidthWithoutNewLine + 1, alignMessages: true, stringBuilderProvider: new TestStringBuilderProvider()); string output = aligner.AlignConsoleOutput(message: input + "x", prefixAlreadyWritten: prefixAlreadyWritten, prefixWidth: prefix.Length); @@ -366,11 +368,17 @@ public void MultiLinesOverLimit_IndentAndAlign(string prefix, string input, stri { input = input.Replace("\n", Environment.NewLine); expected = expected.Replace("\n", Environment.NewLine); - var aligner = new ConsoleOutputAligner(bufferWidth: bufferWidthWithoutNewLine + 1, alignMessages: true); + var aligner = new ConsoleOutputAligner(bufferWidth: bufferWidthWithoutNewLine + 1, alignMessages: true, stringBuilderProvider: new TestStringBuilderProvider()); string output = aligner.AlignConsoleOutput(message: input, prefixAlreadyWritten: prefixAlreadyWritten, prefixWidth: prefix.Length); output.ShouldBe(expected); } + + private sealed class TestStringBuilderProvider : IReusableStringBuilderProvider + { + public StringBuilder Acquire(int capacity) => new StringBuilder(capacity); + public string GetStringAndRelease(StringBuilder builder) => builder.ToString(); + } } } diff --git a/src/Build/Logging/BaseConsoleLogger.cs b/src/Build/Logging/BaseConsoleLogger.cs index d0863f48af2..fb525db841f 100644 --- a/src/Build/Logging/BaseConsoleLogger.cs +++ b/src/Build/Logging/BaseConsoleLogger.cs @@ -4,10 +4,12 @@ using System; using System.Collections; using System.Collections.Generic; +using System.Diagnostics; using System.Globalization; using System.IO; using System.Linq; using System.Text; +using System.Threading; using Microsoft.Build.Evaluation; using Microsoft.Build.Framework; using Microsoft.Build.Internal; @@ -24,7 +26,7 @@ namespace Microsoft.Build.BackEnd.Logging internal delegate void WriteLinePrettyFromResourceDelegate(int indentLevel, string resourceString, params object[] args); #endregion - internal abstract class BaseConsoleLogger : INodeLogger + internal abstract class BaseConsoleLogger : INodeLogger, IReusableStringBuilderProvider { #region Properties @@ -130,7 +132,7 @@ public int Compare(Object a, Object b) /// Depth to indent. internal string IndentString(string s, int indent) { - return OptimizedStringIndenter.IndentString(s, indent); + return OptimizedStringIndenter.IndentString(s, indent, (IReusableStringBuilderProvider)this); } /// @@ -1187,6 +1189,14 @@ private bool ApplyVerbosityParameter(string parameterValue) internal bool runningWithCharacterFileType = false; + /// + /// Since logging messages are processed serially, we can use a single StringBuilder wherever needed. + /// It should not be done directly, but rather through the interface methods. + /// + private StringBuilder _sharedStringBuilder = new StringBuilder(0x100); + + #endregion + #region Per-build Members /// @@ -1231,6 +1241,72 @@ private bool ApplyVerbosityParameter(string parameterValue) #endregion - #endregion + /// + /// Since logging messages are processed serially, we can reuse a single StringBuilder wherever needed. + /// + StringBuilder IReusableStringBuilderProvider.Acquire(int capacity) + { + StringBuilder shared = Interlocked.Exchange(ref _sharedStringBuilder, null); + + Debug.Assert(shared != null, "This is not supposed to be used in multiple threads or multiple time. One method is expected to return it before next acquire. Most probably it was not returned."); + if (shared == null) + { + // This is not supposed to be used concurrently. One method is expected to return it before next acquire. + // However to avoid bugs in production, we will create new string builder + return StringBuilderCache.Acquire(capacity); + } + + if (shared.Capacity < capacity) + { + const int minimumCapacity = 0x100; // 256 characters, 512 bytes + const int maximumBracketedCapacity = 0x80_000; // 512K characters, 1MB + + if (capacity <= minimumCapacity) + { + capacity = minimumCapacity; + } + else if (capacity < maximumBracketedCapacity) + { + // GC likes arrays allocated with power of two bytes. Lets make it happy. + + // Find next power of two http://graphics.stanford.edu/~seander/bithacks.html#RoundUpPowerOf2 + int v = capacity; + + v--; + v |= v >> 1; + v |= v >> 2; + v |= v >> 4; + v |= v >> 8; + v |= v >> 16; + v++; + + capacity = v; + } + // If capacity is > maximumCapacity we will respect it and use it as is. + + // Lets create new instance with enough capacity. + shared = new StringBuilder(capacity); + } + + // Prepare for next use. + // Equivalent of sb.Clear() that works on .Net 3.5 + shared.Length = 0; + + return shared; + } + + /// + /// Acquired StringBuilder must be returned before next use. + /// Unbalanced releases are not supported. + /// + string IReusableStringBuilderProvider.GetStringAndRelease(StringBuilder builder) + { + // This is not supposed to be used concurrently. One method is expected to return it before next acquire. + // But just for sure if _sharedBuilder was already returned, keep the former. + StringBuilder previous = Interlocked.CompareExchange(ref _sharedStringBuilder, builder, null); + Debug.Assert(previous == null, "This is not supposed to be used in multiple threads or multiple time. One method is expected to return it before next acquire. Most probably it was double returned."); + + return builder.ToString(); + } } } diff --git a/src/Build/Logging/OptimizedStringIndenter.cs b/src/Build/Logging/OptimizedStringIndenter.cs index d7ce739f3de..459e28aeb82 100644 --- a/src/Build/Logging/OptimizedStringIndenter.cs +++ b/src/Build/Logging/OptimizedStringIndenter.cs @@ -8,8 +8,8 @@ using System.Runtime.CompilerServices; #else using System.Text; -using Microsoft.Build.Framework; #endif +using Microsoft.Build.Framework; namespace Microsoft.Build.BackEnd.Logging; @@ -52,7 +52,7 @@ internal static class OptimizedStringIndenter #if NET7_0_OR_GREATER [SkipLocalsInit] #endif - internal static unsafe string IndentString(string? s, int indent) + internal static unsafe string IndentString(string? s, int indent, IReusableStringBuilderProvider stringBuilderProvider) { if (s is null) { @@ -89,7 +89,7 @@ internal static unsafe string IndentString(string? s, int indent) }); #pragma warning restore CS8500 #else - StringBuilder builder = StringBuilderCache.Acquire(indentedStringLength); + StringBuilder builder = stringBuilderProvider.Acquire(indentedStringLength); foreach (StringSegment segment in segments) { @@ -99,7 +99,7 @@ internal static unsafe string IndentString(string? s, int indent) .AppendLine(); } - string result = StringBuilderCache.GetStringAndRelease(builder); + string result = stringBuilderProvider.GetStringAndRelease(builder); #endif if (pooledArray is not null) diff --git a/src/Build/Logging/ParallelLogger/ConsoleOutputAligner.cs b/src/Build/Logging/ParallelLogger/ConsoleOutputAligner.cs index 204ed5ceee8..2a4974ba7eb 100644 --- a/src/Build/Logging/ParallelLogger/ConsoleOutputAligner.cs +++ b/src/Build/Logging/ParallelLogger/ConsoleOutputAligner.cs @@ -3,6 +3,7 @@ using System; using System.Text; +using Microsoft.Build.Framework; using Microsoft.Build.Shared; namespace Microsoft.Build.BackEnd.Logging @@ -18,19 +19,21 @@ internal class ConsoleOutputAligner { internal const int ConsoleTabWidth = 8; - private readonly StringBuilder _reusedStringBuilder = new(1024); private readonly int _bufferWidth; private readonly bool _alignMessages; + private readonly IReusableStringBuilderProvider _stringBuilderProvider; /// /// Constructor. /// /// Console buffer width. -1 if unknown/unlimited /// Whether messages are aligned/wrapped into console buffer width - public ConsoleOutputAligner(int bufferWidth, bool alignMessages) + /// + public ConsoleOutputAligner(int bufferWidth, bool alignMessages, IReusableStringBuilderProvider stringBuilderProvider) { _bufferWidth = bufferWidth; _alignMessages = alignMessages; + _stringBuilderProvider = stringBuilderProvider; } /// @@ -50,9 +53,12 @@ public string AlignConsoleOutput(string message, bool prefixAlreadyWritten, int int i = 0; int j = message.IndexOfAny(MSBuildConstants.CrLf); - StringBuilder sb = _reusedStringBuilder; - // prepare reused StringBuilder instance for new use. - sb.Length = 0; + // Empiric value of average line length in console output. Used to estimate number of lines in message for StringBuilder capacity. + // Wrongly estimated capacity is not a problem as StringBuilder will grow as needed. It is just optimization to avoid multiple reallocations. + const int averageLineLength = 40; + int estimatedCapacity = message.Length + ((prefixAlreadyWritten ? 0 : prefixWidth) + Environment.NewLine.Length) * (message.Length / averageLineLength + 1); + StringBuilder sb = _stringBuilderProvider.Acquire(estimatedCapacity); + // The string contains new lines, treat each new line as a different string to format and send to the console while (j >= 0) { @@ -64,7 +70,7 @@ public string AlignConsoleOutput(string message, bool prefixAlreadyWritten, int // Process rest of message AlignAndIndentLineOfMessage(sb, prefixAlreadyWritten, prefixWidth, message, i, message.Length - i); - return sb.ToString(); + return _stringBuilderProvider.GetStringAndRelease(sb); } /// diff --git a/src/Build/Logging/ParallelLogger/ParallelConsoleLogger.cs b/src/Build/Logging/ParallelLogger/ParallelConsoleLogger.cs index 54ca8cba62e..c97255fe8d6 100644 --- a/src/Build/Logging/ParallelLogger/ParallelConsoleLogger.cs +++ b/src/Build/Logging/ParallelLogger/ParallelConsoleLogger.cs @@ -101,7 +101,7 @@ private void CheckIfOutputSupportsAlignment() } } - _consoleOutputAligner = new ConsoleOutputAligner(_bufferWidth, _alignMessages); + _consoleOutputAligner = new ConsoleOutputAligner(_bufferWidth, _alignMessages, (IReusableStringBuilderProvider)this); } #endregion diff --git a/src/Framework/IStringBuilderProvider.cs b/src/Framework/IStringBuilderProvider.cs new file mode 100644 index 00000000000..d2f51c6a481 --- /dev/null +++ b/src/Framework/IStringBuilderProvider.cs @@ -0,0 +1,38 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#nullable disable +using System.Text; + +namespace Microsoft.Build.Framework; + +/// +/// Provider of instances. +/// Main design goal is for reusable String Builders and string builder pools. +/// +/// +/// It is up to particular implementations to decide how to handle unbalanced releases. +/// +internal interface IStringBuilderProvider +{ + /// + /// Get a of at least the specified capacity. + /// + /// The suggested starting size of this instance. + /// A that may or may not be reused. + /// + /// It can be called any number of times; if a is in the cache then + /// it will be returned and the cache emptied. Subsequent calls will return a new . + /// + StringBuilder Acquire(int capacity); + + /// + /// Get a string and return its builder to the cache. + /// + /// Builder to cache (if it's not too big). + /// The equivalent to 's contents. + /// + /// The StringBuilder should not be used after it has been released. + /// + string GetStringAndRelease(StringBuilder builder); +} From a505e39df6ddf9d3ec9f996240886bcba2019854 Mon Sep 17 00:00:00 2001 From: Roman Konecny Date: Fri, 14 Jul 2023 15:24:41 +0200 Subject: [PATCH 10/10] Share string builder in console loggers. --- src/Build.UnitTests/ConsoleOutputAlignerTests.cs | 2 +- src/Build/Logging/BaseConsoleLogger.cs | 10 +++++----- src/Build/Logging/OptimizedStringIndenter.cs | 2 +- .../Logging/ParallelLogger/ConsoleOutputAligner.cs | 4 ++-- .../Logging/ParallelLogger/ParallelConsoleLogger.cs | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Build.UnitTests/ConsoleOutputAlignerTests.cs b/src/Build.UnitTests/ConsoleOutputAlignerTests.cs index 896837b46fd..6b7e72a4679 100644 --- a/src/Build.UnitTests/ConsoleOutputAlignerTests.cs +++ b/src/Build.UnitTests/ConsoleOutputAlignerTests.cs @@ -375,7 +375,7 @@ public void MultiLinesOverLimit_IndentAndAlign(string prefix, string input, stri output.ShouldBe(expected); } - private sealed class TestStringBuilderProvider : IReusableStringBuilderProvider + private sealed class TestStringBuilderProvider : IStringBuilderProvider { public StringBuilder Acquire(int capacity) => new StringBuilder(capacity); public string GetStringAndRelease(StringBuilder builder) => builder.ToString(); diff --git a/src/Build/Logging/BaseConsoleLogger.cs b/src/Build/Logging/BaseConsoleLogger.cs index fb525db841f..b7014cd6ac2 100644 --- a/src/Build/Logging/BaseConsoleLogger.cs +++ b/src/Build/Logging/BaseConsoleLogger.cs @@ -26,7 +26,7 @@ namespace Microsoft.Build.BackEnd.Logging internal delegate void WriteLinePrettyFromResourceDelegate(int indentLevel, string resourceString, params object[] args); #endregion - internal abstract class BaseConsoleLogger : INodeLogger, IReusableStringBuilderProvider + internal abstract class BaseConsoleLogger : INodeLogger, IStringBuilderProvider { #region Properties @@ -132,7 +132,7 @@ public int Compare(Object a, Object b) /// Depth to indent. internal string IndentString(string s, int indent) { - return OptimizedStringIndenter.IndentString(s, indent, (IReusableStringBuilderProvider)this); + return OptimizedStringIndenter.IndentString(s, indent, (IStringBuilderProvider)this); } /// @@ -1191,7 +1191,7 @@ private bool ApplyVerbosityParameter(string parameterValue) /// /// Since logging messages are processed serially, we can use a single StringBuilder wherever needed. - /// It should not be done directly, but rather through the interface methods. + /// It should not be done directly, but rather through the interface methods. /// private StringBuilder _sharedStringBuilder = new StringBuilder(0x100); @@ -1244,7 +1244,7 @@ private bool ApplyVerbosityParameter(string parameterValue) /// /// Since logging messages are processed serially, we can reuse a single StringBuilder wherever needed. /// - StringBuilder IReusableStringBuilderProvider.Acquire(int capacity) + StringBuilder IStringBuilderProvider.Acquire(int capacity) { StringBuilder shared = Interlocked.Exchange(ref _sharedStringBuilder, null); @@ -1299,7 +1299,7 @@ StringBuilder IReusableStringBuilderProvider.Acquire(int capacity) /// Acquired StringBuilder must be returned before next use. /// Unbalanced releases are not supported. /// - string IReusableStringBuilderProvider.GetStringAndRelease(StringBuilder builder) + string IStringBuilderProvider.GetStringAndRelease(StringBuilder builder) { // This is not supposed to be used concurrently. One method is expected to return it before next acquire. // But just for sure if _sharedBuilder was already returned, keep the former. diff --git a/src/Build/Logging/OptimizedStringIndenter.cs b/src/Build/Logging/OptimizedStringIndenter.cs index 459e28aeb82..d98f1d62094 100644 --- a/src/Build/Logging/OptimizedStringIndenter.cs +++ b/src/Build/Logging/OptimizedStringIndenter.cs @@ -52,7 +52,7 @@ internal static class OptimizedStringIndenter #if NET7_0_OR_GREATER [SkipLocalsInit] #endif - internal static unsafe string IndentString(string? s, int indent, IReusableStringBuilderProvider stringBuilderProvider) + internal static unsafe string IndentString(string? s, int indent, IStringBuilderProvider stringBuilderProvider) { if (s is null) { diff --git a/src/Build/Logging/ParallelLogger/ConsoleOutputAligner.cs b/src/Build/Logging/ParallelLogger/ConsoleOutputAligner.cs index 2a4974ba7eb..63e8f26bbe3 100644 --- a/src/Build/Logging/ParallelLogger/ConsoleOutputAligner.cs +++ b/src/Build/Logging/ParallelLogger/ConsoleOutputAligner.cs @@ -21,7 +21,7 @@ internal class ConsoleOutputAligner private readonly int _bufferWidth; private readonly bool _alignMessages; - private readonly IReusableStringBuilderProvider _stringBuilderProvider; + private readonly IStringBuilderProvider _stringBuilderProvider; /// /// Constructor. @@ -29,7 +29,7 @@ internal class ConsoleOutputAligner /// Console buffer width. -1 if unknown/unlimited /// Whether messages are aligned/wrapped into console buffer width /// - public ConsoleOutputAligner(int bufferWidth, bool alignMessages, IReusableStringBuilderProvider stringBuilderProvider) + public ConsoleOutputAligner(int bufferWidth, bool alignMessages, IStringBuilderProvider stringBuilderProvider) { _bufferWidth = bufferWidth; _alignMessages = alignMessages; diff --git a/src/Build/Logging/ParallelLogger/ParallelConsoleLogger.cs b/src/Build/Logging/ParallelLogger/ParallelConsoleLogger.cs index c97255fe8d6..d8d1dce3a34 100644 --- a/src/Build/Logging/ParallelLogger/ParallelConsoleLogger.cs +++ b/src/Build/Logging/ParallelLogger/ParallelConsoleLogger.cs @@ -101,7 +101,7 @@ private void CheckIfOutputSupportsAlignment() } } - _consoleOutputAligner = new ConsoleOutputAligner(_bufferWidth, _alignMessages, (IReusableStringBuilderProvider)this); + _consoleOutputAligner = new ConsoleOutputAligner(_bufferWidth, _alignMessages, (IStringBuilderProvider)this); } #endregion