-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[release/7.0] Fix performance regression in STJ JsonNode instantiations #78147
Conversation
Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis Issue DetailsBackport of #78130 to release/7.0 /cc @eiriktsarpalis @stephentoub Customer ImpactTestingRiskIMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.
|
@carlossanlop do we need to increment the servicing version for this change? It's already been incremented as part of #77388. |
Is there any risk that this is now static? Are they mutable and would changes made by one instance now effect others when they didn't before? |
No such risk. The value is used as the default |
I see. Perhaps in |
Yes we could definitely make that change to avoid potential leaks of the value in the future. |
Servicing approved over email. |
Now we only need a code review sign-off. @eiriktsarpalis or @layomia or @ericstj. |
Approved by Tactics and signed-off by area owner. runtime/src/libraries/System.Text.Json/src/System.Text.Json.csproj Lines 10 to 12 in 9e7c283
Ready to merge. |
Backport of #78130 to release/7.0
/cc @eiriktsarpalis @stephentoub
Customer Impact
Fixes a customer reported performance regression when creating
JsonNode
instances. This was caused by a static field in the class not having included thestatic
keyword, resulting in expensive initializations on everyJsonNode
instantiation. According to benchmarks, this has resulted in said instantiation being 7x slower compared to .NET 6.Testing
Added benchmarks to dotnet/performance measuring performance of
JsonNode
.Risk
Low. Adds a missing
static
keyword to a readonly field declaration.