Return pooled arrays in Regex.Replace when no replacements (#44833)
authorStephen Toub <stoub@microsoft.com>
Tue, 17 Nov 2020 22:55:53 +0000 (17:55 -0500)
committerGitHub <noreply@github.com>
Tue, 17 Nov 2020 22:55:53 +0000 (17:55 -0500)
When Regex.Replace doesn't actually need to replace anything, we're inadvertently not returning a previously rented ArrayPool array to the pool.  On repeated use, that drains the pool of the desired size, such that every attempt ends up allocating a new array, even if there are no replacements to be made.

The fix is to lazily rent from the pool.  This not only fixes the problem, but helps perf further by not taking the rental cost unless we actually need an array to store a replacement segment.

src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Match.cs
src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Regex.Replace.cs
src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexReplacement.cs
src/libraries/System.Text.RegularExpressions/src/System/Text/SegmentStringBuilder.cs

index be217bf..e5022d1 100644 (file)
@@ -122,7 +122,7 @@ namespace System.Text.RegularExpressions
 
             // Gets the weakly cached replacement helper or creates one if there isn't one already.
             RegexReplacement repl = RegexReplacement.GetOrCreate(regex._replref!, replacement, regex.caps!, regex.capsize, regex.capnames!, regex.roptions);
-            var segments = new SegmentStringBuilder(256);
+            SegmentStringBuilder segments = SegmentStringBuilder.Create();
             repl.ReplacementImpl(ref segments, this);
             return segments.ToString();
         }
index 32d7d54..f947305 100644 (file)
@@ -170,7 +170,7 @@ namespace System.Text.RegularExpressions
                 return input;
             }
 
-            var state = (segments: new SegmentStringBuilder(256), evaluator, prevat: 0, input, count);
+            var state = (segments: SegmentStringBuilder.Create(), evaluator, prevat: 0, input, count);
 
             if (!regex.RightToLeft)
             {
index 44450fe..3a27f58 100644 (file)
@@ -211,7 +211,7 @@ namespace System.Text.RegularExpressions
                 return input;
             }
 
-            var state = (replacement: this, segments: new SegmentStringBuilder(256), inputMemory: input.AsMemory(), prevat: 0, count);
+            var state = (replacement: this, segments: SegmentStringBuilder.Create(), inputMemory: input.AsMemory(), prevat: 0, count);
 
             if (!regex.RightToLeft)
             {
index 6348667..87b7297 100644 (file)
@@ -16,14 +16,9 @@ namespace System.Text
         /// <summary>The number of items in <see cref="_array"/>, and thus also the next position in the array to be filled.</summary>
         private int _count;
 
-        /// <summary>Initializes the builder.</summary>
-        /// <param name="capacity">The initial capacity of the builder.</param>
-        public SegmentStringBuilder(int capacity)
-        {
-            Debug.Assert(capacity > 0);
-            _array = ArrayPool<ReadOnlyMemory<char>>.Shared.Rent(capacity);
-            _count = 0;
-        }
+        /// <summary>Creates a new builder.</summary>
+        /// <remarks>Should be used instead of default struct initialization.</remarks>
+        public static SegmentStringBuilder Create() => new SegmentStringBuilder() { _array = Array.Empty<ReadOnlyMemory<char>>() };
 
         /// <summary>Gets the number of segments added to the builder.</summary>
         public int Count => _count;
@@ -54,7 +49,10 @@ namespace System.Text
             ReadOnlyMemory<char>[] array = _array;
             Debug.Assert(array.Length == _count);
 
-            ReadOnlyMemory<char>[] newArray = _array = ArrayPool<ReadOnlyMemory<char>>.Shared.Rent(array.Length * 2);
+            const int DefaultArraySize = 256;
+            int newSize = array.Length == 0 ? DefaultArraySize : array.Length * 2;
+
+            ReadOnlyMemory<char>[] newArray = _array = ArrayPool<ReadOnlyMemory<char>>.Shared.Rent(newSize);
             Array.Copy(array, newArray, _count);
             ArrayPool<ReadOnlyMemory<char>>.Shared.Return(array, clearArray: true);
             newArray[_count++] = segment;