Improve ArrayBufferWriter re-alloc perf when size is > int.MaxSize / 2 (#42950)

diff --git a/src/libraries/Common/src/System/Buffers/ArrayBufferWriter.cs b/src/libraries/Common/src/System/Buffers/ArrayBufferWriter.cs
index b6271ba..905e9f8 100644 (file)
--- a/src/libraries/Common/src/System/Buffers/ArrayBufferWriter.cs
+++ b/src/libraries/Common/src/System/Buffers/ArrayBufferWriter.cs
@@ -15,10 +15,14 @@ namespace System.Buffers
 #endif
     sealed class ArrayBufferWriter<T> : IBufferWriter<T>
     {
+        // Copy of Array.MaxArrayLength. For byte arrays the limit is slightly larger
+        private const int MaxArrayLength = 0X7FEFFFFF;
+
+        private const int DefaultInitialBufferSize = 256;
+
         private T[] _buffer;
         private int _index;
 
-        private const int DefaultInitialBufferSize = 256;
 
         /// <summary>
         /// Creates an instance of an <see cref="ArrayBufferWriter{T}"/>, in which data can be written to,
@@ -167,6 +171,8 @@ namespace System.Buffers
             if (sizeHint > FreeCapacity)
             {
                 int currentLength = _buffer.Length;
+
+                // Attempt to grow by the larger of the sizeHint and double the current size.
                 int growBy = Math.Max(sizeHint, currentLength);
 
                 if (currentLength == 0)
@@ -178,11 +184,16 @@ namespace System.Buffers
 
                 if ((uint)newSize > int.MaxValue)
                 {
-                    newSize = currentLength + sizeHint;
-                    if ((uint)newSize > int.MaxValue)
+                    // Attempt to grow to MaxArrayLength.
+                    uint needed = (uint)(currentLength - FreeCapacity + sizeHint);
+                    Debug.Assert(needed > currentLength);
+
+                    if (needed > MaxArrayLength)
                     {
-                        ThrowOutOfMemoryException((uint)newSize);
+                        ThrowOutOfMemoryException(needed);
                     }
+
+                    newSize = MaxArrayLength;
                 }
 
                 Array.Resize(ref _buffer, newSize);

diff --git a/src/libraries/System.Memory/tests/ArrayBufferWriter/ArrayBufferWriterTests.Byte.cs b/src/libraries/System.Memory/tests/ArrayBufferWriter/ArrayBufferWriterTests.Byte.cs
index 7583b9a..8b079c8 100644 (file)
--- a/src/libraries/System.Memory/tests/ArrayBufferWriter/ArrayBufferWriterTests.Byte.cs
+++ b/src/libraries/System.Memory/tests/ArrayBufferWriter/ArrayBufferWriterTests.Byte.cs
@@ -98,10 +98,20 @@ namespace System.Buffers.Tests
         [OuterLoop]
         public void GetMemory_ExceedMaximumBufferSize()
         {
-            var output = new ArrayBufferWriter<byte>(int.MaxValue / 2 + 1);
-            output.Advance(int.MaxValue / 2 + 1);
-            Memory<byte> memory = output.GetMemory(1); // Validate we can't double the buffer size, but can grow by sizeHint
-            Assert.Equal(1, memory.Length);
+            const int MaxArrayLength = 0X7FEFFFFF;
+
+            int initialCapacity = int.MaxValue / 2 + 1;
+
+            var output = new ArrayBufferWriter<byte>(initialCapacity);
+            output.Advance(initialCapacity);
+
+            // Validate we can't double the buffer size, but can grow
+            Memory<byte> memory = output.GetMemory(1);
+
+            // The buffer should grow more than the 1 byte requested otherwise performance will not be usable
+            // between 1GB and 2GB. The current implementation maxes out the buffer size to MaxArrayLength.
+            Assert.Equal(MaxArrayLength - initialCapacity, memory.Length);
+
             Assert.Throws<OutOfMemoryException>(() => output.GetMemory(int.MaxValue));
         }
     }

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/JsonHelpers.cs b/src/libraries/System.Text.Json/src/System/Text/Json/JsonHelpers.cs
index 813627a..bcdbdf0 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/JsonHelpers.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/JsonHelpers.cs
@@ -11,6 +11,9 @@ namespace System.Text.Json
 {
     internal static partial class JsonHelpers
     {
+        // Copy of Array.MaxArrayLength. For byte arrays the limit is slightly larger
+        private const int MaxArrayLength = 0X7FEFFFFF;
+
         /// <summary>
         /// Returns the span for the given reader.
         /// </summary>
@@ -143,5 +146,14 @@ namespace System.Text.Json
                 JsonNumberHandling.AllowReadingFromString |
                 JsonNumberHandling.WriteAsString |
                 JsonNumberHandling.AllowNamedFloatingPointLiterals));
+
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public static void ValidateInt32MaxArrayLength(uint length)
+        {
+            if (length > MaxArrayLength)
+            {
+                ThrowHelper.ThrowOutOfMemoryException(length);
+            }
+        }
     }
 }

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.cs b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.cs
index 1e4c528..f51b9e7 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.cs
@@ -506,6 +506,12 @@ namespace System.Text.Json
             return ex;
         }
 
+        [DoesNotReturn]
+        public static void ThrowOutOfMemoryException(uint capacity)
+        {
+            throw new OutOfMemoryException(SR.Format(SR.BufferMaximumSizeExceeded, capacity));
+        }
+
         // This function will convert an ExceptionResource enum value to the resource string.
         [MethodImpl(MethodImplOptions.NoInlining)]
         private static string GetResourceString(ExceptionResource resource, int currentDepth, byte token, JsonTokenType tokenType)

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.cs
index 196e8d2..2c5cc11 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.cs
@@ -1004,7 +1004,10 @@ namespace System.Text.Json
             {
                 Debug.Assert(_arrayBufferWriter != null);
 
-                _memory = _arrayBufferWriter.GetMemory(checked(BytesPending + sizeHint));
+                int needed = BytesPending + sizeHint;
+                JsonHelpers.ValidateInt32MaxArrayLength((uint)needed);
+
+                _memory = _arrayBufferWriter.GetMemory(needed);
 
                 Debug.Assert(_memory.Length >= sizeHint);
             }

diff --git a/src/libraries/System.Text.Json/tests/Utf8JsonWriterTests.cs b/src/libraries/System.Text.Json/tests/Utf8JsonWriterTests.cs
index 9e966bf..f4c4644 100644 (file)
--- a/src/libraries/System.Text.Json/tests/Utf8JsonWriterTests.cs
+++ b/src/libraries/System.Text.Json/tests/Utf8JsonWriterTests.cs
@@ -744,16 +744,17 @@ namespace System.Text.Json.Tests
                 }
                 Assert.Equal(150_097_503, writer.BytesPending);
 
-                for (int i = 0; i < 6; i++)
+                for (int i = 0; i < 13; i++)
                 {
                     writer.WriteStringValue(text3);
                 }
-                Assert.Equal(1_050_097_521, writer.BytesPending);
+                Assert.Equal(2_100_097_542, writer.BytesPending);
+
+                // Next write forces a grow beyond max array length
 
-                // Next write forces a grow beyond 2 GB
                 Assert.Throws<OutOfMemoryException>(() => writer.WriteStringValue(text3));
 
-                Assert.Equal(1_050_097_521, writer.BytesPending);
+                Assert.Equal(2_100_097_542, writer.BytesPending);
 
                 var text4 = JsonEncodedText.Encode(largeArray.AsSpan(0, 1));
                 for (int i = 0; i < 10_000_000; i++)
@@ -761,7 +762,7 @@ namespace System.Text.Json.Tests
                     writer.WriteStringValue(text4);
                 }
 
-                Assert.Equal(1_050_097_521 + (4 * 10_000_000), writer.BytesPending);
+                Assert.Equal(2_100_097_542 + (4 * 10_000_000), writer.BytesPending);
             }
         }