From a2d7228835ca49402b2a7bce0e980b1af854c20c Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Fri, 29 May 2020 13:47:05 -0500 Subject: [PATCH 1/2] Remove todos and add comments --- .../JsonSerializer.Write.Stream.cs | 33 ++++++++++++++----- .../Json/Serialization/WriteStackFrame.cs | 7 ++-- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.Stream.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.Stream.cs index bf959ba1bd0860..ca20ac7b164263 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.Stream.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.Stream.cs @@ -26,7 +26,11 @@ public static partial class JsonSerializer /// There is no compatible /// for or its serializable members. /// - public static Task SerializeAsync(Stream utf8Json, TValue value, JsonSerializerOptions? options = null, CancellationToken cancellationToken = default) + public static Task SerializeAsync( + Stream utf8Json, + TValue value, + JsonSerializerOptions? options = null, + CancellationToken cancellationToken = default) { if (utf8Json == null) throw new ArgumentNullException(nameof(utf8Json)); @@ -53,7 +57,12 @@ public static Task SerializeAsync(Stream utf8Json, TValue value, JsonSer /// There is no compatible /// for or its serializable members. /// - public static Task SerializeAsync(Stream utf8Json, object? value, Type inputType, JsonSerializerOptions? options = null, CancellationToken cancellationToken = default) + public static Task SerializeAsync( + Stream utf8Json, + object? value, + Type inputType, + JsonSerializerOptions? options = null, + CancellationToken cancellationToken = default) { if (utf8Json == null) { @@ -73,8 +82,20 @@ public static Task SerializeAsync(Stream utf8Json, object? value, Type inputType return WriteAsyncCore(utf8Json, value!, inputType, options, cancellationToken); } - private static async Task WriteAsyncCore(Stream utf8Json, TValue value, Type inputType, JsonSerializerOptions? options, CancellationToken cancellationToken) + private static async Task WriteAsyncCore( + Stream utf8Json, + TValue value, + Type inputType, + JsonSerializerOptions? options, + CancellationToken cancellationToken) { + // We flush the Stream when the buffer is >=90% of capacity. + // With the default buffer size of 16K, if a single JSON value is >=1.6K it is possible for + // the buffer to max out before it can be flushed, causing the buffer to expand. + // So this threshold is a compromise between buffer utilization and minimizing cases where the + // buffer is maxed out (and expanded) before it can be flushed. + const float FlushThreshold = .9f; + if (options == null) { options = JsonSerializerOptions.s_defaultOptions; @@ -100,9 +121,7 @@ private static async Task WriteAsyncCore(Stream utf8Json, TValue value, do { - // todo: determine best value here - // https://github.com/dotnet/runtime/issues/32356 - state.FlushThreshold = (int)(bufferWriter.Capacity * .9); + state.FlushThreshold = (int)(bufferWriter.Capacity * FlushThreshold); isFinalBlock = WriteCore(converterBase, writer, value, options, ref state); @@ -111,8 +130,6 @@ private static async Task WriteAsyncCore(Stream utf8Json, TValue value, bufferWriter.Clear(); } while (!isFinalBlock); } - - // todo: verify that we do want to call FlushAsync here (or above). It seems like leaving it to the caller would be best. } } } 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 f93a51fb9a3f65..03451035cdb77b 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 @@ -96,15 +96,12 @@ public JsonPropertyInfo GetPolymorphicJsonPropertyInfo() /// public JsonConverter InitializeReEntry(Type type, JsonSerializerOptions options, string? propertyName = null) { - JsonClassInfo newClassInfo = options.GetOrAddClass(type); - - // todo: check if type==newtype and skip below? - // https://github.com/dotnet/runtime/issues/32358 + JsonClassInfo classInfo = options.GetOrAddClass(type); // Set for exception handling calculation of JsonPath. JsonPropertyNameAsString = propertyName; - PolymorphicJsonPropertyInfo = newClassInfo.PropertyInfoForClassInfo; + PolymorphicJsonPropertyInfo = classInfo.PropertyInfoForClassInfo; return PolymorphicJsonPropertyInfo.ConverterBase; } From c6960ebc66d9ec242a3f52b36901a2db1468e6df Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Mon, 1 Jun 2020 14:00:16 -0500 Subject: [PATCH 2/2] Update comment --- .../Json/Serialization/JsonSerializer.Write.Stream.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.Stream.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.Stream.cs index ca20ac7b164263..4ab037a7c2b464 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.Stream.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.Stream.cs @@ -90,10 +90,11 @@ private static async Task WriteAsyncCore( CancellationToken cancellationToken) { // We flush the Stream when the buffer is >=90% of capacity. - // With the default buffer size of 16K, if a single JSON value is >=1.6K it is possible for - // the buffer to max out before it can be flushed, causing the buffer to expand. - // So this threshold is a compromise between buffer utilization and minimizing cases where the - // buffer is maxed out (and expanded) before it can be flushed. + // This threshold is a compromise between buffer utilization and minimizing cases where the buffer + // needs to be expanded\doubled because it is not large enough to write the current property or element. + // We check for flush after each object property and array element is written to the buffer. + // Once the buffer is expanded to contain the largest single element\property, a 90% thresold + // means the buffer may be expanded a maximum of 4 times: 1-(1\(2^4))==.9375. const float FlushThreshold = .9f; if (options == null)