Skip to content
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

Performance improvements in JsonValue. #103733

Merged
merged 4 commits into from
Jun 20, 2024

Conversation

eiriktsarpalis
Copy link
Member

One of the oddities of JsonNode design is that it permits encapsulation of arbitrary .NET objects via the JsonValue type. This is known to create performance problems since it's not possible to introspect JsonValue 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 using JsonValue.

This PR refactors the implementation of JsonValue so that common primitive types and deserialized values backed by JsonElement are fully segregated from the implementation that admits arbitrary objects. Consequently this change records substantial 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 eiriktsarpalis added this to the 9.0.0 milestone Jun 19, 2024
@eiriktsarpalis eiriktsarpalis self-assigned this Jun 19, 2024
@eiriktsarpalis eiriktsarpalis added the tenet-performance Performance related issue label Jun 19, 2024
Copy link
Contributor

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

@eiriktsarpalis eiriktsarpalis requested review from jozkee and krwq June 19, 2024 21:31
@gregsdennis
Copy link
Contributor

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

@eiriktsarpalis
Copy link
Member Author

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

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

@eiriktsarpalis
Copy link
Member Author

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

@eiriktsarpalis eiriktsarpalis merged commit d9b9676 into dotnet:main Jun 20, 2024
75 of 83 checks passed
@eiriktsarpalis eiriktsarpalis deleted the jsonnode-perf branch June 20, 2024 20:52
rzikm pushed a commit to rzikm/dotnet-runtime that referenced this pull request Jun 24, 2024
* Performance improvements in JsonValue.

* Fix dotnet#103715.

* Add more test cases and fix a number of bugs related to DeepEquals and escaping.

* Fix number handling corner case.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants