From 035b729d829368c2790d825bd02db14f0c0fd2ea Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Wed, 14 Oct 2020 14:55:42 -0500 Subject: [PATCH] Improve ArrayBufferWriter re-alloc perf when size is > int.MaxSize / 2 (#42950) --- .../Common/src/System/Buffers/ArrayBufferWriter.cs | 19 +++++++++++++++---- .../ArrayBufferWriter/ArrayBufferWriterTests.Byte.cs | 18 ++++++++++++++---- .../src/System/Text/Json/JsonHelpers.cs | 12 ++++++++++++ .../src/System/Text/Json/ThrowHelper.cs | 6 ++++++ .../src/System/Text/Json/Writer/Utf8JsonWriter.cs | 5 ++++- .../System.Text.Json/tests/Utf8JsonWriterTests.cs | 11 ++++++----- 6 files changed, 57 insertions(+), 14 deletions(-) diff --git a/src/libraries/Common/src/System/Buffers/ArrayBufferWriter.cs b/src/libraries/Common/src/System/Buffers/ArrayBufferWriter.cs index b6271ba..905e9f8 100644 --- 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 : IBufferWriter { + // 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; /// /// Creates an instance of an , 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 --- 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(int.MaxValue / 2 + 1); - output.Advance(int.MaxValue / 2 + 1); - Memory 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(initialCapacity); + output.Advance(initialCapacity); + + // Validate we can't double the buffer size, but can grow + Memory 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(() => 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 --- 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; + /// /// Returns the span for the given reader. /// @@ -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 --- 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 --- 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 --- 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(() => 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); } } -- 2.7.4