Remove buffer copying from Utf8JsonReader Deserialize methods. (#73092)
authorEirik Tsarpalis <eirik.tsarpalis@gmail.com>
Mon, 1 Aug 2022 23:04:27 +0000 (00:04 +0100)
committerGitHub <noreply@github.com>
Mon, 1 Aug 2022 23:04:27 +0000 (00:04 +0100)
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverter.ReadAhead.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.ReadCore.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Utf8JsonReader.cs
src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs

index d0faaa3..aeba2b9 100644 (file)
@@ -44,6 +44,7 @@ namespace System.Text.Json.Serialization.Converters
                 }
 
                 ReadOnlySpan<byte> originalSpan = reader.OriginalSpan;
+                ReadOnlySequence<byte> originalSequence = reader.OriginalSequence;
 
                 ReadConstructorArguments(ref state, ref reader, options);
 
@@ -65,10 +66,15 @@ namespace System.Text.Json.Serialization.Converters
                         byte[]? propertyNameArray = properties[i].Item4;
                         string? dataExtKey = properties[i].Item5;
 
-                        tempReader = new Utf8JsonReader(
-                            originalSpan.Slice(checked((int)resumptionByteIndex)),
-                            isFinalBlock: true,
-                            state: properties[i].Item2);
+                        tempReader = originalSequence.IsEmpty
+                            ? new Utf8JsonReader(
+                                originalSpan.Slice(checked((int)resumptionByteIndex)),
+                                isFinalBlock: true,
+                                state: properties[i].Item2)
+                            : new Utf8JsonReader(
+                                originalSequence.Slice(resumptionByteIndex),
+                                isFinalBlock: true,
+                                state: properties[i].Item2);
 
                         Debug.Assert(tempReader.TokenType == JsonTokenType.PropertyName);
 
index 813dad7..50987aa 100644 (file)
@@ -32,8 +32,7 @@ namespace System.Text.Json.Serialization
         {
             // When we're reading ahead we always have to save the state as we don't know if the next token
             // is an opening object or an array brace.
-            JsonReaderState initialReaderState = reader.CurrentState;
-            long initialReaderBytesConsumed = reader.BytesConsumed;
+            Utf8JsonReader restore = reader;
 
             if (!reader.Read())
             {
@@ -42,20 +41,14 @@ namespace System.Text.Json.Serialization
 
             // Perform the actual read-ahead.
             JsonTokenType tokenType = reader.TokenType;
-            if (tokenType == JsonTokenType.StartObject || tokenType == JsonTokenType.StartArray)
+            if (tokenType is JsonTokenType.StartObject or JsonTokenType.StartArray)
             {
                 // Attempt to skip to make sure we have all the data we need.
                 bool complete = reader.TrySkip();
 
                 // We need to restore the state in all cases as we need to be positioned back before
                 // the current token to either attempt to skip again or to actually read the value.
-
-                reader = new Utf8JsonReader(reader.OriginalSpan.Slice(checked((int)initialReaderBytesConsumed)),
-                    isFinalBlock: reader.IsFinalBlock,
-                    state: initialReaderState);
-
-                Debug.Assert(reader.BytesConsumed == 0);
-                state.BytesConsumed += initialReaderBytesConsumed;
+                reader = restore;
 
                 if (!complete)
                 {
index c327ada..1294163 100644 (file)
@@ -58,8 +58,7 @@ namespace System.Text.Json.Serialization
                     }
                 }
 
-                JsonPropertyInfo jsonPropertyInfo = state.Current.JsonTypeInfo.PropertyInfoForTypeInfo;
-                bool success = TryRead(ref reader, jsonPropertyInfo.PropertyType, options, ref state, out T? value);
+                bool success = TryRead(ref reader, TypeToConvert, options, ref state, out T? value);
                 if (success)
                 {
                     // Read any trailing whitespace. This will throw if JsonCommentHandling=Disallow.
index b29d7d5..7eaadb2 100644 (file)
@@ -240,17 +240,32 @@ namespace System.Text.Json
         private static TValue? Read<TValue>(ref Utf8JsonReader reader, JsonTypeInfo jsonTypeInfo)
         {
             Debug.Assert(jsonTypeInfo.IsConfigured);
+
+            if (reader.CurrentState.Options.CommentHandling == JsonCommentHandling.Allow)
+            {
+                ThrowHelper.ThrowArgumentException_SerializerDoesNotSupportComments(nameof(reader));
+            }
+
             ReadStack state = default;
             state.Initialize(jsonTypeInfo);
+            Utf8JsonReader restore = reader;
 
-            JsonReaderState readerState = reader.CurrentState;
-            if (readerState.Options.CommentHandling == JsonCommentHandling.Allow)
+            try
             {
-                throw new ArgumentException(SR.JsonSerializerDoesNotSupportComments, nameof(reader));
+                Utf8JsonReader scopedReader = GetReaderScopedToNextValue(ref reader, ref state);
+                return ReadCore<TValue>(ref scopedReader, jsonTypeInfo, ref state);
+            }
+            catch (JsonException)
+            {
+                reader = restore;
+                throw;
             }
+        }
 
-            // Value copy to overwrite the ref on an exception and undo the destructive reads.
-            Utf8JsonReader restore = reader;
+        private static Utf8JsonReader GetReaderScopedToNextValue(ref Utf8JsonReader reader, ref ReadStack state)
+        {
+            // Advances the provided reader, validating that it is pointing to a complete JSON value.
+            // If successful, returns a new Utf8JsonReader that is scoped to the next value, reusing existing buffers.
 
             ReadOnlySpan<byte> valueSpan = default;
             ReadOnlySequence<byte> valueSequence = default;
@@ -280,166 +295,111 @@ namespace System.Text.Json
                     // Any of the "value start" states are acceptable.
                     case JsonTokenType.StartObject:
                     case JsonTokenType.StartArray:
-                        {
-                            long startingOffset = reader.TokenStartIndex;
-
-                            if (!reader.TrySkip())
-                            {
-                                ThrowHelper.ThrowJsonReaderException(ref reader, ExceptionResource.NotEnoughData);
-                            }
-
-                            long totalLength = reader.BytesConsumed - startingOffset;
-                            ReadOnlySequence<byte> sequence = reader.OriginalSequence;
+                        long startingOffset = reader.TokenStartIndex;
 
-                            if (sequence.IsEmpty)
-                            {
-                                valueSpan = reader.OriginalSpan.Slice(
-                                    checked((int)startingOffset),
-                                    checked((int)totalLength));
-                            }
-                            else
-                            {
-                                valueSequence = sequence.Slice(startingOffset, totalLength);
-                            }
+                        if (!reader.TrySkip())
+                        {
+                            ThrowHelper.ThrowJsonReaderException(ref reader, ExceptionResource.NotEnoughData);
+                        }
 
-                            Debug.Assert(
-                                reader.TokenType == JsonTokenType.EndObject ||
-                                reader.TokenType == JsonTokenType.EndArray);
+                        long totalLength = reader.BytesConsumed - startingOffset;
+                        ReadOnlySequence<byte> sequence = reader.OriginalSequence;
 
-                            break;
+                        if (sequence.IsEmpty)
+                        {
+                            valueSpan = reader.OriginalSpan.Slice(
+                                checked((int)startingOffset),
+                                checked((int)totalLength));
+                        }
+                        else
+                        {
+                            valueSequence = sequence.Slice(startingOffset, totalLength);
                         }
 
+                        Debug.Assert(reader.TokenType is JsonTokenType.EndObject or JsonTokenType.EndArray);
+                        break;
+
                     // Single-token values
                     case JsonTokenType.Number:
                     case JsonTokenType.True:
                     case JsonTokenType.False:
                     case JsonTokenType.Null:
+                        if (reader.HasValueSequence)
                         {
-                            if (reader.HasValueSequence)
-                            {
-                                valueSequence = reader.ValueSequence;
-                            }
-                            else
-                            {
-                                valueSpan = reader.ValueSpan;
-                            }
-
-                            break;
+                            valueSequence = reader.ValueSequence;
                         }
+                        else
+                        {
+                            valueSpan = reader.ValueSpan;
+                        }
+
+                        break;
+
                     // String's ValueSequence/ValueSpan omits the quotes, we need them back.
                     case JsonTokenType.String:
+                        ReadOnlySequence<byte> originalSequence = reader.OriginalSequence;
+
+                        if (originalSequence.IsEmpty)
                         {
-                            ReadOnlySequence<byte> sequence = reader.OriginalSequence;
+                            // Since the quoted string fit in a ReadOnlySpan originally
+                            // the contents length plus the two quotes can't overflow.
+                            int payloadLength = reader.ValueSpan.Length + 2;
+                            Debug.Assert(payloadLength > 1);
 
-                            if (sequence.IsEmpty)
-                            {
-                                // Since the quoted string fit in a ReadOnlySpan originally
-                                // the contents length plus the two quotes can't overflow.
-                                int payloadLength = reader.ValueSpan.Length + 2;
-                                Debug.Assert(payloadLength > 1);
+                            ReadOnlySpan<byte> readerSpan = reader.OriginalSpan;
 
-                                ReadOnlySpan<byte> readerSpan = reader.OriginalSpan;
+                            Debug.Assert(
+                                readerSpan[(int)reader.TokenStartIndex] == (byte)'"',
+                                $"Calculated span starts with {readerSpan[(int)reader.TokenStartIndex]}");
 
-                                Debug.Assert(
-                                    readerSpan[(int)reader.TokenStartIndex] == (byte)'"',
-                                    $"Calculated span starts with {readerSpan[(int)reader.TokenStartIndex]}");
+                            Debug.Assert(
+                                readerSpan[(int)reader.TokenStartIndex + payloadLength - 1] == (byte)'"',
+                                $"Calculated span ends with {readerSpan[(int)reader.TokenStartIndex + payloadLength - 1]}");
 
-                                Debug.Assert(
-                                    readerSpan[(int)reader.TokenStartIndex + payloadLength - 1] == (byte)'"',
-                                    $"Calculated span ends with {readerSpan[(int)reader.TokenStartIndex + payloadLength - 1]}");
+                            valueSpan = readerSpan.Slice((int)reader.TokenStartIndex, payloadLength);
+                        }
+                        else
+                        {
+                            long payloadLength = reader.HasValueSequence
+                                ? reader.ValueSequence.Length + 2
+                                : reader.ValueSpan.Length + 2;
 
-                                valueSpan = readerSpan.Slice((int)reader.TokenStartIndex, payloadLength);
-                            }
-                            else
-                            {
-                                long payloadLength = 2;
-
-                                if (reader.HasValueSequence)
-                                {
-                                    payloadLength += reader.ValueSequence.Length;
-                                }
-                                else
-                                {
-                                    payloadLength += reader.ValueSpan.Length;
-                                }
-
-                                valueSequence = sequence.Slice(reader.TokenStartIndex, payloadLength);
-                                Debug.Assert(
-                                    valueSequence.First.Span[0] == (byte)'"',
-                                    $"Calculated sequence starts with {valueSequence.First.Span[0]}");
-
-                                Debug.Assert(
-                                    valueSequence.ToArray()[payloadLength - 1] == (byte)'"',
-                                    $"Calculated sequence ends with {valueSequence.ToArray()[payloadLength - 1]}");
-                            }
+                            valueSequence = originalSequence.Slice(reader.TokenStartIndex, payloadLength);
+                            Debug.Assert(
+                                valueSequence.First.Span[0] == (byte)'"',
+                                $"Calculated sequence starts with {valueSequence.First.Span[0]}");
 
-                            break;
+                            Debug.Assert(
+                                valueSequence.ToArray()[payloadLength - 1] == (byte)'"',
+                                $"Calculated sequence ends with {valueSequence.ToArray()[payloadLength - 1]}");
                         }
-                    default:
-                        {
-                            byte displayByte;
 
-                            if (reader.HasValueSequence)
-                            {
-                                displayByte = reader.ValueSequence.First.Span[0];
-                            }
-                            else
-                            {
-                                displayByte = reader.ValueSpan[0];
-                            }
+                        break;
 
-                            ThrowHelper.ThrowJsonReaderException(
-                                ref reader,
-                                ExceptionResource.ExpectedStartOfValueNotFound,
-                                displayByte);
+                    default:
+                        byte displayByte = reader.HasValueSequence
+                            ? reader.ValueSequence.First.Span[0]
+                            : reader.ValueSpan[0];
 
-                            break;
-                        }
+                        ThrowHelper.ThrowJsonReaderException(
+                            ref reader,
+                            ExceptionResource.ExpectedStartOfValueNotFound,
+                            displayByte);
+
+                        break;
                 }
             }
             catch (JsonReaderException ex)
             {
-                reader = restore;
                 // Re-throw with Path information.
                 ThrowHelper.ReThrowWithPath(ref state, ex);
             }
 
-            int length = valueSpan.IsEmpty ? checked((int)valueSequence.Length) : valueSpan.Length;
-            byte[] rented = ArrayPool<byte>.Shared.Rent(length);
-            Span<byte> rentedSpan = rented.AsSpan(0, length);
-
-            try
-            {
-                if (valueSpan.IsEmpty)
-                {
-                    valueSequence.CopyTo(rentedSpan);
-                }
-                else
-                {
-                    valueSpan.CopyTo(rentedSpan);
-                }
-
-                JsonReaderOptions originalReaderOptions = readerState.Options;
-
-                var newReader = new Utf8JsonReader(rentedSpan, originalReaderOptions);
+            Debug.Assert(!valueSpan.IsEmpty ^ !valueSequence.IsEmpty);
 
-                TValue? value = ReadCore<TValue>(ref newReader, jsonTypeInfo, ref state);
-
-                // The reader should have thrown if we have remaining bytes.
-                Debug.Assert(newReader.BytesConsumed == length);
-
-                return value;
-            }
-            catch (JsonException)
-            {
-                reader = restore;
-                throw;
-            }
-            finally
-            {
-                rentedSpan.Clear();
-                ArrayPool<byte>.Shared.Return(rented);
-            }
+            return valueSpan.IsEmpty
+                ? new Utf8JsonReader(valueSequence, reader.CurrentState.Options)
+                : new Utf8JsonReader(valueSpan, reader.CurrentState.Options);
         }
     }
 }
index 66384b0..22d3395 100644 (file)
@@ -23,6 +23,12 @@ namespace System.Text.Json
         }
 
         [DoesNotReturn]
+        public static void ThrowArgumentException_SerializerDoesNotSupportComments(string paramName)
+        {
+            throw new ArgumentException(SR.JsonSerializerDoesNotSupportComments, paramName);
+        }
+
+        [DoesNotReturn]
         public static void ThrowNotSupportedException_SerializationNotSupported(Type propertyType)
         {
             throw new NotSupportedException(SR.Format(SR.SerializationNotSupportedType, propertyType));