From d3591191dac8f13dcf03d13b2426c6971ba04087 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Fri, 18 Sep 2020 15:39:35 -0500 Subject: [PATCH 1/8] Improve performance of polymorphism --- .../Json/Document/JsonDocument.MetadataDb.cs | 76 +++++++++------- .../Text/Json/Document/JsonDocument.Parse.cs | 87 +++++++++++++------ .../System/Text/Json/Document/JsonDocument.cs | 6 +- .../System/Text/Json/Document/JsonElement.cs | 50 +++++++++++ .../Converters/Value/JsonElementConverter.cs | 5 +- .../Converters/Value/ObjectConverter.cs | 5 +- .../Json/Serialization/WriteStackFrame.cs | 14 +-- 7 files changed, 166 insertions(+), 77 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs index 4e0f7b3652778..10957e73cf6d3 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs @@ -87,7 +87,7 @@ private struct MetadataDb : IDisposable internal int Length { get; private set; } private byte[] _data; #if DEBUG - private readonly bool _isLocked; + private bool _isLocked; #endif internal MetadataDb(byte[] completeDb) @@ -127,25 +127,6 @@ internal MetadataDb(int payloadLength) #endif } - internal MetadataDb(MetadataDb source, bool useArrayPools) - { - Length = source.Length; - -#if DEBUG - _isLocked = !useArrayPools; -#endif - - if (useArrayPools) - { - _data = ArrayPool.Shared.Rent(Length); - source._data.AsSpan(0, Length).CopyTo(_data); - } - else - { - _data = source._data.AsSpan(0, Length).ToArray(); - } - } - public void Dispose() { byte[]? data = Interlocked.Exchange(ref _data, null!); @@ -165,23 +146,52 @@ public void Dispose() Length = 0; } - internal void TrimExcess() + /// + /// If using array pools, trim excess if necessary. + /// If not using array pools, release the temporary array pool and alloc. + /// + /// + internal void CompleteAllocations(bool useArrayPools) { - // There's a chance that the size we have is the size we'd get for this - // amount of usage (particularly if Enlarge ever got called); and there's - // the small copy-cost associated with trimming anyways. "Is half-empty" is - // just a rough metric for "is trimming worth it?". - if (Length <= _data.Length / 2) + if (useArrayPools) { - byte[] newRent = ArrayPool.Shared.Rent(Length); - byte[] returnBuf = newRent; - - if (newRent.Length < _data.Length) + // There's a chance that the size we have is the size we'd get for this + // amount of usage (particularly if Enlarge ever got called); and there's + // the small copy-cost associated with trimming anyways. "Is half-empty" is + // just a rough metric for "is trimming worth it?". + if (Length <= _data.Length / 2) { - Buffer.BlockCopy(_data, 0, newRent, 0, Length); - returnBuf = _data; - _data = newRent; + byte[] newRent = ArrayPool.Shared.Rent(Length); + byte[] returnBuf = newRent; + + if (newRent.Length < _data.Length) + { + Buffer.BlockCopy(_data, 0, newRent, 0, Length); + returnBuf = _data; + _data = newRent; + } + + // The data in this rented buffer only conveys the positions and + // lengths of tokens in a document, but no content; so it does not + // need to be cleared. + ArrayPool.Shared.Return(returnBuf); } + } + else + { + // This is faster and allocates less than creating a new MetadataDb. + // This is used internally by JsonElement.Parse() so there are no concurrency concerns. +#if DEBUG + Debug.Assert(!_isLocked); +#endif + + Debug.Assert(_data != null); + byte[] returnBuf = _data; + _data = _data.AsSpan(0, Length).ToArray(); + +#if DEBUG + _isLocked = true; +#endif // The data in this rented buffer only conveys the positions and // lengths of tokens in a document, but no content; so it does not diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.Parse.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.Parse.cs index 2c6b46ee2240c..4ee21aba6966f 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.Parse.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.Parse.cs @@ -41,7 +41,7 @@ public sealed partial class JsonDocument /// public static JsonDocument Parse(ReadOnlyMemory utf8Json, JsonDocumentOptions options = default) { - return Parse(utf8Json, options.GetReaderOptions(), null); + return Parse(utf8Json, options.GetReaderOptions(), null, useArrayPools: true); } /// @@ -75,7 +75,7 @@ public static JsonDocument Parse(ReadOnlySequence utf8Json, JsonDocumentOp if (utf8Json.IsSingleSegment) { - return Parse(utf8Json.First, readerOptions, null); + return Parse(utf8Json.First, readerOptions, null, useArrayPools: true); } int length = checked((int)utf8Json.Length); @@ -84,7 +84,7 @@ public static JsonDocument Parse(ReadOnlySequence utf8Json, JsonDocumentOp try { utf8Json.CopyTo(utf8Bytes.AsSpan()); - return Parse(utf8Bytes.AsMemory(0, length), readerOptions, utf8Bytes); + return Parse(utf8Bytes.AsMemory(0, length), readerOptions, utf8Bytes, useArrayPools: true); } catch { @@ -121,7 +121,7 @@ public static JsonDocument Parse(Stream utf8Json, JsonDocumentOptions options = Debug.Assert(drained.Array != null); try { - return Parse(drained.AsMemory(), options.GetReaderOptions(), drained.Array); + return Parse(drained.AsMemory(), options.GetReaderOptions(), drained.Array, useArrayPools: true); } catch { @@ -170,7 +170,7 @@ private static async Task ParseAsyncCore( Debug.Assert(drained.Array != null); try { - return Parse(drained.AsMemory(), options.GetReaderOptions(), drained.Array); + return Parse(drained.AsMemory(), options.GetReaderOptions(), drained.Array, useArrayPools: true); } catch { @@ -211,7 +211,11 @@ public static JsonDocument Parse(ReadOnlyMemory json, JsonDocumentOptions int actualByteCount = JsonReaderHelper.GetUtf8FromText(jsonChars, utf8Bytes); Debug.Assert(expectedByteCount == actualByteCount); - return Parse(utf8Bytes.AsMemory(0, actualByteCount), options.GetReaderOptions(), utf8Bytes); + return Parse( + utf8Bytes.AsMemory(0, actualByteCount), + options.GetReaderOptions(), + utf8Bytes, + useArrayPools: true); } catch { @@ -286,7 +290,7 @@ public static JsonDocument Parse(string json, JsonDocumentOptions options = defa /// public static bool TryParseValue(ref Utf8JsonReader reader, [NotNullWhen(true)] out JsonDocument? document) { - return TryParseValue(ref reader, out document, shouldThrow: false); + return TryParseValue(ref reader, out document, shouldThrow: false, useArrayPools: true); } /// @@ -326,12 +330,21 @@ public static bool TryParseValue(ref Utf8JsonReader reader, [NotNullWhen(true)] /// public static JsonDocument ParseValue(ref Utf8JsonReader reader) { - bool ret = TryParseValue(ref reader, out JsonDocument? document, shouldThrow: true); + bool ret = TryParseValue( + ref reader, + out JsonDocument? document, + shouldThrow: true, + useArrayPools: true); + Debug.Assert(ret, "TryParseValue returned false with shouldThrow: true."); return document!; } - private static bool TryParseValue(ref Utf8JsonReader reader, [NotNullWhen(true)] out JsonDocument? document, bool shouldThrow) + internal static bool TryParseValue( + ref Utf8JsonReader reader, + [NotNullWhen(true)] out JsonDocument? document, + bool shouldThrow, + bool useArrayPools) { JsonReaderState state = reader.CurrentState; CheckSupportedOptions(state.Options, nameof(reader)); @@ -507,38 +520,58 @@ private static bool TryParseValue(ref Utf8JsonReader reader, [NotNullWhen(true)] } int length = valueSpan.IsEmpty ? checked((int)valueSequence.Length) : valueSpan.Length; - byte[] rented = ArrayPool.Shared.Rent(length); - Span rentedSpan = rented.AsSpan(0, length); + if (useArrayPools) + { + byte[] rented = ArrayPool.Shared.Rent(length); + Span rentedSpan = rented.AsSpan(0, length); - try + try + { + if (valueSpan.IsEmpty) + { + valueSequence.CopyTo(rentedSpan); + } + else + { + valueSpan.CopyTo(rentedSpan); + } + + document = Parse(rented.AsMemory(0, length), state.Options, rented, useArrayPools); + } + catch + { + // This really shouldn't happen since the document was already checked + // for consistency by Skip. But if data mutations happened just after + // the calls to Read then the copy may not be valid. + rentedSpan.Clear(); + ArrayPool.Shared.Return(rented); + throw; + } + } + else { + byte[] utf8Json = new byte[length]; + if (valueSpan.IsEmpty) { - valueSequence.CopyTo(rentedSpan); + valueSequence.CopyTo(utf8Json); } else { - valueSpan.CopyTo(rentedSpan); + valueSpan.CopyTo(utf8Json); } - document = Parse(rented.AsMemory(0, length), state.Options, rented); - return true; - } - catch - { - // This really shouldn't happen since the document was already checked - // for consistency by Skip. But if data mutations happened just after - // the calls to Read then the copy may not be valid. - rentedSpan.Clear(); - ArrayPool.Shared.Return(rented); - throw; + document = Parse(utf8Json, state.Options, extraRentedBytes: null, useArrayPools); } + + return true; } private static JsonDocument Parse( ReadOnlyMemory utf8Json, JsonReaderOptions readerOptions, - byte[]? extraRentedBytes) + byte[]? extraRentedBytes, + bool useArrayPools) { ReadOnlySpan utf8JsonSpan = utf8Json.Span; var database = new MetadataDb(utf8Json.Length); @@ -546,7 +579,7 @@ private static JsonDocument Parse( try { - Parse(utf8JsonSpan, readerOptions, ref database, ref stack); + Parse(utf8JsonSpan, readerOptions, ref database, ref stack, useArrayPools); } catch { diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs index 5a472f5c2607e..3b8a90650b174 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs @@ -922,7 +922,8 @@ private static void Parse( ReadOnlySpan utf8JsonSpan, JsonReaderOptions readerOptions, ref MetadataDb database, - ref StackRowStack stack) + ref StackRowStack stack, + bool useArrayPools) { bool inArray = false; int arrayItemsCount = 0; @@ -1068,7 +1069,8 @@ private static void Parse( } Debug.Assert(reader.BytesConsumed == utf8JsonSpan.Length); - database.TrimExcess(); + + database.CompleteAllocations(useArrayPools); } private void CheckNotDisposed() diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonElement.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonElement.cs index c339aa766c92d..27225f6fbf886 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonElement.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonElement.cs @@ -27,6 +27,56 @@ internal JsonElement(JsonDocument parent, int idx) _idx = idx; } + /// + /// Parses one JSON value (including objects or arrays) from the provided reader. + /// For performance, the JsonDocument Parse() methods should be used instead when the + /// Dispose pattern is applicable. + /// + /// The reader to read. + /// + /// A JsonElement representing the value (and nested values) read from the reader. + /// + /// + /// + /// If the property of + /// is or , the + /// reader will be advanced by one call to to determine + /// the start of the value. + /// + /// + /// + /// Upon completion of this method will be positioned at the + /// final token in the JSON value. If an exception is thrown the reader is reset to + /// the state it was in when the method was called. + /// + /// + /// + /// This method makes a copy of the data the reader acted on, so there is no caller + /// requirement to maintain data integrity beyond the return of this method. + /// + /// + /// + /// is using unsupported options. + /// + /// + /// The current token does not start or represent a value. + /// + /// + /// A value could not be read from the reader. + /// + + internal static JsonElement Parse(ref Utf8JsonReader reader) + { + bool ret = JsonDocument.TryParseValue( + ref reader, + out JsonDocument? document, + shouldThrow: true, + useArrayPools: false); + + Debug.Assert(ret, "Parse returned false with shouldThrow: true."); + return document!.RootElement; + } + [DebuggerBrowsable(DebuggerBrowsableState.Never)] private JsonTokenType TokenType { diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/JsonElementConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/JsonElementConverter.cs index a3a0d1ede1372..49da9e7fb1606 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/JsonElementConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/JsonElementConverter.cs @@ -7,10 +7,7 @@ internal sealed class JsonElementConverter : JsonConverter { public override JsonElement Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { - using (JsonDocument document = JsonDocument.ParseValue(ref reader)) - { - return document.RootElement.Clone(); - } + return JsonElement.Parse(ref reader); } public override void Write(Utf8JsonWriter writer, JsonElement value, JsonSerializerOptions options) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/ObjectConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/ObjectConverter.cs index a9feb74b5c6c1..3657e36304c94 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/ObjectConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/ObjectConverter.cs @@ -12,10 +12,7 @@ public ObjectConverter() public override object Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { - using (JsonDocument document = JsonDocument.ParseValue(ref reader)) - { - return document.RootElement.Clone(); - } + return JsonElement.Parse(ref reader); } public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOptions options) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs index d5a671dd4794f..ebf8885bd6825 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs @@ -94,16 +94,16 @@ public JsonPropertyInfo GetPolymorphicJsonPropertyInfo() } /// - /// Initializes the state for polymorphic or re-entry cases. + /// Initializes the state for polymorphic cases. /// - public JsonConverter InitializeReEntry(Type type, JsonSerializerOptions options, string? propertyName = null) + public JsonConverter InitializeReEntry(Type type, JsonSerializerOptions options) { - JsonClassInfo classInfo = options.GetOrAddClass(type); + if (PolymorphicJsonPropertyInfo?.RuntimePropertyType != type) + { + JsonClassInfo classInfo = options.GetOrAddClass(type); + PolymorphicJsonPropertyInfo = classInfo.PropertyInfoForClassInfo; + } - // Set for exception handling calculation of JsonPath. - JsonPropertyNameAsString = propertyName; - - PolymorphicJsonPropertyInfo = classInfo.PropertyInfoForClassInfo; return PolymorphicJsonPropertyInfo.ConverterBase; } From af01d59f6ed51dbef340625b1574e8d205774454 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Fri, 25 Sep 2020 13:20:11 -0500 Subject: [PATCH 2/8] Add additional caching; avoid more allocs; feedback --- .../Json/Document/JsonDocument.MetadataDb.cs | 121 +++++++------- .../Text/Json/Document/JsonDocument.Parse.cs | 149 +++++++++++++++--- .../Document/JsonDocument.StackRowStack.cs | 21 ++- .../System/Text/Json/Document/JsonDocument.cs | 7 +- .../System/Text/Json/Document/JsonElement.cs | 12 +- .../Converters/Value/ObjectConverter.cs | 5 +- 6 files changed, 210 insertions(+), 105 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs index 10957e73cf6d3..ac7bc29554adf 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs @@ -86,29 +86,34 @@ private struct MetadataDb : IDisposable internal int Length { get; private set; } private byte[] _data; -#if DEBUG - private bool _isLocked; -#endif + private bool _convertToAlloc; // Convert the rented data to an alloc when complete. + private bool _isLocked; // Is the array the correct fixed size. + + private MetadataDb(byte[] initialDb, bool isLocked, bool convertToAlloc) + { + _data = initialDb; + _isLocked = isLocked; + _convertToAlloc = convertToAlloc; + Length = 0; + } internal MetadataDb(byte[] completeDb) { _data = completeDb; - Length = completeDb.Length; - -#if DEBUG _isLocked = true; -#endif + _convertToAlloc = false; + Length = completeDb.Length; } - internal MetadataDb(int payloadLength) + internal static MetadataDb CreateRented(int payloadLength, bool convertToAlloc) { // Assume that a token happens approximately every 12 bytes. // int estimatedTokens = payloadLength / 12 // now acknowledge that the number of bytes we need per token is 12. // So that's just the payload length. // - // Add one token's worth of data just because. - int initialSize = DbRow.Size + payloadLength; + // Add one row worth of data since we need at least one row for a primitive type. + int initialSize = payloadLength + DbRow.Size; // Stick with ArrayPool's rent/return range if it looks feasible. // If it's wrong, we'll just grow and copy as we would if the tokens @@ -120,11 +125,17 @@ internal MetadataDb(int payloadLength) initialSize = OneMegabyte; } - _data = ArrayPool.Shared.Rent(initialSize); - Length = 0; -#if DEBUG - _isLocked = false; -#endif + byte[] data = ArrayPool.Shared.Rent(initialSize); + return new MetadataDb(data, isLocked: false, convertToAlloc); + } + + internal static MetadataDb CreateLocked(int payloadLength) + { + // Add one row worth of data since we need at least one row for a primitive type. + int size = payloadLength + DbRow.Size; + + byte[] data = new byte[size]; + return new MetadataDb(data, isLocked: true, convertToAlloc: false); } public void Dispose() @@ -135,9 +146,7 @@ public void Dispose() return; } -#if DEBUG Debug.Assert(!_isLocked, "Dispose called on a locked database"); -#endif // The data in this rented buffer only conveys the positions and // lengths of tokens in a document, but no content; so it does not @@ -150,53 +159,51 @@ public void Dispose() /// If using array pools, trim excess if necessary. /// If not using array pools, release the temporary array pool and alloc. /// - /// - internal void CompleteAllocations(bool useArrayPools) + internal void CompleteAllocations() { - if (useArrayPools) + if (!_isLocked) { - // There's a chance that the size we have is the size we'd get for this - // amount of usage (particularly if Enlarge ever got called); and there's - // the small copy-cost associated with trimming anyways. "Is half-empty" is - // just a rough metric for "is trimming worth it?". - if (Length <= _data.Length / 2) + if (_convertToAlloc) { - byte[] newRent = ArrayPool.Shared.Rent(Length); - byte[] returnBuf = newRent; + // Mutate the _data. This is faster and allocates less than creating a new MetadataDb. + // This is used by JsonElement.Parse() so there are no concurrency concerns since the element + // is not returned until it is done parsing. - if (newRent.Length < _data.Length) - { - Buffer.BlockCopy(_data, 0, newRent, 0, Length); - returnBuf = _data; - _data = newRent; - } + Debug.Assert(_data != null); + byte[] returnBuf = _data; + _data = _data.AsSpan(0, Length).ToArray(); + _isLocked = true; + _convertToAlloc = false; // The data in this rented buffer only conveys the positions and // lengths of tokens in a document, but no content; so it does not // need to be cleared. ArrayPool.Shared.Return(returnBuf); } - } - else - { - // This is faster and allocates less than creating a new MetadataDb. - // This is used internally by JsonElement.Parse() so there are no concurrency concerns. -#if DEBUG - Debug.Assert(!_isLocked); -#endif - - Debug.Assert(_data != null); - byte[] returnBuf = _data; - _data = _data.AsSpan(0, Length).ToArray(); - -#if DEBUG - _isLocked = true; -#endif - - // The data in this rented buffer only conveys the positions and - // lengths of tokens in a document, but no content; so it does not - // need to be cleared. - ArrayPool.Shared.Return(returnBuf); + else + { + // There's a chance that the size we have is the size we'd get for this + // amount of usage (particularly if Enlarge ever got called); and there's + // the small copy-cost associated with trimming anyways. "Is half-empty" is + // just a rough metric for "is trimming worth it?". + if (Length <= _data.Length / 2) + { + byte[] newRent = ArrayPool.Shared.Rent(Length); + byte[] returnBuf = newRent; + + if (newRent.Length < _data.Length) + { + Buffer.BlockCopy(_data, 0, newRent, 0, Length); + returnBuf = _data; + _data = newRent; + } + + // The data in this rented buffer only conveys the positions and + // lengths of tokens in a document, but no content; so it does not + // need to be cleared. + ArrayPool.Shared.Return(returnBuf); + } + } } } @@ -207,10 +214,6 @@ internal void Append(JsonTokenType tokenType, int startLocation, int length) (tokenType == JsonTokenType.StartArray || tokenType == JsonTokenType.StartObject) == (length == DbRow.UnknownSize)); -#if DEBUG - Debug.Assert(!_isLocked, "Appending to a locked database"); -#endif - if (Length >= _data.Length - DbRow.Size) { Enlarge(); @@ -223,6 +226,8 @@ internal void Append(JsonTokenType tokenType, int startLocation, int length) private void Enlarge() { + Debug.Assert(!_isLocked, "Appending to a locked database"); + byte[] toReturn = _data; _data = ArrayPool.Shared.Rent(toReturn.Length * 2); Buffer.BlockCopy(toReturn, 0, _data, 0, toReturn.Length); diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.Parse.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.Parse.cs index 4ee21aba6966f..0d6ac5589aaf3 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.Parse.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.Parse.cs @@ -12,6 +12,11 @@ namespace System.Text.Json { public sealed partial class JsonDocument { + // Cached unrented documents for literal values. + private static JsonDocument? s_nullLiteral; + private static JsonDocument? s_trueLiteral; + private static JsonDocument? s_falseLiteral; + private const int UnseekableStreamInitialRentSize = 4096; /// @@ -41,7 +46,7 @@ public sealed partial class JsonDocument /// public static JsonDocument Parse(ReadOnlyMemory utf8Json, JsonDocumentOptions options = default) { - return Parse(utf8Json, options.GetReaderOptions(), null, useArrayPools: true); + return Parse(utf8Json, options.GetReaderOptions(), null); } /// @@ -75,7 +80,7 @@ public static JsonDocument Parse(ReadOnlySequence utf8Json, JsonDocumentOp if (utf8Json.IsSingleSegment) { - return Parse(utf8Json.First, readerOptions, null, useArrayPools: true); + return Parse(utf8Json.First, readerOptions, null); } int length = checked((int)utf8Json.Length); @@ -84,7 +89,7 @@ public static JsonDocument Parse(ReadOnlySequence utf8Json, JsonDocumentOp try { utf8Json.CopyTo(utf8Bytes.AsSpan()); - return Parse(utf8Bytes.AsMemory(0, length), readerOptions, utf8Bytes, useArrayPools: true); + return Parse(utf8Bytes.AsMemory(0, length), readerOptions, utf8Bytes); } catch { @@ -121,7 +126,7 @@ public static JsonDocument Parse(Stream utf8Json, JsonDocumentOptions options = Debug.Assert(drained.Array != null); try { - return Parse(drained.AsMemory(), options.GetReaderOptions(), drained.Array, useArrayPools: true); + return Parse(drained.AsMemory(), options.GetReaderOptions(), drained.Array); } catch { @@ -170,7 +175,7 @@ private static async Task ParseAsyncCore( Debug.Assert(drained.Array != null); try { - return Parse(drained.AsMemory(), options.GetReaderOptions(), drained.Array, useArrayPools: true); + return Parse(drained.AsMemory(), options.GetReaderOptions(), drained.Array); } catch { @@ -214,8 +219,7 @@ public static JsonDocument Parse(ReadOnlyMemory json, JsonDocumentOptions return Parse( utf8Bytes.AsMemory(0, actualByteCount), options.GetReaderOptions(), - utf8Bytes, - useArrayPools: true); + utf8Bytes); } catch { @@ -330,11 +334,7 @@ public static bool TryParseValue(ref Utf8JsonReader reader, [NotNullWhen(true)] /// public static JsonDocument ParseValue(ref Utf8JsonReader reader) { - bool ret = TryParseValue( - ref reader, - out JsonDocument? document, - shouldThrow: true, - useArrayPools: true); + bool ret = TryParseValue(ref reader, out JsonDocument? document, shouldThrow: true, useArrayPools: true); Debug.Assert(ret, "TryParseValue returned false with shouldThrow: true."); return document!; @@ -380,6 +380,7 @@ internal static bool TryParseValue( document = null; return false; } + break; } } @@ -427,11 +428,64 @@ internal static bool TryParseValue( break; } - // Single-token values - case JsonTokenType.Number: + case JsonTokenType.Null: + if (useArrayPools) + { + if (reader.HasValueSequence) + { + valueSequence = reader.ValueSequence; + } + else + { + valueSpan = reader.ValueSpan; + } + + break; + } + + s_nullLiteral ??= CreateForLiteral(JsonConstants.NullValue.ToArray(), reader.TokenType); + document = s_nullLiteral; + return true; + case JsonTokenType.True: + if (useArrayPools) + { + if (reader.HasValueSequence) + { + valueSequence = reader.ValueSequence; + } + else + { + valueSpan = reader.ValueSpan; + } + + break; + } + + s_trueLiteral ??= CreateForLiteral(JsonConstants.TrueValue.ToArray(), reader.TokenType); + document = s_trueLiteral; + return true; + case JsonTokenType.False: - case JsonTokenType.Null: + if (useArrayPools) + { + if (reader.HasValueSequence) + { + valueSequence = reader.ValueSequence; + } + else + { + valueSpan = reader.ValueSpan; + } + + break; + } + + s_falseLiteral ??= CreateForLiteral(JsonConstants.FalseValue.ToArray(), reader.TokenType); + document = s_falseLiteral; + return true; + + case JsonTokenType.Number: { if (reader.HasValueSequence) { @@ -444,6 +498,7 @@ internal static bool TryParseValue( break; } + // String's ValueSequence/ValueSpan omits the quotes, we need them back. case JsonTokenType.String: { @@ -536,7 +591,7 @@ internal static bool TryParseValue( valueSpan.CopyTo(rentedSpan); } - document = Parse(rented.AsMemory(0, length), state.Options, rented, useArrayPools); + document = Parse(rented.AsMemory(0, length), state.Options, rented); } catch { @@ -550,36 +605,42 @@ internal static bool TryParseValue( } else { - byte[] utf8Json = new byte[length]; + byte[] owned; if (valueSpan.IsEmpty) { - valueSequence.CopyTo(utf8Json); + owned = valueSequence.ToArray(); } else { - valueSpan.CopyTo(utf8Json); + owned = valueSpan.ToArray(); } - document = Parse(utf8Json, state.Options, extraRentedBytes: null, useArrayPools); + document = ParseUnrented(owned, state.Options, reader.TokenType); } return true; } + private static JsonDocument CreateForLiteral(byte[] utf8Json, JsonTokenType tokenType) + { + MetadataDb database = MetadataDb.CreateLocked(utf8Json.Length); + database.Append(tokenType, startLocation: 0, utf8Json.Length); + return new JsonDocument(utf8Json, database, extraRentedBytes: null); + } + private static JsonDocument Parse( ReadOnlyMemory utf8Json, JsonReaderOptions readerOptions, - byte[]? extraRentedBytes, - bool useArrayPools) + byte[]? extraRentedBytes) { ReadOnlySpan utf8JsonSpan = utf8Json.Span; - var database = new MetadataDb(utf8Json.Length); + var database = MetadataDb.CreateRented(utf8Json.Length, convertToAlloc: false); var stack = new StackRowStack(JsonDocumentOptions.DefaultMaxDepth * StackRow.Size); try { - Parse(utf8JsonSpan, readerOptions, ref database, ref stack, useArrayPools); + Parse(utf8JsonSpan, readerOptions, ref database, ref stack); } catch { @@ -594,6 +655,46 @@ private static JsonDocument Parse( return new JsonDocument(utf8Json, database, extraRentedBytes); } + private static JsonDocument ParseUnrented( + ReadOnlyMemory utf8Json, + JsonReaderOptions readerOptions, + JsonTokenType tokenType) + { + // These tokens should already have been processed. + Debug.Assert( + tokenType != JsonTokenType.Null && + tokenType != JsonTokenType.False && + tokenType != JsonTokenType.True); + + ReadOnlySpan utf8JsonSpan = utf8Json.Span; + MetadataDb database; + + if (tokenType == JsonTokenType.String || + tokenType == JsonTokenType.Number) + { + // For primitive types, we can avoid renting and there is no need for a StackRowStack. + database = MetadataDb.CreateLocked(utf8Json.Length); + var _ = new StackRowStack(initialSize: -1); + Parse(utf8JsonSpan, readerOptions, ref database, ref _); + } + else + { + database = MetadataDb.CreateRented(utf8Json.Length, convertToAlloc: true); + var stack = new StackRowStack(JsonDocumentOptions.DefaultMaxDepth * StackRow.Size); + + try + { + Parse(utf8JsonSpan, readerOptions, ref database, ref stack); + } + finally + { + stack.Dispose(); + } + } + + return new JsonDocument(utf8Json, database, extraRentedBytes: null); + } + private static ArraySegment ReadToEnd(Stream stream) { int written = 0; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.StackRowStack.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.StackRowStack.cs index 4978148a819f8..d6df915cbaae1 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.StackRowStack.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.StackRowStack.cs @@ -11,18 +11,27 @@ public sealed partial class JsonDocument { private struct StackRowStack : IDisposable { - private byte[] _rentedBuffer; + private byte[]? _rentedBuffer; private int _topOfStack; internal StackRowStack(int initialSize) { - _rentedBuffer = ArrayPool.Shared.Rent(initialSize); - _topOfStack = _rentedBuffer.Length; + if (initialSize == -1) + { + // Used for primitive values that don't need a stack. + _rentedBuffer = null; + _topOfStack = 0; + } + else + { + _rentedBuffer = ArrayPool.Shared.Rent(initialSize); + _topOfStack = _rentedBuffer.Length; + } } public void Dispose() { - byte[] toReturn = _rentedBuffer; + byte[]? toReturn = _rentedBuffer; _rentedBuffer = null!; _topOfStack = 0; @@ -48,7 +57,7 @@ internal void Push(StackRow row) internal StackRow Pop() { - Debug.Assert(_topOfStack <= _rentedBuffer.Length - StackRow.Size); + Debug.Assert(_topOfStack <= _rentedBuffer!.Length - StackRow.Size); StackRow row = MemoryMarshal.Read(_rentedBuffer.AsSpan(_topOfStack)); _topOfStack += StackRow.Size; return row; @@ -56,7 +65,7 @@ internal StackRow Pop() private void Enlarge() { - byte[] toReturn = _rentedBuffer; + byte[] toReturn = _rentedBuffer!; _rentedBuffer = ArrayPool.Shared.Rent(toReturn.Length * 2); Buffer.BlockCopy( diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs index 3b8a90650b174..0b23787ece7a7 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs @@ -1,12 +1,10 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; using System.Buffers; using System.Buffers.Text; using System.Diagnostics; using System.Runtime.InteropServices; -using System.Runtime.CompilerServices; using System.Threading; using System.Diagnostics.CodeAnalysis; @@ -922,8 +920,7 @@ private static void Parse( ReadOnlySpan utf8JsonSpan, JsonReaderOptions readerOptions, ref MetadataDb database, - ref StackRowStack stack, - bool useArrayPools) + ref StackRowStack stack) { bool inArray = false; int arrayItemsCount = 0; @@ -1070,7 +1067,7 @@ private static void Parse( Debug.Assert(reader.BytesConsumed == utf8JsonSpan.Length); - database.CompleteAllocations(useArrayPools); + database.CompleteAllocations(); } private void CheckNotDisposed() diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonElement.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonElement.cs index 27225f6fbf886..2b86b5e024809 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonElement.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonElement.cs @@ -29,8 +29,9 @@ internal JsonElement(JsonDocument parent, int idx) /// /// Parses one JSON value (including objects or arrays) from the provided reader. - /// For performance, the JsonDocument Parse() methods should be used instead when the - /// Dispose pattern is applicable. + /// This is intended to be used when the pattern is not + /// applicable. If the Dispose pattern is applicable, then use the + /// Parse methods instead since they have better performance. /// /// The reader to read. /// @@ -64,14 +65,9 @@ internal JsonElement(JsonDocument parent, int idx) /// /// A value could not be read from the reader. /// - internal static JsonElement Parse(ref Utf8JsonReader reader) { - bool ret = JsonDocument.TryParseValue( - ref reader, - out JsonDocument? document, - shouldThrow: true, - useArrayPools: false); + bool ret = JsonDocument.TryParseValue(ref reader, out JsonDocument? document, shouldThrow: true, useArrayPools: false); Debug.Assert(ret, "Parse returned false with shouldThrow: true."); return document!.RootElement; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/ObjectConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/ObjectConverter.cs index 3657e36304c94..5cf1dfb4766d5 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/ObjectConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/ObjectConverter.cs @@ -42,10 +42,7 @@ private JsonConverter GetRuntimeConverter(Type runtimeType, JsonSerializerOption internal override object ReadNumberWithCustomHandling(ref Utf8JsonReader reader, JsonNumberHandling handling) { - using (JsonDocument document = JsonDocument.ParseValue(ref reader)) - { - return document.RootElement.Clone(); - } + return JsonElement.Parse(ref reader); } } } From d034ae82d4d3c9e551669d0622548df1e2c72449 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Tue, 29 Sep 2020 09:56:39 -0500 Subject: [PATCH 3/8] Avoid some pool usage in non-serializer path; address feedback --- .../Json/Document/JsonDocument.MetadataDb.cs | 4 - .../Text/Json/Document/JsonDocument.Parse.cs | 89 ++----- .../Document/JsonDocument.StackRowStack.cs | 17 +- .../System/Text/Json/Document/JsonDocument.cs | 235 ++++++++++-------- .../System/Text/Json/Document/JsonElement.cs | 42 +--- .../Converters/Value/JsonElementConverter.cs | 2 +- .../Converters/Value/ObjectConverter.cs | 4 +- 7 files changed, 165 insertions(+), 228 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs index ac7bc29554adf..5c6f65069a9f0 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs @@ -165,10 +165,6 @@ internal void CompleteAllocations() { if (_convertToAlloc) { - // Mutate the _data. This is faster and allocates less than creating a new MetadataDb. - // This is used by JsonElement.Parse() so there are no concurrency concerns since the element - // is not returned until it is done parsing. - Debug.Assert(_data != null); byte[] returnBuf = _data; _data = _data.AsSpan(0, Length).ToArray(); diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.Parse.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.Parse.cs index 0d6ac5589aaf3..3d16406160df3 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.Parse.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.Parse.cs @@ -428,45 +428,9 @@ internal static bool TryParseValue( break; } - case JsonTokenType.Null: - if (useArrayPools) - { - if (reader.HasValueSequence) - { - valueSequence = reader.ValueSequence; - } - else - { - valueSpan = reader.ValueSpan; - } - - break; - } - - s_nullLiteral ??= CreateForLiteral(JsonConstants.NullValue.ToArray(), reader.TokenType); - document = s_nullLiteral; - return true; - - case JsonTokenType.True: - if (useArrayPools) - { - if (reader.HasValueSequence) - { - valueSequence = reader.ValueSequence; - } - else - { - valueSpan = reader.ValueSpan; - } - - break; - } - - s_trueLiteral ??= CreateForLiteral(JsonConstants.TrueValue.ToArray(), reader.TokenType); - document = s_trueLiteral; - return true; - case JsonTokenType.False: + case JsonTokenType.True: + case JsonTokenType.Null: if (useArrayPools) { if (reader.HasValueSequence) @@ -481,8 +445,7 @@ internal static bool TryParseValue( break; } - s_falseLiteral ??= CreateForLiteral(JsonConstants.FalseValue.ToArray(), reader.TokenType); - document = s_falseLiteral; + document = CreateForLiteral(reader.TokenType); return true; case JsonTokenType.Number: @@ -622,11 +585,28 @@ internal static bool TryParseValue( return true; } - private static JsonDocument CreateForLiteral(byte[] utf8Json, JsonTokenType tokenType) + private static JsonDocument CreateForLiteral(JsonTokenType tokenType) { - MetadataDb database = MetadataDb.CreateLocked(utf8Json.Length); - database.Append(tokenType, startLocation: 0, utf8Json.Length); - return new JsonDocument(utf8Json, database, extraRentedBytes: null); + switch (tokenType) + { + case JsonTokenType.False: + s_falseLiteral ??= Create(JsonConstants.FalseValue.ToArray()); + return s_falseLiteral; + case JsonTokenType.True: + s_trueLiteral ??= Create(JsonConstants.TrueValue.ToArray()); + return s_trueLiteral; + default: + Debug.Assert(tokenType == JsonTokenType.Null); + s_nullLiteral ??= Create(JsonConstants.NullValue.ToArray()); + return s_nullLiteral; + } + + JsonDocument Create(byte[] utf8Json) + { + MetadataDb database = MetadataDb.CreateLocked(utf8Json.Length); + database.Append(tokenType, startLocation: 0, utf8Json.Length); + return new JsonDocument(utf8Json, database, extraRentedBytes: null); + } } private static JsonDocument Parse( @@ -636,21 +616,16 @@ private static JsonDocument Parse( { ReadOnlySpan utf8JsonSpan = utf8Json.Span; var database = MetadataDb.CreateRented(utf8Json.Length, convertToAlloc: false); - var stack = new StackRowStack(JsonDocumentOptions.DefaultMaxDepth * StackRow.Size); try { - Parse(utf8JsonSpan, readerOptions, ref database, ref stack); + Parse(utf8JsonSpan, readerOptions, ref database); } catch { database.Dispose(); throw; } - finally - { - stack.Dispose(); - } return new JsonDocument(utf8Json, database, extraRentedBytes); } @@ -674,22 +649,12 @@ private static JsonDocument ParseUnrented( { // For primitive types, we can avoid renting and there is no need for a StackRowStack. database = MetadataDb.CreateLocked(utf8Json.Length); - var _ = new StackRowStack(initialSize: -1); - Parse(utf8JsonSpan, readerOptions, ref database, ref _); + Parse(utf8JsonSpan, readerOptions, ref database); } else { database = MetadataDb.CreateRented(utf8Json.Length, convertToAlloc: true); - var stack = new StackRowStack(JsonDocumentOptions.DefaultMaxDepth * StackRow.Size); - - try - { - Parse(utf8JsonSpan, readerOptions, ref database, ref stack); - } - finally - { - stack.Dispose(); - } + Parse(utf8JsonSpan, readerOptions, ref database); } return new JsonDocument(utf8Json, database, extraRentedBytes: null); diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.StackRowStack.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.StackRowStack.cs index d6df915cbaae1..924f43b2d2a5e 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.StackRowStack.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.StackRowStack.cs @@ -14,21 +14,14 @@ private struct StackRowStack : IDisposable private byte[]? _rentedBuffer; private int _topOfStack; - internal StackRowStack(int initialSize) + public void Initialize(int initialSize) { - if (initialSize == -1) - { - // Used for primitive values that don't need a stack. - _rentedBuffer = null; - _topOfStack = 0; - } - else - { - _rentedBuffer = ArrayPool.Shared.Rent(initialSize); - _topOfStack = _rentedBuffer.Length; - } + _rentedBuffer = ArrayPool.Shared.Rent(initialSize); + _topOfStack = _rentedBuffer.Length; } + public bool IsInitialized { get { return _rentedBuffer != null; } } + public void Dispose() { byte[]? toReturn = _rentedBuffer; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs index 0b23787ece7a7..fef00bc3b713a 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs @@ -919,133 +919,110 @@ private void WriteString(in DbRow row, Utf8JsonWriter writer) private static void Parse( ReadOnlySpan utf8JsonSpan, JsonReaderOptions readerOptions, - ref MetadataDb database, - ref StackRowStack stack) + ref MetadataDb database) { bool inArray = false; int arrayItemsCount = 0; int numberOfRowsForMembers = 0; int numberOfRowsForValues = 0; - Utf8JsonReader reader = new Utf8JsonReader( - utf8JsonSpan, - isFinalBlock: true, - new JsonReaderState(options: readerOptions)); - - while (reader.Read()) + StackRowStack stack = default; + using (stack) { - JsonTokenType tokenType = reader.TokenType; - - // Since the input payload is contained within a Span, - // token start index can never be larger than int.MaxValue (i.e. utf8JsonSpan.Length). - Debug.Assert(reader.TokenStartIndex <= int.MaxValue); - int tokenStart = (int)reader.TokenStartIndex; + Utf8JsonReader reader = new Utf8JsonReader( + utf8JsonSpan, + isFinalBlock: true, + new JsonReaderState(options: readerOptions)); - if (tokenType == JsonTokenType.StartObject) - { - if (inArray) - { - arrayItemsCount++; - } - - numberOfRowsForValues++; - database.Append(tokenType, tokenStart, DbRow.UnknownSize); - var row = new StackRow(numberOfRowsForMembers + 1); - stack.Push(row); - numberOfRowsForMembers = 0; - } - else if (tokenType == JsonTokenType.EndObject) + while (reader.Read()) { - int rowIndex = database.FindIndexOfFirstUnsetSizeOrLength(JsonTokenType.StartObject); + JsonTokenType tokenType = reader.TokenType; - numberOfRowsForValues++; - numberOfRowsForMembers++; - database.SetLength(rowIndex, numberOfRowsForMembers); + // Since the input payload is contained within a Span, + // token start index can never be larger than int.MaxValue (i.e. utf8JsonSpan.Length). + Debug.Assert(reader.TokenStartIndex <= int.MaxValue); + int tokenStart = (int)reader.TokenStartIndex; - int newRowIndex = database.Length; - database.Append(tokenType, tokenStart, reader.ValueSpan.Length); - database.SetNumberOfRows(rowIndex, numberOfRowsForMembers); - database.SetNumberOfRows(newRowIndex, numberOfRowsForMembers); - - StackRow row = stack.Pop(); - numberOfRowsForMembers += row.SizeOrLength; - } - else if (tokenType == JsonTokenType.StartArray) - { - if (inArray) + if (tokenType == JsonTokenType.StartObject) { - arrayItemsCount++; - } + if (inArray) + { + arrayItemsCount++; + } - numberOfRowsForMembers++; - database.Append(tokenType, tokenStart, DbRow.UnknownSize); - var row = new StackRow(arrayItemsCount, numberOfRowsForValues + 1); - stack.Push(row); - arrayItemsCount = 0; - numberOfRowsForValues = 0; - } - else if (tokenType == JsonTokenType.EndArray) - { - int rowIndex = database.FindIndexOfFirstUnsetSizeOrLength(JsonTokenType.StartArray); - - numberOfRowsForValues++; - numberOfRowsForMembers++; - database.SetLength(rowIndex, arrayItemsCount); - database.SetNumberOfRows(rowIndex, numberOfRowsForValues); - - // If the array item count is (e.g.) 12 and the number of rows is (e.g.) 13 - // then the extra row is just this EndArray item, so the array was made up - // of simple values. - // - // If the off-by-one relationship does not hold, then one of the values was - // more than one row, making it a complex object. - // - // This check is similar to tracking the start array and painting it when - // StartObject or StartArray is encountered, but avoids the mixed state - // where "UnknownSize" implies "has complex children". - if (arrayItemsCount + 1 != numberOfRowsForValues) - { - database.SetHasComplexChildren(rowIndex); + numberOfRowsForValues++; + database.Append(tokenType, tokenStart, DbRow.UnknownSize); + var row = new StackRow(numberOfRowsForMembers + 1); + Push(row); + numberOfRowsForMembers = 0; } + else if (tokenType == JsonTokenType.EndObject) + { + int rowIndex = database.FindIndexOfFirstUnsetSizeOrLength(JsonTokenType.StartObject); - int newRowIndex = database.Length; - database.Append(tokenType, tokenStart, reader.ValueSpan.Length); - database.SetNumberOfRows(newRowIndex, numberOfRowsForValues); - - StackRow row = stack.Pop(); - arrayItemsCount = row.SizeOrLength; - numberOfRowsForValues += row.NumberOfRows; - } - else if (tokenType == JsonTokenType.PropertyName) - { - numberOfRowsForValues++; - numberOfRowsForMembers++; - - // Adding 1 to skip the start quote will never overflow - Debug.Assert(tokenStart < int.MaxValue); + numberOfRowsForValues++; + numberOfRowsForMembers++; + database.SetLength(rowIndex, numberOfRowsForMembers); - database.Append(tokenType, tokenStart + 1, reader.ValueSpan.Length); + int newRowIndex = database.Length; + database.Append(tokenType, tokenStart, reader.ValueSpan.Length); + database.SetNumberOfRows(rowIndex, numberOfRowsForMembers); + database.SetNumberOfRows(newRowIndex, numberOfRowsForMembers); - if (reader._stringHasEscaping) + StackRow row = Pop(); + numberOfRowsForMembers += row.SizeOrLength; + } + else if (tokenType == JsonTokenType.StartArray) { - database.SetHasComplexChildren(database.Length - DbRow.Size); + if (inArray) + { + arrayItemsCount++; + } + + numberOfRowsForMembers++; + database.Append(tokenType, tokenStart, DbRow.UnknownSize); + var row = new StackRow(arrayItemsCount, numberOfRowsForValues + 1); + Push(row); + arrayItemsCount = 0; + numberOfRowsForValues = 0; } + else if (tokenType == JsonTokenType.EndArray) + { + int rowIndex = database.FindIndexOfFirstUnsetSizeOrLength(JsonTokenType.StartArray); + + numberOfRowsForValues++; + numberOfRowsForMembers++; + database.SetLength(rowIndex, arrayItemsCount); + database.SetNumberOfRows(rowIndex, numberOfRowsForValues); + + // If the array item count is (e.g.) 12 and the number of rows is (e.g.) 13 + // then the extra row is just this EndArray item, so the array was made up + // of simple values. + // + // If the off-by-one relationship does not hold, then one of the values was + // more than one row, making it a complex object. + // + // This check is similar to tracking the start array and painting it when + // StartObject or StartArray is encountered, but avoids the mixed state + // where "UnknownSize" implies "has complex children". + if (arrayItemsCount + 1 != numberOfRowsForValues) + { + database.SetHasComplexChildren(rowIndex); + } - Debug.Assert(!inArray); - } - else - { - Debug.Assert(tokenType >= JsonTokenType.String && tokenType <= JsonTokenType.Null); - numberOfRowsForValues++; - numberOfRowsForMembers++; + int newRowIndex = database.Length; + database.Append(tokenType, tokenStart, reader.ValueSpan.Length); + database.SetNumberOfRows(newRowIndex, numberOfRowsForValues); - if (inArray) - { - arrayItemsCount++; + StackRow row = Pop(); + arrayItemsCount = row.SizeOrLength; + numberOfRowsForValues += row.NumberOfRows; } - - if (tokenType == JsonTokenType.String) + else if (tokenType == JsonTokenType.PropertyName) { + numberOfRowsForValues++; + numberOfRowsForMembers++; + // Adding 1 to skip the start quote will never overflow Debug.Assert(tokenStart < int.MaxValue); @@ -1055,19 +1032,61 @@ private static void Parse( { database.SetHasComplexChildren(database.Length - DbRow.Size); } + + Debug.Assert(!inArray); } else { - database.Append(tokenType, tokenStart, reader.ValueSpan.Length); + Debug.Assert(tokenType >= JsonTokenType.String && tokenType <= JsonTokenType.Null); + numberOfRowsForValues++; + numberOfRowsForMembers++; + + if (inArray) + { + arrayItemsCount++; + } + + if (tokenType == JsonTokenType.String) + { + // Adding 1 to skip the start quote will never overflow + Debug.Assert(tokenStart < int.MaxValue); + + database.Append(tokenType, tokenStart + 1, reader.ValueSpan.Length); + + if (reader._stringHasEscaping) + { + database.SetHasComplexChildren(database.Length - DbRow.Size); + } + } + else + { + database.Append(tokenType, tokenStart, reader.ValueSpan.Length); + } } + + inArray = reader.IsInArray; } - inArray = reader.IsInArray; + Debug.Assert(reader.BytesConsumed == utf8JsonSpan.Length); + + database.CompleteAllocations(); } - Debug.Assert(reader.BytesConsumed == utf8JsonSpan.Length); + void Push(StackRow row) + { + if (!stack.IsInitialized) + { + stack.Initialize(JsonDocumentOptions.DefaultMaxDepth * StackRow.Size); + } - database.CompleteAllocations(); + stack.Push(row); + } + + StackRow Pop() + { + Debug.Assert(stack.IsInitialized); + return stack.Pop(); + } } private void CheckNotDisposed() diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonElement.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonElement.cs index 2b86b5e024809..5ca1a91406bf6 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonElement.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonElement.cs @@ -27,45 +27,9 @@ internal JsonElement(JsonDocument parent, int idx) _idx = idx; } - /// - /// Parses one JSON value (including objects or arrays) from the provided reader. - /// This is intended to be used when the pattern is not - /// applicable. If the Dispose pattern is applicable, then use the - /// Parse methods instead since they have better performance. - /// - /// The reader to read. - /// - /// A JsonElement representing the value (and nested values) read from the reader. - /// - /// - /// - /// If the property of - /// is or , the - /// reader will be advanced by one call to to determine - /// the start of the value. - /// - /// - /// - /// Upon completion of this method will be positioned at the - /// final token in the JSON value. If an exception is thrown the reader is reset to - /// the state it was in when the method was called. - /// - /// - /// - /// This method makes a copy of the data the reader acted on, so there is no caller - /// requirement to maintain data integrity beyond the return of this method. - /// - /// - /// - /// is using unsupported options. - /// - /// - /// The current token does not start or represent a value. - /// - /// - /// A value could not be read from the reader. - /// - internal static JsonElement Parse(ref Utf8JsonReader reader) + // Currently used only as an optimization by the serializer, which does not want to + // return elements that are based on the pattern. + internal static JsonElement ParseValue(ref Utf8JsonReader reader) { bool ret = JsonDocument.TryParseValue(ref reader, out JsonDocument? document, shouldThrow: true, useArrayPools: false); diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/JsonElementConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/JsonElementConverter.cs index 49da9e7fb1606..f371fe920a23c 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/JsonElementConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/JsonElementConverter.cs @@ -7,7 +7,7 @@ internal sealed class JsonElementConverter : JsonConverter { public override JsonElement Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { - return JsonElement.Parse(ref reader); + return JsonElement.ParseValue(ref reader); } public override void Write(Utf8JsonWriter writer, JsonElement value, JsonSerializerOptions options) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/ObjectConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/ObjectConverter.cs index 5cf1dfb4766d5..a014b08548b8a 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/ObjectConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/ObjectConverter.cs @@ -12,7 +12,7 @@ public ObjectConverter() public override object Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { - return JsonElement.Parse(ref reader); + return JsonElement.ParseValue(ref reader); } public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOptions options) @@ -42,7 +42,7 @@ private JsonConverter GetRuntimeConverter(Type runtimeType, JsonSerializerOption internal override object ReadNumberWithCustomHandling(ref Utf8JsonReader reader, JsonNumberHandling handling) { - return JsonElement.Parse(ref reader); + return JsonElement.ParseValue(ref reader); } } } From cafef3bcfde1e867ce92bd6ab5d1892f5d87727c Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Wed, 30 Sep 2020 09:53:42 -0500 Subject: [PATCH 4/8] Encapsulate logic better for StackRowStack --- .../Text/Json/Document/JsonDocument.Parse.cs | 5 ++- .../Document/JsonDocument.StackRowStack.cs | 10 +++++- .../System/Text/Json/Document/JsonDocument.cs | 34 +++++-------------- 3 files changed, 20 insertions(+), 29 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.Parse.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.Parse.cs index 3d16406160df3..f3b29f5824404 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.Parse.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.Parse.cs @@ -647,16 +647,15 @@ private static JsonDocument ParseUnrented( if (tokenType == JsonTokenType.String || tokenType == JsonTokenType.Number) { - // For primitive types, we can avoid renting and there is no need for a StackRowStack. + // For primitive types, we can avoid renting. database = MetadataDb.CreateLocked(utf8Json.Length); - Parse(utf8JsonSpan, readerOptions, ref database); } else { database = MetadataDb.CreateRented(utf8Json.Length, convertToAlloc: true); - Parse(utf8JsonSpan, readerOptions, ref database); } + Parse(utf8JsonSpan, readerOptions, ref database); return new JsonDocument(utf8Json, database, extraRentedBytes: null); } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.StackRowStack.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.StackRowStack.cs index 924f43b2d2a5e..b6469c6818cfa 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.StackRowStack.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.StackRowStack.cs @@ -18,9 +18,10 @@ public void Initialize(int initialSize) { _rentedBuffer = ArrayPool.Shared.Rent(initialSize); _topOfStack = _rentedBuffer.Length; + Debug.Assert(IsInitialized); } - public bool IsInitialized { get { return _rentedBuffer != null; } } + private bool IsInitialized { get { return _rentedBuffer != null; } } public void Dispose() { @@ -39,6 +40,11 @@ public void Dispose() internal void Push(StackRow row) { + if (!IsInitialized) + { + Initialize(JsonDocumentOptions.DefaultMaxDepth * StackRow.Size); + } + if (_topOfStack < StackRow.Size) { Enlarge(); @@ -50,7 +56,9 @@ internal void Push(StackRow row) internal StackRow Pop() { + Debug.Assert(IsInitialized); Debug.Assert(_topOfStack <= _rentedBuffer!.Length - StackRow.Size); + StackRow row = MemoryMarshal.Read(_rentedBuffer.AsSpan(_topOfStack)); _topOfStack += StackRow.Size; return row; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs index fef00bc3b713a..bcc605cabb766 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs @@ -926,14 +926,14 @@ private static void Parse( int numberOfRowsForMembers = 0; int numberOfRowsForValues = 0; + Utf8JsonReader reader = new Utf8JsonReader( + utf8JsonSpan, + isFinalBlock: true, + new JsonReaderState(options: readerOptions)); + StackRowStack stack = default; using (stack) { - Utf8JsonReader reader = new Utf8JsonReader( - utf8JsonSpan, - isFinalBlock: true, - new JsonReaderState(options: readerOptions)); - while (reader.Read()) { JsonTokenType tokenType = reader.TokenType; @@ -953,7 +953,7 @@ private static void Parse( numberOfRowsForValues++; database.Append(tokenType, tokenStart, DbRow.UnknownSize); var row = new StackRow(numberOfRowsForMembers + 1); - Push(row); + stack.Push(row); numberOfRowsForMembers = 0; } else if (tokenType == JsonTokenType.EndObject) @@ -969,7 +969,7 @@ private static void Parse( database.SetNumberOfRows(rowIndex, numberOfRowsForMembers); database.SetNumberOfRows(newRowIndex, numberOfRowsForMembers); - StackRow row = Pop(); + StackRow row = stack.Pop(); numberOfRowsForMembers += row.SizeOrLength; } else if (tokenType == JsonTokenType.StartArray) @@ -982,7 +982,7 @@ private static void Parse( numberOfRowsForMembers++; database.Append(tokenType, tokenStart, DbRow.UnknownSize); var row = new StackRow(arrayItemsCount, numberOfRowsForValues + 1); - Push(row); + stack.Push(row); arrayItemsCount = 0; numberOfRowsForValues = 0; } @@ -1014,7 +1014,7 @@ private static void Parse( database.Append(tokenType, tokenStart, reader.ValueSpan.Length); database.SetNumberOfRows(newRowIndex, numberOfRowsForValues); - StackRow row = Pop(); + StackRow row = stack.Pop(); arrayItemsCount = row.SizeOrLength; numberOfRowsForValues += row.NumberOfRows; } @@ -1071,22 +1071,6 @@ private static void Parse( database.CompleteAllocations(); } - - void Push(StackRow row) - { - if (!stack.IsInitialized) - { - stack.Initialize(JsonDocumentOptions.DefaultMaxDepth * StackRow.Size); - } - - stack.Push(row); - } - - StackRow Pop() - { - Debug.Assert(stack.IsInitialized); - return stack.Pop(); - } } private void CheckNotDisposed() From c36687f096a073b3cbbe9e84b35852db3174c533 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Fri, 9 Oct 2020 09:20:34 -0500 Subject: [PATCH 5/8] Minor refactoring for perf --- .../Json/Document/JsonDocument.MetadataDb.cs | 6 + .../Text/Json/Document/JsonDocument.Parse.cs | 24 +- .../Document/JsonDocument.StackRowStack.cs | 18 +- .../System/Text/Json/Document/JsonDocument.cs | 214 +++++++++--------- 4 files changed, 135 insertions(+), 127 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs index 5c6f65069a9f0..48494efd3f045 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs @@ -86,8 +86,14 @@ private struct MetadataDb : IDisposable internal int Length { get; private set; } private byte[] _data; + private bool _convertToAlloc; // Convert the rented data to an alloc when complete. private bool _isLocked; // Is the array the correct fixed size. + // _isLocked _convertToAlloc truth table: + // false false Standard flow. Size is not known and renting used throughout lifetime. + // true false Used by JsonElement.ParseValue() for primitives and JsonDocument.Clone(). Size is known and no renting. + // false true Used by JsonElement.ParseValue() for arrays and objects. Renting used until size is known. + // true true not valid private MetadataDb(byte[] initialDb, bool isLocked, bool convertToAlloc) { diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.Parse.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.Parse.cs index f3b29f5824404..0339ac4e50f9e 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.Parse.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.Parse.cs @@ -616,16 +616,21 @@ private static JsonDocument Parse( { ReadOnlySpan utf8JsonSpan = utf8Json.Span; var database = MetadataDb.CreateRented(utf8Json.Length, convertToAlloc: false); + var stack = new StackRowStack(JsonDocumentOptions.DefaultMaxDepth * StackRow.Size); try { - Parse(utf8JsonSpan, readerOptions, ref database); + Parse(utf8JsonSpan, readerOptions, ref database, ref stack); } catch { database.Dispose(); throw; } + finally + { + stack.Dispose(); + } return new JsonDocument(utf8Json, database, extraRentedBytes); } @@ -644,18 +649,27 @@ private static JsonDocument ParseUnrented( ReadOnlySpan utf8JsonSpan = utf8Json.Span; MetadataDb database; - if (tokenType == JsonTokenType.String || - tokenType == JsonTokenType.Number) + if (tokenType == JsonTokenType.String || tokenType == JsonTokenType.Number) { - // For primitive types, we can avoid renting. + // For primitive types, we can avoid renting MetadataDb and creating StackRowStack. database = MetadataDb.CreateLocked(utf8Json.Length); + StackRowStack stack = default; + Parse(utf8JsonSpan, readerOptions, ref database, ref stack); } else { database = MetadataDb.CreateRented(utf8Json.Length, convertToAlloc: true); + var stack = new StackRowStack(JsonDocumentOptions.DefaultMaxDepth * StackRow.Size); + try + { + Parse(utf8JsonSpan, readerOptions, ref database, ref stack); + } + finally + { + stack.Dispose(); + } } - Parse(utf8JsonSpan, readerOptions, ref database); return new JsonDocument(utf8Json, database, extraRentedBytes: null); } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.StackRowStack.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.StackRowStack.cs index b6469c6818cfa..3288e954fad2e 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.StackRowStack.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.StackRowStack.cs @@ -11,21 +11,18 @@ public sealed partial class JsonDocument { private struct StackRowStack : IDisposable { - private byte[]? _rentedBuffer; + private byte[] _rentedBuffer; private int _topOfStack; - public void Initialize(int initialSize) + public StackRowStack(int initialSize) { _rentedBuffer = ArrayPool.Shared.Rent(initialSize); _topOfStack = _rentedBuffer.Length; - Debug.Assert(IsInitialized); } - private bool IsInitialized { get { return _rentedBuffer != null; } } - public void Dispose() { - byte[]? toReturn = _rentedBuffer; + byte[] toReturn = _rentedBuffer; _rentedBuffer = null!; _topOfStack = 0; @@ -40,11 +37,6 @@ public void Dispose() internal void Push(StackRow row) { - if (!IsInitialized) - { - Initialize(JsonDocumentOptions.DefaultMaxDepth * StackRow.Size); - } - if (_topOfStack < StackRow.Size) { Enlarge(); @@ -56,7 +48,7 @@ internal void Push(StackRow row) internal StackRow Pop() { - Debug.Assert(IsInitialized); + Debug.Assert(_rentedBuffer != null); Debug.Assert(_topOfStack <= _rentedBuffer!.Length - StackRow.Size); StackRow row = MemoryMarshal.Read(_rentedBuffer.AsSpan(_topOfStack)); @@ -66,7 +58,7 @@ internal StackRow Pop() private void Enlarge() { - byte[] toReturn = _rentedBuffer!; + byte[] toReturn = _rentedBuffer; _rentedBuffer = ArrayPool.Shared.Rent(toReturn.Length * 2); Buffer.BlockCopy( diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs index bcc605cabb766..fb120a49ae72d 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs @@ -919,7 +919,8 @@ private void WriteString(in DbRow row, Utf8JsonWriter writer) private static void Parse( ReadOnlySpan utf8JsonSpan, JsonReaderOptions readerOptions, - ref MetadataDb database) + ref MetadataDb database, + ref StackRowStack stack) { bool inArray = false; int arrayItemsCount = 0; @@ -931,98 +932,120 @@ private static void Parse( isFinalBlock: true, new JsonReaderState(options: readerOptions)); - StackRowStack stack = default; - using (stack) + while (reader.Read()) { - while (reader.Read()) - { - JsonTokenType tokenType = reader.TokenType; + JsonTokenType tokenType = reader.TokenType; - // Since the input payload is contained within a Span, - // token start index can never be larger than int.MaxValue (i.e. utf8JsonSpan.Length). - Debug.Assert(reader.TokenStartIndex <= int.MaxValue); - int tokenStart = (int)reader.TokenStartIndex; + // Since the input payload is contained within a Span, + // token start index can never be larger than int.MaxValue (i.e. utf8JsonSpan.Length). + Debug.Assert(reader.TokenStartIndex <= int.MaxValue); + int tokenStart = (int)reader.TokenStartIndex; - if (tokenType == JsonTokenType.StartObject) + if (tokenType == JsonTokenType.StartObject) + { + if (inArray) { - if (inArray) - { - arrayItemsCount++; - } - - numberOfRowsForValues++; - database.Append(tokenType, tokenStart, DbRow.UnknownSize); - var row = new StackRow(numberOfRowsForMembers + 1); - stack.Push(row); - numberOfRowsForMembers = 0; + arrayItemsCount++; } - else if (tokenType == JsonTokenType.EndObject) - { - int rowIndex = database.FindIndexOfFirstUnsetSizeOrLength(JsonTokenType.StartObject); - numberOfRowsForValues++; - numberOfRowsForMembers++; - database.SetLength(rowIndex, numberOfRowsForMembers); + numberOfRowsForValues++; + database.Append(tokenType, tokenStart, DbRow.UnknownSize); + var row = new StackRow(numberOfRowsForMembers + 1); + stack.Push(row); + numberOfRowsForMembers = 0; + } + else if (tokenType == JsonTokenType.EndObject) + { + int rowIndex = database.FindIndexOfFirstUnsetSizeOrLength(JsonTokenType.StartObject); - int newRowIndex = database.Length; - database.Append(tokenType, tokenStart, reader.ValueSpan.Length); - database.SetNumberOfRows(rowIndex, numberOfRowsForMembers); - database.SetNumberOfRows(newRowIndex, numberOfRowsForMembers); + numberOfRowsForValues++; + numberOfRowsForMembers++; + database.SetLength(rowIndex, numberOfRowsForMembers); - StackRow row = stack.Pop(); - numberOfRowsForMembers += row.SizeOrLength; - } - else if (tokenType == JsonTokenType.StartArray) - { - if (inArray) - { - arrayItemsCount++; - } + int newRowIndex = database.Length; + database.Append(tokenType, tokenStart, reader.ValueSpan.Length); + database.SetNumberOfRows(rowIndex, numberOfRowsForMembers); + database.SetNumberOfRows(newRowIndex, numberOfRowsForMembers); - numberOfRowsForMembers++; - database.Append(tokenType, tokenStart, DbRow.UnknownSize); - var row = new StackRow(arrayItemsCount, numberOfRowsForValues + 1); - stack.Push(row); - arrayItemsCount = 0; - numberOfRowsForValues = 0; + StackRow row = stack.Pop(); + numberOfRowsForMembers += row.SizeOrLength; + } + else if (tokenType == JsonTokenType.StartArray) + { + if (inArray) + { + arrayItemsCount++; } - else if (tokenType == JsonTokenType.EndArray) + + numberOfRowsForMembers++; + database.Append(tokenType, tokenStart, DbRow.UnknownSize); + var row = new StackRow(arrayItemsCount, numberOfRowsForValues + 1); + stack.Push(row); + arrayItemsCount = 0; + numberOfRowsForValues = 0; + } + else if (tokenType == JsonTokenType.EndArray) + { + int rowIndex = database.FindIndexOfFirstUnsetSizeOrLength(JsonTokenType.StartArray); + + numberOfRowsForValues++; + numberOfRowsForMembers++; + database.SetLength(rowIndex, arrayItemsCount); + database.SetNumberOfRows(rowIndex, numberOfRowsForValues); + + // If the array item count is (e.g.) 12 and the number of rows is (e.g.) 13 + // then the extra row is just this EndArray item, so the array was made up + // of simple values. + // + // If the off-by-one relationship does not hold, then one of the values was + // more than one row, making it a complex object. + // + // This check is similar to tracking the start array and painting it when + // StartObject or StartArray is encountered, but avoids the mixed state + // where "UnknownSize" implies "has complex children". + if (arrayItemsCount + 1 != numberOfRowsForValues) { - int rowIndex = database.FindIndexOfFirstUnsetSizeOrLength(JsonTokenType.StartArray); - - numberOfRowsForValues++; - numberOfRowsForMembers++; - database.SetLength(rowIndex, arrayItemsCount); - database.SetNumberOfRows(rowIndex, numberOfRowsForValues); - - // If the array item count is (e.g.) 12 and the number of rows is (e.g.) 13 - // then the extra row is just this EndArray item, so the array was made up - // of simple values. - // - // If the off-by-one relationship does not hold, then one of the values was - // more than one row, making it a complex object. - // - // This check is similar to tracking the start array and painting it when - // StartObject or StartArray is encountered, but avoids the mixed state - // where "UnknownSize" implies "has complex children". - if (arrayItemsCount + 1 != numberOfRowsForValues) - { - database.SetHasComplexChildren(rowIndex); - } + database.SetHasComplexChildren(rowIndex); + } - int newRowIndex = database.Length; - database.Append(tokenType, tokenStart, reader.ValueSpan.Length); - database.SetNumberOfRows(newRowIndex, numberOfRowsForValues); + int newRowIndex = database.Length; + database.Append(tokenType, tokenStart, reader.ValueSpan.Length); + database.SetNumberOfRows(newRowIndex, numberOfRowsForValues); + + StackRow row = stack.Pop(); + arrayItemsCount = row.SizeOrLength; + numberOfRowsForValues += row.NumberOfRows; + } + else if (tokenType == JsonTokenType.PropertyName) + { + numberOfRowsForValues++; + numberOfRowsForMembers++; + + // Adding 1 to skip the start quote will never overflow + Debug.Assert(tokenStart < int.MaxValue); - StackRow row = stack.Pop(); - arrayItemsCount = row.SizeOrLength; - numberOfRowsForValues += row.NumberOfRows; + database.Append(tokenType, tokenStart + 1, reader.ValueSpan.Length); + + if (reader._stringHasEscaping) + { + database.SetHasComplexChildren(database.Length - DbRow.Size); } - else if (tokenType == JsonTokenType.PropertyName) + + Debug.Assert(!inArray); + } + else + { + Debug.Assert(tokenType >= JsonTokenType.String && tokenType <= JsonTokenType.Null); + numberOfRowsForValues++; + numberOfRowsForMembers++; + + if (inArray) { - numberOfRowsForValues++; - numberOfRowsForMembers++; + arrayItemsCount++; + } + if (tokenType == JsonTokenType.String) + { // Adding 1 to skip the start quote will never overflow Debug.Assert(tokenStart < int.MaxValue); @@ -1032,45 +1055,18 @@ private static void Parse( { database.SetHasComplexChildren(database.Length - DbRow.Size); } - - Debug.Assert(!inArray); } else { - Debug.Assert(tokenType >= JsonTokenType.String && tokenType <= JsonTokenType.Null); - numberOfRowsForValues++; - numberOfRowsForMembers++; - - if (inArray) - { - arrayItemsCount++; - } - - if (tokenType == JsonTokenType.String) - { - // Adding 1 to skip the start quote will never overflow - Debug.Assert(tokenStart < int.MaxValue); - - database.Append(tokenType, tokenStart + 1, reader.ValueSpan.Length); - - if (reader._stringHasEscaping) - { - database.SetHasComplexChildren(database.Length - DbRow.Size); - } - } - else - { - database.Append(tokenType, tokenStart, reader.ValueSpan.Length); - } + database.Append(tokenType, tokenStart, reader.ValueSpan.Length); } - - inArray = reader.IsInArray; } - Debug.Assert(reader.BytesConsumed == utf8JsonSpan.Length); - - database.CompleteAllocations(); + inArray = reader.IsInArray; } + + Debug.Assert(reader.BytesConsumed == utf8JsonSpan.Length); + database.CompleteAllocations(); } private void CheckNotDisposed() From 8b370caadd99d29fd6c2bd5dcb10f24b48a56dff Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Wed, 14 Oct 2020 14:38:46 -0500 Subject: [PATCH 6/8] Make WSF.PolymorphicJsonPropertyInfo private and add comments --- .../src/System/Text/Json/Serialization/WriteStackFrame.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs index ebf8885bd6825..59533e5c54231 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs @@ -66,7 +66,7 @@ internal struct WriteStackFrame /// For objects, it is the for the class and current property. /// For collections, it is the for the class and current element. /// - public JsonPropertyInfo? PolymorphicJsonPropertyInfo; + private JsonPropertyInfo? PolymorphicJsonPropertyInfo; // Whether to use custom number handling. public JsonNumberHandling? NumberHandling; @@ -94,10 +94,12 @@ public JsonPropertyInfo GetPolymorphicJsonPropertyInfo() } /// - /// Initializes the state for polymorphic cases. + /// Initializes the state for polymorphic cases and returns the appropriate converter. /// public JsonConverter InitializeReEntry(Type type, JsonSerializerOptions options) { + // For perf, avoid the dictionary lookup in GetOrAddClass() for every element of a collection + // if the current element is the same type as the previous element. if (PolymorphicJsonPropertyInfo?.RuntimePropertyType != type) { JsonClassInfo classInfo = options.GetOrAddClass(type); From d4bdb199ebc57cb81f37fa982f02513ec5f8e3f0 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Fri, 16 Oct 2020 12:03:57 -0500 Subject: [PATCH 7/8] Misc feedback --- .../Json/Document/JsonDocument.MetadataDb.cs | 25 ++++++++----------- .../System/Text/Json/Document/JsonElement.cs | 4 +-- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs index 48494efd3f045..d12a36c719826 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs @@ -218,7 +218,16 @@ internal void Append(JsonTokenType tokenType, int startLocation, int length) if (Length >= _data.Length - DbRow.Size) { - Enlarge(); + Debug.Assert(!_isLocked, "Appending to a locked database"); + + byte[] toReturn = _data; + _data = ArrayPool.Shared.Rent(toReturn.Length * 2); + Buffer.BlockCopy(toReturn, 0, _data, 0, toReturn.Length); + + // The data in this rented buffer only conveys the positions and + // lengths of tokens in a document, but no content; so it does not + // need to be cleared. + ArrayPool.Shared.Return(toReturn); } DbRow row = new DbRow(tokenType, startLocation, length); @@ -226,20 +235,6 @@ internal void Append(JsonTokenType tokenType, int startLocation, int length) Length += DbRow.Size; } - private void Enlarge() - { - Debug.Assert(!_isLocked, "Appending to a locked database"); - - byte[] toReturn = _data; - _data = ArrayPool.Shared.Rent(toReturn.Length * 2); - Buffer.BlockCopy(toReturn, 0, _data, 0, toReturn.Length); - - // The data in this rented buffer only conveys the positions and - // lengths of tokens in a document, but no content; so it does not - // need to be cleared. - ArrayPool.Shared.Return(toReturn); - } - [Conditional("DEBUG")] private void AssertValidIndex(int index) { diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonElement.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonElement.cs index 5ca1a91406bf6..0e00e1dba0a13 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonElement.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonElement.cs @@ -33,8 +33,8 @@ internal static JsonElement ParseValue(ref Utf8JsonReader reader) { bool ret = JsonDocument.TryParseValue(ref reader, out JsonDocument? document, shouldThrow: true, useArrayPools: false); - Debug.Assert(ret, "Parse returned false with shouldThrow: true."); - return document!.RootElement; + Debug.Assert(document != null, "Parse returned false with shouldThrow: true."); + return document.RootElement; } [DebuggerBrowsable(DebuggerBrowsableState.Never)] From 48db4b9c92a24d76d049d26c154520d0c0e1785d Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Mon, 19 Oct 2020 10:24:43 -0500 Subject: [PATCH 8/8] Add back Enlarge(); update Asserts --- .../Json/Document/JsonDocument.MetadataDb.cs | 25 +++++++++++-------- .../System/Text/Json/Document/JsonElement.cs | 3 ++- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs index d12a36c719826..48494efd3f045 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.MetadataDb.cs @@ -218,16 +218,7 @@ internal void Append(JsonTokenType tokenType, int startLocation, int length) if (Length >= _data.Length - DbRow.Size) { - Debug.Assert(!_isLocked, "Appending to a locked database"); - - byte[] toReturn = _data; - _data = ArrayPool.Shared.Rent(toReturn.Length * 2); - Buffer.BlockCopy(toReturn, 0, _data, 0, toReturn.Length); - - // The data in this rented buffer only conveys the positions and - // lengths of tokens in a document, but no content; so it does not - // need to be cleared. - ArrayPool.Shared.Return(toReturn); + Enlarge(); } DbRow row = new DbRow(tokenType, startLocation, length); @@ -235,6 +226,20 @@ internal void Append(JsonTokenType tokenType, int startLocation, int length) Length += DbRow.Size; } + private void Enlarge() + { + Debug.Assert(!_isLocked, "Appending to a locked database"); + + byte[] toReturn = _data; + _data = ArrayPool.Shared.Rent(toReturn.Length * 2); + Buffer.BlockCopy(toReturn, 0, _data, 0, toReturn.Length); + + // The data in this rented buffer only conveys the positions and + // lengths of tokens in a document, but no content; so it does not + // need to be cleared. + ArrayPool.Shared.Return(toReturn); + } + [Conditional("DEBUG")] private void AssertValidIndex(int index) { diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonElement.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonElement.cs index 0e00e1dba0a13..49215698e7c38 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonElement.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonElement.cs @@ -33,7 +33,8 @@ internal static JsonElement ParseValue(ref Utf8JsonReader reader) { bool ret = JsonDocument.TryParseValue(ref reader, out JsonDocument? document, shouldThrow: true, useArrayPools: false); - Debug.Assert(document != null, "Parse returned false with shouldThrow: true."); + Debug.Assert(ret != false, "Parse returned false with shouldThrow: true."); + Debug.Assert(document != null, "null document returned with shouldThrow: true."); return document.RootElement; }