NotificationsYou must be signed in to change notification settings
Fork5.1k
Star16.6k

Performance improvements in`JsonValue`.#103733

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

eiriktsarpalis merged 4 commits intodotnet:mainfromeiriktsarpalis:jsonnode-perf

Jun 20, 2024

Merged

Performance improvements in`JsonValue`.#103733

eiriktsarpalis merged 4 commits intodotnet:mainfromeiriktsarpalis:jsonnode-perf

Jun 20, 2024

Conversation

Copy link

Member

eiriktsarpalis commentedJun 19, 2024

One of the oddities ofJsonNode design is that it permits encapsulation of arbitrary .NET objects via theJsonValue type. This is known to create performance problems since it's not possible to introspectJsonValue instances without serializing and deserializing the underlying object first. The problem is fairly pervasive since JSON primitives such as boolean, numbers and strings are all represented usingJsonValue.

This PR refactors the implementation ofJsonValue so that common primitive types and deserialized values backed byJsonElement are fully segregated from the implementation that admits arbitrary objects. Consequently this change recordssubstantial performance improvements:

Method	Branch	Mean	Ratio	Allocated	Alloc Ratio
DeepEquals	main	2,859.165 ns	1.00	4224 B	1.00
DeepEquals	PR	270.130 ns	0.09	-	0.00

DeepEquals_JsonElement	main	660.551 ns	1.00	328 B	1.00
DeepEquals_JsonElement	PR	432.391 ns	0.65	-	0.00

GetValueKind	main	612.784 ns	1.000	616 B	1.00
GetValueKind	PR	5.243 ns	0.009	-	0.00

GetValueKind_JsonElement	main	15.557 ns	1.00	-	NA
GetValueKind_JsonElement	PR	11.111 ns	0.71	-	NA

eiriktsarpalis added2 commits

June 19, 2024 22:18

Performance improvements in JsonValue.

124e54c

Fixdotnet#103715.

91c9d3c

eiriktsarpalis requested a review fromstephentoub

June 19, 2024 21:28

ghost added the area-System.Text.Json label

Jun 19, 2024

eiriktsarpalis added this to the9.0.0 milestone

Jun 19, 2024

eiriktsarpalis self-assigned this

Jun 19, 2024

eiriktsarpalis added the tenet-performancePerformance related issue label

Jun 19, 2024

Copy link

Contributor

dotnet-policy-servicebot commentedJun 19, 2024

Tagging subscribers to this area: @dotnet/area-system-text-json,@gregsdennis
See info inarea-owners.md if you want to be subscribed.

eiriktsarpalis requested review fromjozkee andkrwq

June 19, 2024 21:31

Copy link

Contributor

gregsdennis commentedJun 19, 2024

Just confirming that we're not losing the ability to store arbitrary objects inJsonValue. I actually utilize this inmy JSON-e implementation.

Copy link

MemberAuthor

eiriktsarpalis commentedJun 19, 2024

Just confirming that we're not losing the ability to store arbitrary objects inJsonValue. I actually utilize this inmy JSON-e implementation.

We couldn't do that. This change simply isolates that use case from the hot path scenaria.

stephentoub reviewed

Jun 19, 2024

View reviewed changes

src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonNode.csShow resolvedHide resolved

stephentoub reviewed

Jun 19, 2024

View reviewed changes

src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonObject.IDictionary.csShow resolvedHide resolved

stephentoub approved these changes

Jun 20, 2024

View reviewed changes

This was referencedJun 20, 2024

Debug assertion failure in System.Net.Http.HttpConnection.SendAsync#100616

Closed

The Operation will be canceled. The next steps may not contain expected logs.dotnet/dnceng#3008

Open

jozkee approved these changes

Jun 20, 2024

View reviewed changes

src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfElement.csShow resolvedHide resolved

src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValue.CreateOverloads.csShow resolvedHide resolved

eiriktsarpalis added2 commits

June 20, 2024 16:56

Add more test cases and fix a number of bugs related to DeepEquals an…

b59f3fc

…d escaping.

Fix number handling corner case.

b7485ed

This was referencedJun 20, 2024

Add JsonElement.ValueSpan#77666

Closed

System.Text.Json: add ability to do semantic comparisons of JSON values à la JToken.DeepEquals()#33388

Closed

Copy link

MemberAuthor

eiriktsarpalis commentedJun 20, 2024

/ba-g suppressing unrelated test failure to merge ahead of snap.

eiriktsarpalis merged commitd9b9676 intodotnet:main

Jun 20, 2024

75 of 83 checks passed

eiriktsarpalis deleted the jsonnode-perf branch

June 20, 2024 20:52

build-analysisbot mentioned this pull request

Jun 20, 2024

[Test Failure] System.Net.Http.WinHttpHandlerFunctional.Tests.BidirectionStreamingTest.BackwardsCompatibility_DowngradeToHttp11#103754

Closed

eiriktsarpalis mentioned this pull request

Jun 20, 2024

System.Text.Json.Nodes.Tests.JsonObjectTests.NullPropertyNameFail [FAIL]#103784

Closed

rzikm pushed a commit to rzikm/dotnet-runtime that referenced this pull request

Jun 24, 2024

Performance improvements inJsonValue. (dotnet#103733)

7c6859e

* Performance improvements in JsonValue.*Fixdotnet#103715.* Add more test cases and fix a number of bugs related to DeepEquals and escaping.* Fix number handling corner case.