diff --git a/src/Build.OM.UnitTests/Microsoft.Build.Engine.OM.UnitTests.csproj b/src/Build.OM.UnitTests/Microsoft.Build.Engine.OM.UnitTests.csproj index 36c8a20a7f0..00221821ec4 100644 --- a/src/Build.OM.UnitTests/Microsoft.Build.Engine.OM.UnitTests.csproj +++ b/src/Build.OM.UnitTests/Microsoft.Build.Engine.OM.UnitTests.csproj @@ -77,9 +77,6 @@ true - - True - true diff --git a/src/Build/Microsoft.Build.csproj b/src/Build/Microsoft.Build.csproj index cbfee6c66db..19c39093f3f 100644 --- a/src/Build/Microsoft.Build.csproj +++ b/src/Build/Microsoft.Build.csproj @@ -99,9 +99,6 @@ Collections\ReadOnlyEmptyCollection.cs - - True - @@ -140,7 +137,6 @@ true - diff --git a/src/Framework.UnitTests/Microsoft.Build.Framework.UnitTests.csproj b/src/Framework.UnitTests/Microsoft.Build.Framework.UnitTests.csproj index 2ce98b0c404..da03486b433 100644 --- a/src/Framework.UnitTests/Microsoft.Build.Framework.UnitTests.csproj +++ b/src/Framework.UnitTests/Microsoft.Build.Framework.UnitTests.csproj @@ -41,7 +41,6 @@ - diff --git a/src/Framework/MSBuildEventSource.cs b/src/Framework/MSBuildEventSource.cs index e32cb633fbc..f8587c129c4 100644 --- a/src/Framework/MSBuildEventSource.cs +++ b/src/Framework/MSBuildEventSource.cs @@ -490,6 +490,35 @@ public void CachedSdkResolverServiceResolveSdkStop(string sdkName, string soluti WriteEvent(67, sdkName, solutionPath, projectPath, success); } - #endregion + /// + /// This events are quite frequent so they are collected by Debug binaries only. + /// + [Event(68, Keywords = Keywords.All)] + public void ReusableStringBuilderFactoryStart(int hash, int newCapacity, int oldCapacity, string type) + { + WriteEvent(68, hash, newCapacity, oldCapacity, type); + } + + /// + /// This events are quite frequent so they are collected by Debug binaries only. + /// + [Event(69, Keywords = Keywords.All)] + public void ReusableStringBuilderFactoryStop(int hash, int returningCapacity, int returningLength, string type) + { + WriteEvent(69, hash, returningCapacity, returningLength, type); + } + + /// + /// As oppose to other ReusableStringBuilderFactory events this one is expected to happens very un-frequently + /// and if it is seen more than 100x per build it might indicates wrong usage patterns resulting into degrading + /// efficiency of ReusableStringBuilderFactory. Hence it is collected in release build as well. + /// + [Event(70, Keywords = Keywords.All)] + public void ReusableStringBuilderFactoryUnbalanced(int oldHash, int newHash) + { + WriteEvent(70, oldHash, newHash); + } + +#endregion } } diff --git a/src/Framework/ReuseableStringBuilder.cs b/src/Framework/ReuseableStringBuilder.cs new file mode 100644 index 00000000000..18457bee6f6 --- /dev/null +++ b/src/Framework/ReuseableStringBuilder.cs @@ -0,0 +1,355 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#nullable enable +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.Text; +using System.Threading; +using Microsoft.Build.Eventing; + +namespace Microsoft.Build.Framework +{ + /// + /// A StringBuilder lookalike that reuses its internal storage. + /// + /// + /// This class is being deprecated in favor of SpanBasedStringBuilder in StringTools. Avoid adding more uses. + /// + internal sealed class ReuseableStringBuilder : IDisposable + { + /// + /// Captured string builder. + /// + private StringBuilder? _borrowedBuilder; + + /// + /// Capacity of borrowed string builder at the time of borrowing. + /// + private int _borrowedWithCapacity; + + /// + /// Capacity to initialize the builder with. + /// + private int _capacity; + + /// + /// Create a new builder, under the covers wrapping a reused one. + /// + internal ReuseableStringBuilder(int capacity = 16) // StringBuilder default is 16 + { + // lazy initialization of the builder + _capacity = capacity; + } + + /// + /// The length of the target. + /// + public int Length + { + get { return _borrowedBuilder?.Length ?? 0; } + set + { + LazyPrepare(); + _borrowedBuilder.Length = value; + } + } + + /// + /// Convert to a string. + /// + public override string ToString() + { + if (_borrowedBuilder == null) + { + return String.Empty; + } + + return _borrowedBuilder.ToString(); + } + + /// + /// Dispose, indicating you are done with this builder. + /// + public void Dispose() + { + if (_borrowedBuilder != null) + { + ReuseableStringBuilderFactory.Release(this); + _borrowedBuilder = null; + _capacity = -1; + } + } + + /// + /// Append a character. + /// + internal ReuseableStringBuilder Append(char value) + { + LazyPrepare(); + _borrowedBuilder.Append(value); + return this; + } + + /// + /// Append a string. + /// + internal ReuseableStringBuilder Append(string value) + { + LazyPrepare(); + _borrowedBuilder.Append(value); + return this; + } + + /// + /// Append a substring. + /// + internal ReuseableStringBuilder Append(string value, int startIndex, int count) + { + LazyPrepare(); + _borrowedBuilder.Append(value, startIndex, count); + return this; + } + + public ReuseableStringBuilder AppendSeparated(char separator, ICollection strings) + { + LazyPrepare(); + + var separatorsRemaining = strings.Count - 1; + + foreach (var s in strings) + { + _borrowedBuilder.Append(s); + + if (separatorsRemaining > 0) + { + _borrowedBuilder.Append(separator); + } + + separatorsRemaining--; + } + + return this; + } + + public ReuseableStringBuilder Clear() + { + LazyPrepare(); + _borrowedBuilder.Clear(); + return this; + } + + /// + /// Remove a substring. + /// + internal ReuseableStringBuilder Remove(int startIndex, int length) + { + LazyPrepare(); + _borrowedBuilder.Remove(startIndex, length); + return this; + } + + /// + /// Grab a backing builder if necessary. + /// + [MemberNotNull(nameof(_borrowedBuilder))] + private void LazyPrepare() + { + if (_borrowedBuilder == null) + { + FrameworkErrorUtilities.VerifyThrow(_capacity != -1, "Reusing after dispose"); + + _borrowedBuilder = ReuseableStringBuilderFactory.Get(_capacity); + _borrowedWithCapacity = _borrowedBuilder.Capacity; + } + } + + /// + /// A utility class that mediates access to a shared string builder. + /// + /// + /// If this shared builder is highly contended, this class could add + /// a second one and try both in turn. + /// + private static class ReuseableStringBuilderFactory + { + /// + /// Made up limit beyond which we won't share the builder + /// because we could otherwise hold a huge builder indefinitely. + /// This was picked empirically to save at least 95% of allocated data size. + /// This constant has to be exactly 2^n (power of 2) where n = 4 ... 32 as GC is optimized to work with such block sizes. + /// Same approach is used in ArrayPool or RecyclableMemoryStream so having same uniform allocation sizes will + /// reduce likelihood of heaps fragmentation. + /// + /// + /// In order to collect and analyze ETW ReusableStringBuilderFactory events developer could follow these steps: + /// - With compiled as Debug capture events by perfview; example: "perfview collect /NoGui /OnlyProviders=*Microsoft-Build" + /// - Open Events view and filter for ReusableStringBuilderFactory and pick ReusableStringBuilderFactory/Stop + /// - Display columns: returning length, type + /// - Set MaxRet limit to 1_000_000 + /// - Right click and Open View in Excel + /// - Use Excel data analytic tools to extract required data from it. I recommend to use + /// Pivot Table/Chart with + /// filter: type=[return-se,discarder]; + /// rows: returningLength grouped (right click and Group... into sufficient size bins) + /// value: sum of returningLength + /// + /// + /// This constant might looks huge, but rather than lowering this constant, + /// we shall focus on eliminating code which requires creating such huge strings. + /// + private const int MaxBuilderSizeBytes = 2 * 1024 * 1024; // ~1M chars + private const int MaxBuilderSizeCapacity = MaxBuilderSizeBytes / sizeof(char); + + /// + /// The shared builder. + /// + private static StringBuilder? s_sharedBuilder; + +#if DEBUG && ASSERT_BALANCE + /// + /// Balance between calling Get and Release. + /// Shall be always 0 as Get and 1 at Release. + /// + private static int s_getVsReleaseBalance; +#endif + + /// + /// Obtains a string builder which may or may not already + /// have been used. + /// Never returns null. + /// + internal static StringBuilder Get(int capacity) + { +#if DEBUG && ASSERT_BALANCE + int balance = Interlocked.Increment(ref s_getVsReleaseBalance); + Debug.Assert(balance == 1, "Unbalanced Get vs Release. Either forgotten Release or used from multiple threads concurrently."); +#endif + + StringBuilder? returned = Interlocked.Exchange(ref s_sharedBuilder, null); + + if (returned == null) + { + // Currently loaned out so return a new one with capacity in given bracket. + // If user wants bigger capacity than maximum capacity, respect it. + returned = new StringBuilder(SelectBracketedCapacity(capacity)); +#if DEBUG + MSBuildEventSource.Log.ReusableStringBuilderFactoryStart(hash: returned.GetHashCode(), newCapacity:capacity, oldCapacity:0, type:"miss"); +#endif + } + else if (returned.Capacity < capacity) + { + // It's essential we guarantee the capacity because this + // may be used as a buffer to a PInvoke call. + int newCapacity = SelectBracketedCapacity(capacity); +#if DEBUG + MSBuildEventSource.Log.ReusableStringBuilderFactoryStart(hash: returned.GetHashCode(), newCapacity: newCapacity, oldCapacity: returned.Capacity, type: "miss-need-bigger"); +#endif + // Let the current StringBuilder be collected and create new with bracketed capacity. This way it allocates only char[newCapacity] + // otherwise it would allocate char[new_capacity_of_last_chunk] (in set_Capacity) and char[newCapacity] (in Clear). + returned = new StringBuilder(newCapacity); + } + else + { +#if DEBUG + MSBuildEventSource.Log.ReusableStringBuilderFactoryStart(hash: returned.GetHashCode(), newCapacity: capacity, oldCapacity: returned.Capacity, type: "hit"); +#endif + } + + return returned; + } + + /// + /// Returns the shared builder for the next caller to use. + /// ** CALLERS, DO NOT USE THE BUILDER AFTER RELEASING IT HERE! ** + /// + internal static void Release(ReuseableStringBuilder returning) + { +#if DEBUG && ASSERT_BALANCE // Please define ASSERT_BALANCE if you need to analyze where we have cross thread competing usage of ReuseableStringBuilder + int balance = Interlocked.Decrement(ref s_getVsReleaseBalance); + Debug.Assert(balance == 0, "Unbalanced Get vs Release. Either forgotten Release or used from multiple threads concurrently."); +#endif + FrameworkErrorUtilities.VerifyThrowInternalNull(returning._borrowedBuilder, nameof(returning._borrowedBuilder)); + + StringBuilder returningBuilder = returning._borrowedBuilder!; + int returningLength = returningBuilder.Length; + + // It's possible for someone to cause the builder to + // enlarge to such an extent that this static field + // would be a leak. To avoid that, only accept + // the builder if it's no more than a certain size. + // + // If some code has a bug and forgets to return their builder + // (or we refuse it here because it's too big) the next user will + // get given a new one, and then return it soon after. + // So the shared builder will be "replaced". + if (returningBuilder.Capacity > MaxBuilderSizeCapacity) + { + // In order to free memory usage by huge string builder, do not pool this one and let it be collected. +#if DEBUG + MSBuildEventSource.Log.ReusableStringBuilderFactoryStop(hash: returningBuilder.GetHashCode(), returningCapacity: returningBuilder.Capacity, returningLength: returningLength, type: "discard"); +#endif + } + else + { + if (returningBuilder.Capacity != returning._borrowedWithCapacity) + { + Debug.Assert(returningBuilder.Capacity > returning._borrowedWithCapacity, "Capacity can only increase"); + + // This builder used more than pre-allocated capacity bracket. + // Let this builder be collected and put new builder, with reflecting bracket capacity, into the pool. + // If we would just return this builder into pool as is, it would allocated new array[capacity] anyway (current implementation of returningBuilder.Clear() does it) + // and that could lead to unpredictable amount of LOH allocations and eventual LOH fragmentation. + // Below implementation has predictable max Log2(MaxBuilderSizeBytes) string builder array re-allocations during whole process lifetime - unless MaxBuilderSizeCapacity is reached frequently. + int newCapacity = SelectBracketedCapacity(returningBuilder.Capacity); + returningBuilder = new StringBuilder(newCapacity); + } + + returningBuilder.Clear(); // Clear before pooling + + var oldSharedBuilder = Interlocked.Exchange(ref s_sharedBuilder, returningBuilder); + if (oldSharedBuilder != null) + { + // This can identify improper usage from multiple thread or bug in code - Get was reentered before Release. + // User of ReuseableStringBuilder has to make sure that calling method call stacks do not also use ReuseableStringBuilder. + // Look at stack traces of ETW events which contains reported string builder hashes. + MSBuildEventSource.Log.ReusableStringBuilderFactoryUnbalanced(oldHash: oldSharedBuilder.GetHashCode(), newHash: returningBuilder.GetHashCode()); + } +#if DEBUG + MSBuildEventSource.Log.ReusableStringBuilderFactoryStop(hash: returningBuilder.GetHashCode(), returningCapacity: returningBuilder.Capacity, returningLength: returningLength, type: returning._borrowedBuilder != returningBuilder ? "return-new" : "return"); +#endif + } + + // Ensure ReuseableStringBuilder can no longer use _borrowedBuilder + returning._borrowedBuilder = null; + } + + private static int SelectBracketedCapacity(int requiredCapacity) + { + const int minimumCapacity = 0x100; // 256 characters, 512 bytes + + if (requiredCapacity <= minimumCapacity) + return minimumCapacity; + + // If user wants bigger capacity than maximum respect it as it could be used as buffer in P/Invoke. + if (requiredCapacity >= MaxBuilderSizeCapacity) + return requiredCapacity; + + // Find next power of two http://graphics.stanford.edu/~seander/bithacks.html#RoundUpPowerOf2 + int v = requiredCapacity; + + v--; + v |= v >> 1; + v |= v >> 2; + v |= v >> 4; + v |= v >> 8; + v |= v >> 16; + v++; + + return v; + } + } + } +} diff --git a/src/Shared/StringBuilderCache.cs b/src/Framework/StringBuilderCache.cs similarity index 61% rename from src/Shared/StringBuilderCache.cs rename to src/Framework/StringBuilderCache.cs index 9bc6ffeb15d..abf23d07ff4 100644 --- a/src/Shared/StringBuilderCache.cs +++ b/src/Framework/StringBuilderCache.cs @@ -32,16 +32,18 @@ ===========================================================*/ using System; +using System.Diagnostics; using System.Text; +#if !CLR2COMPATIBILITY && !MICROSOFT_BUILD_ENGINE_OM_UNITTESTS +using Microsoft.Build.Eventing; +#endif -namespace Microsoft.Build.Shared +namespace Microsoft.Build.Framework { internal static class StringBuilderCache { - // The value 360 was chosen in discussion with performance experts as a compromise between using - // as little memory (per thread) as possible and still covering a large part of short-lived - // StringBuilder creations on the startup path of VS designers. - private const int MAX_BUILDER_SIZE = 360; + // The value 512 was chosen empirically as 95% percentile of returning string length. + private const int MAX_BUILDER_SIZE = 512; [ThreadStatic] private static StringBuilder t_cachedInstance; @@ -51,27 +53,41 @@ public static StringBuilder Acquire(int capacity = 16 /*StringBuilder.DefaultCap if (capacity <= MAX_BUILDER_SIZE) { StringBuilder sb = StringBuilderCache.t_cachedInstance; + StringBuilderCache.t_cachedInstance = null; if (sb != null) { // Avoid StringBuilder block fragmentation by getting a new StringBuilder // when the requested size is larger than the current capacity if (capacity <= sb.Capacity) { - StringBuilderCache.t_cachedInstance = null; sb.Length = 0; // Equivalent of sb.Clear() that works on .Net 3.5 +#if DEBUG && !CLR2COMPATIBILITY && !MICROSOFT_BUILD_ENGINE_OM_UNITTESTS + MSBuildEventSource.Log.ReusableStringBuilderFactoryStart(hash: sb.GetHashCode(), newCapacity: capacity, oldCapacity: sb.Capacity, type: "sbc-hit"); +#endif return sb; } } } - return new StringBuilder(capacity); + + StringBuilder stringBuilder = new StringBuilder(capacity); +#if DEBUG && !CLR2COMPATIBILITY && !MICROSOFT_BUILD_ENGINE_OM_UNITTESTS + MSBuildEventSource.Log.ReusableStringBuilderFactoryStart(hash: stringBuilder.GetHashCode(), newCapacity: capacity, oldCapacity: stringBuilder.Capacity, type: "sbc-miss"); +#endif + return stringBuilder; } public static void Release(StringBuilder sb) { if (sb.Capacity <= MAX_BUILDER_SIZE) { + // Assert we are not replacing another string builder. That could happen when Acquire is reentered. + // User of StringBuilderCache has to make sure that calling method call stacks do not also use StringBuilderCache. + Debug.Assert(StringBuilderCache.t_cachedInstance == null, "Unexpected replacing of other StringBuilder."); StringBuilderCache.t_cachedInstance = sb; } +#if DEBUG && !CLR2COMPATIBILITY && !MICROSOFT_BUILD_ENGINE_OM_UNITTESTS + MSBuildEventSource.Log.ReusableStringBuilderFactoryStop(hash: sb.GetHashCode(), returningCapacity: sb.Capacity, returningLength: sb.Length, type: sb.Capacity <= MAX_BUILDER_SIZE ? "sbc-return" : "sbc-discard"); +#endif } public static string GetStringAndRelease(StringBuilder sb) diff --git a/src/MSBuild/MSBuild.csproj b/src/MSBuild/MSBuild.csproj index 4361f23bd06..9305e18c7ff 100644 --- a/src/MSBuild/MSBuild.csproj +++ b/src/MSBuild/MSBuild.csproj @@ -152,9 +152,6 @@ true - - true - true diff --git a/src/MSBuildTaskHost/MSBuildTaskHost.csproj b/src/MSBuildTaskHost/MSBuildTaskHost.csproj index a2e79399940..cc9f10fdb27 100644 --- a/src/MSBuildTaskHost/MSBuildTaskHost.csproj +++ b/src/MSBuildTaskHost/MSBuildTaskHost.csproj @@ -139,7 +139,7 @@ ResourceUtilities.cs - + StringBuilderCache.cs diff --git a/src/Shared/EscapingUtilities.cs b/src/Shared/EscapingUtilities.cs index 309e39a2988..723a4df0139 100644 --- a/src/Shared/EscapingUtilities.cs +++ b/src/Shared/EscapingUtilities.cs @@ -6,6 +6,7 @@ using System.Globalization; using System.Text; +using Microsoft.Build.Framework; using Microsoft.NET.StringTools; namespace Microsoft.Build.Shared diff --git a/src/Shared/ReuseableStringBuilder.cs b/src/Shared/ReuseableStringBuilder.cs deleted file mode 100644 index 8abf89a0093..00000000000 --- a/src/Shared/ReuseableStringBuilder.cs +++ /dev/null @@ -1,313 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using System; -using System.Collections.Concurrent; -using System.Collections.Generic; -using System.Diagnostics.CodeAnalysis; -using System.Globalization; -using System.Text; -using System.Threading; - -namespace Microsoft.Build.Shared -{ - /// - /// A StringBuilder lookalike that reuses its internal storage. - /// - /// - /// This class is being deprecated in favor of SpanBasedStringBuilder in StringTools. Avoid adding more uses. - /// - internal sealed class ReuseableStringBuilder : IDisposable - { - /// - /// Captured string builder. - /// - private StringBuilder _borrowedBuilder; - - /// - /// Capacity to initialize the builder with. - /// - private int _capacity; - - /// - /// Create a new builder, under the covers wrapping a reused one. - /// - internal ReuseableStringBuilder(int capacity = 16) // StringBuilder default is 16 - { - _capacity = capacity; - - // lazy initialization of the builder - } - - /// - /// The length of the target. - /// - public int Length - { - get { return (_borrowedBuilder == null) ? 0 : _borrowedBuilder.Length; } - set - { - LazyPrepare(); - _borrowedBuilder.Length = value; - } - } - - /// - /// Convert to a string. - /// - public override string ToString() - { - if (_borrowedBuilder == null) - { - return String.Empty; - } - - return _borrowedBuilder.ToString(); - } - - /// - /// Dispose, indicating you are done with this builder. - /// - void IDisposable.Dispose() - { - if (_borrowedBuilder != null) - { - ReuseableStringBuilderFactory.Release(_borrowedBuilder); - _borrowedBuilder = null; - _capacity = -1; - } - } - - /// - /// Append a character. - /// - internal ReuseableStringBuilder Append(char value) - { - LazyPrepare(); - _borrowedBuilder.Append(value); - return this; - } - - /// - /// Append a string. - /// - internal ReuseableStringBuilder Append(string value) - { - LazyPrepare(); - _borrowedBuilder.Append(value); - return this; - } - - /// - /// Append a substring. - /// - internal ReuseableStringBuilder Append(string value, int startIndex, int count) - { - LazyPrepare(); - _borrowedBuilder.Append(value, startIndex, count); - return this; - } - - public ReuseableStringBuilder AppendSeparated(char separator, ICollection strings) - { - LazyPrepare(); - - var separatorsRemaining = strings.Count - 1; - - foreach (var s in strings) - { - _borrowedBuilder.Append(s); - - if (separatorsRemaining > 0) - { - _borrowedBuilder.Append(separator); - } - - separatorsRemaining--; - } - - return this; - } - - public ReuseableStringBuilder Clear() - { - LazyPrepare(); - _borrowedBuilder.Clear(); - return this; - } - - /// - /// Remove a substring. - /// - internal ReuseableStringBuilder Remove(int startIndex, int length) - { - LazyPrepare(); - _borrowedBuilder.Remove(startIndex, length); - return this; - } - - /// - /// Grab a backing builder if necessary. - /// - private void LazyPrepare() - { - if (_borrowedBuilder == null) - { - ErrorUtilities.VerifyThrow(_capacity != -1, "Reusing after dispose"); - - _borrowedBuilder = ReuseableStringBuilderFactory.Get(_capacity); - } - } - - /// - /// A utility class that mediates access to a shared string builder. - /// - /// - /// If this shared builder is highly contended, this class could add - /// a second one and try both in turn. - /// - private static class ReuseableStringBuilderFactory - { - /// - /// Made up limit beyond which we won't share the builder - /// because we could otherwise hold a huge builder indefinitely. - /// This size seems reasonable for MSBuild uses (mostly expression expansion) - /// - private const int MaxBuilderSize = 1024; - - /// - /// The shared builder. - /// - private static StringBuilder s_sharedBuilder; - -#if DEBUG - /// - /// Count of successful reuses - /// - private static int s_hits = 0; - - /// - /// Count of failed reuses - a new builder was created - /// - private static int s_misses = 0; - - /// - /// Count of times the builder capacity was raised to satisfy the caller's request - /// - private static int s_upsizes = 0; - - /// - /// Count of times the returned builder was discarded because it was too large - /// - private static int s_discards = 0; - - /// - /// Count of times the builder was returned. - /// - private static int s_accepts = 0; - - /// - /// Aggregate capacity saved (aggregate midpoints of requested and returned) - /// - private static int s_saved = 0; - - /// - /// Callstacks of those handed out and not returned yet - /// - private static ConcurrentDictionary s_handouts = new ConcurrentDictionary(); -#endif - /// - /// Obtains a string builder which may or may not already - /// have been used. - /// Never returns null. - /// - internal static StringBuilder Get(int capacity) - { -#if DEBUG - bool missed = false; -#endif - var returned = Interlocked.Exchange(ref s_sharedBuilder, null); - - if (returned == null) - { -#if DEBUG - missed = true; - Interlocked.Increment(ref s_misses); -#endif - // Currently loaned out so return a new one - returned = new StringBuilder(capacity); - } - else if (returned.Capacity < capacity) - { -#if DEBUG - Interlocked.Increment(ref s_upsizes); -#endif - // It's essential we guarantee the capacity because this - // may be used as a buffer to a PInvoke call. - returned.Capacity = capacity; - } - -#if DEBUG - Interlocked.Increment(ref s_hits); - - if (!missed) - { - Interlocked.Add(ref s_saved, (capacity + returned.Capacity) / 2); - } - - // handouts.TryAdd(returned, Environment.StackTrace); -#endif - return returned; - } - - /// - /// Returns the shared builder for the next caller to use. - /// ** CALLERS, DO NOT USE THE BUILDER AFTER RELEASING IT HERE! ** - /// - internal static void Release(StringBuilder returningBuilder) - { - // It's possible for someone to cause the builder to - // enlarge to such an extent that this static field - // would be a leak. To avoid that, only accept - // the builder if it's no more than a certain size. - // - // If some code has a bug and forgets to return their builder - // (or we refuse it here because it's too big) the next user will - // get given a new one, and then return it soon after. - // So the shared builder will be "replaced". - if (returningBuilder.Capacity < MaxBuilderSize) - { - // ErrorUtilities.VerifyThrow(handouts.TryRemove(returningBuilder, out dummy), "returned but not loaned"); - returningBuilder.Clear(); // Clear before pooling - - Interlocked.Exchange(ref s_sharedBuilder, returningBuilder); -#if DEBUG - Interlocked.Increment(ref s_accepts); - } - else - { - Interlocked.Increment(ref s_discards); -#endif - } - } - -#if DEBUG - /// - /// Debugging dumping - /// - [SuppressMessage("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode", Justification = "Handy helper method that can be used to annotate ReuseableStringBuilder when debugging it, but is not hooked up usually for the sake of perf.")] - [SuppressMessage("Microsoft.Usage", "CA1806:DoNotIgnoreMethodResults", MessageId = "System.String.Format(System.IFormatProvider,System.String,System.Object[])", Justification = "Handy string that can be used to annotate ReuseableStringBuilder when debugging it, but is not hooked up usually.")] - internal static void DumpUnreturned() - { - String.Format(CultureInfo.CurrentUICulture, "{0} Hits of which\n {1} Misses (was on loan)\n {2} Upsizes (needed bigger) \n\n{3} Returns=\n{4} Discards (returned too large)+\n {5} Accepts\n\n{6} estimated bytes saved", s_hits, s_misses, s_upsizes, s_discards + s_accepts, s_discards, s_accepts, s_saved); - - Console.WriteLine("Unreturned string builders were allocated here:"); - foreach (var entry in s_handouts.Values) - { - Console.WriteLine(entry + "\n"); - } - } -#endif - } - } -} diff --git a/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs b/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs index 53b19c17a62..5cb27a7ab28 100644 --- a/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs +++ b/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs @@ -1087,7 +1087,10 @@ quiet at the engine level. bool logWarning = idealAssemblyRemappingsIdentities.Any(i => i.assemblyName.FullName.Equals(fusionName) && i.reference.GetConflictVictims().Count == 0); StringBuilder logConflict = StringBuilderCache.Acquire(); LogConflict(conflictCandidate, fusionName, logConflict); - StringBuilder logDependencies = logWarning ? logConflict.AppendLine() : StringBuilderCache.Acquire(); + + // If we are logging warnings append it into existing StringBuilder, otherwise build details by new StringBuilder. + // Remark: There is no point to use StringBuilderCache.Acquire() here as at this point StringBuilderCache already rent StringBuilder for this thread + StringBuilder logDependencies = logWarning ? logConflict.AppendLine() : new StringBuilder(); // Log the assemblies and primary source items which are related to the conflict which was just logged. Reference victor = dependencyTable.GetReference(conflictCandidate.ConflictVictorName); @@ -1108,7 +1111,7 @@ quiet at the engine level. } else { - details = StringBuilderCache.GetStringAndRelease(logDependencies); + details = logDependencies.ToString(); Log.LogMessage(ChooseReferenceLoggingImportance(conflictCandidate), output); Log.LogMessage(MessageImportance.Low, details); } diff --git a/src/Tasks/Microsoft.Build.Tasks.csproj b/src/Tasks/Microsoft.Build.Tasks.csproj index 19b5dde493f..85672c4c284 100644 --- a/src/Tasks/Microsoft.Build.Tasks.csproj +++ b/src/Tasks/Microsoft.Build.Tasks.csproj @@ -98,13 +98,6 @@ RegistryHelper.cs true - - StringBuilderCache.cs - true - - - ReuseableStringBuilder.cs - StrongNameHelpers.cs diff --git a/src/UnitTests.Shared/Microsoft.Build.UnitTests.Shared.csproj b/src/UnitTests.Shared/Microsoft.Build.UnitTests.Shared.csproj index 3cb8f7f10e4..5ad6f792355 100644 --- a/src/UnitTests.Shared/Microsoft.Build.UnitTests.Shared.csproj +++ b/src/UnitTests.Shared/Microsoft.Build.UnitTests.Shared.csproj @@ -57,9 +57,6 @@ ResourceUtilities.cs - - StringBuilderCache.cs - diff --git a/src/Utilities/Microsoft.Build.Utilities.csproj b/src/Utilities/Microsoft.Build.Utilities.csproj index 065b9fc3233..4406635c278 100644 --- a/src/Utilities/Microsoft.Build.Utilities.csproj +++ b/src/Utilities/Microsoft.Build.Utilities.csproj @@ -1,4 +1,4 @@ - + @@ -131,12 +131,6 @@ Shared\ResourceUtilities.cs - - Shared\ReuseableStringBuilder.cs - - - Shared\StringBuilderCache.cs - Shared\TaskLoggingHelper.cs