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

Recoup some of the perf losses in cold start serialization #73497

Merged
merged 1 commit into from
Aug 8, 2022

Conversation

eiriktsarpalis
Copy link
Member

@eiriktsarpalis eiriktsarpalis commented Aug 5, 2022

Makes the following changes:

  • Use ConstructorInfo invocations instead of static MethodInfo when instantiating generic metadata types. In my measurements this is ~2x faster.
  • Delegate JsonTypeInfo<T> construction to existing JsonConverter<T> instances, where possible.
Method Job Toolchain Mean Error StdDev Median Min Max Ratio MannWhitney(3%) RatioSD Gen 0 Gen 1 Allocated Alloc Ratio
NewCustomConverter Job-GMEIGR main 24.35 us 0.312 us 0.244 us 24.37 us 23.92 us 24.78 us 1.00 Base 0.00 0.9720 0.0972 9.74 KB 1.00
NewCustomConverter Job-XOBSON PR 19.80 us 0.389 us 0.400 us 19.78 us 19.23 us 20.45 us 0.82 Faster 0.02 0.7650 0.0765 7.86 KB 0.81

Fix #73388.

@ghost
Copy link

ghost commented Aug 5, 2022

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

Issue Details

Reflection-based initialization of the generic metadata types was being done via generic MethodInfo invocations, which can be ~2x slower compared to ConstructorInfo invocations.

Method Job Toolchain Mean Error StdDev Median Min Max Ratio MannWhitney(3%) RatioSD Gen 0 Gen 1 Allocated Alloc Ratio
NewCustomConverter Job-CBOUFE main 27.58 us 1.400 us 1.556 us 27.92 us 24.88 us 30.32 us 1.00 Base 0.00 0.8943 0.0994 9.74 KB 1.00
NewCustomConverter Job-NMGPDV PR 24.55 us 1.478 us 1.702 us 24.16 us 21.94 us 27.43 us 0.89 Faster 0.09 0.8059 0.0895 8.52 KB 0.88

Fix #73388.

Author: eiriktsarpalis
Assignees: eiriktsarpalis
Labels:

area-System.Text.Json

Milestone: 7.0.0

Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you @eiriktsarpalis !

@adamsitnik adamsitnik added the tenet-performance Performance related issue label Aug 8, 2022
Copy link
Member

@krwq krwq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@krwq krwq merged commit 3e03eda into dotnet:main Aug 8, 2022
@eiriktsarpalis eiriktsarpalis deleted the coldstart-regression branch August 19, 2022 13:58
@ghost ghost locked as resolved and limited conversation to collaborators Sep 18, 2022
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.

Regressions in System.Text.Json.Serialization.Tests.ColdStartSerialization<SimpleStructWithProperties>
3 participants