-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Avoid string allocation and improve performance of JsonProperty.WriteTo
#90074
Conversation
System.Text.Json.JsonProperty.WriteTo uses get_Name, calling JsonElement.GetPropertyName() which would allocate a string. Use ReadOnlySpan<byte> from the underlying UTF8 json, when possible, by adding helper methods into JsonDocument & JsonElement. Fix dotnet#88767
Current code unescapes & escapes property names and uses ToArray. Avoid alloc by adding internal GetRaw/WriteRaw methods.
Original code doesn't handle GetRaw/WriteRaw on escaped property names correctly.
Allocations are further avoided when the property name is shorter than JsonConstants.StackallocByteThreshold, by inlining JsonReaderHelper.GetUnescapedSpan.
Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis Issue DetailsDescription
This PR reuses the underlying UTF8 json The change also improves the performance of Fix #88767 Microbenchmark
Original
Simple: Micro-benchmark codeusing BenchmarkDotNet.Attributes;
using System;
using System.Linq;
using MicroBenchmarks;
using System.IO;
namespace System.Text.Json.Node.Tests
{
[BenchmarkCategory(Categories.Libraries, Categories.JSON)]
[MemoryDiagnoser]
public class JsonPropertyWriteToTest
{
public enum TestCaseType
{
Simple,
WithEscapedCharacters,
WithUnescapedUnicodeCharacters,
}
[ParamsAllValues]
public TestCaseType TestCase;
const string JsonString1 = """
{"normal1":0,"normal2":0,"normal3":0,"normal4":0,"normal5":0}
""";
const string JsonString2 = """
{"normal1":0,"normal2":0,"normal3":0,"normal4":0,"normal5":0,"\"q\t\\m\"m\t":1,"\u6d4b\u8bd5":2}
""";
const string JsonString3 = """
{"normal1":0,"normal2":0,"normal3":0,"normal4":0,"normal5":0,"\"q\t\\m\"m\t":1,"\u6d4b\u8bd5":2,"测试":3}
""";
private static string GetTestCase(TestCaseType type)
{
switch (type)
{
case TestCaseType.Simple:
return JsonString1;
case TestCaseType.WithEscapedCharacters:
return JsonString2;
case TestCaseType.WithUnescapedUnicodeCharacters:
return JsonString3;
}
return "";
}
const int MAX_FOLD = 10;
private static JsonProperty[] Properties;
private static MemoryStream Stream;
private static Utf8JsonWriter Writer;
[GlobalSetup]
public void PrepareJson()
{
Properties = JsonDocument.Parse(Encoding.UTF8.GetBytes(GetTestCase(TestCase))).RootElement.EnumerateObject().ToArray();
Stream = new(JsonString2.Length * MAX_FOLD);
Writer = new Utf8JsonWriter(Stream);
}
[Benchmark]
public MemoryStream WriteProperties()
{
Stream.Position = 0;
Writer.Reset();
Writer.WriteStartObject();
foreach (var it in Properties)
it.WriteTo(Writer);
Writer.WriteEndObject();
Writer.Flush();
return Stream;
}
}
}
|
@dotnet-policy-service agree |
src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonProperty.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonElement.cs
Outdated
Show resolved
Hide resolved
Shorten names of new methods; Add a test of writing out special names.
src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonElement.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonElement.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs
Outdated
Show resolved
Hide resolved
Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it. |
Hi @karakasa, this was closed automatically because it was still marked as draft. Is there any pending work on this PR before it is marked for review? |
src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs
Outdated
Show resolved
Hide resolved
Could you please reopen the PR? I have fixed the code. |
src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
You are the best! 🤩 |
Test failures are known issues, merging away. |
Description
System.Text.Json.JsonProperty.WriteTo
usesget_Name
, thenJsonElement.GetPropertyName()
, a method would always allocate a string.This PR reuses the underlying UTF8 json
ROS<byte>
by adding helper methods intoJsonDocument
&JsonElement
, to avoid alloc. Allocation is further removed by inliningJsonReaderHelper.GetUnescapedSpan
intoJsonElement.WritePropertyNameTo
, which makes the method allocation-less when property names are shorter thanJsonConstants.StackallocByteThreshold
, or don't need unescaping, and the underlying buffer is long enough.Generally the change improves the performance of
JsonProperty.WriteTo
by ~30%.Fix #88767
Microbenchmark
Simple:
{"normal1":0,"normal2":0,"normal3":0,"normal4":0,"normal5":0}
WithEscapedChars:
{"normal1":0,"normal2":0,"normal3":0,"normal4":0,"normal5":0,"\"q\t\\m\"m\t":1,"\u6d4b\u8bd5":2}
WithUnicodeChars:
{"normal1":0,"normal2":0,"normal3":0,"normal4":0,"normal5":0,"\"q\t\\m\"m\t":1,"\u6d4b\u8bd5":2,"测试":3}
Micro-benchmark code