-
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
Fix Http.Json serialization performance by using static options #35040
Conversation
Tagging subscribers to this area: @dotnet/ncl |
src/libraries/System.Net.Http.Json/src/System/Net/Http/Json/JsonContent.cs
Show resolved
Hide resolved
@@ -20,8 +20,8 @@ public sealed partial class JsonContent : HttpContent | |||
private static MediaTypeHeaderValue DefaultMediaType |
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.
@jozkee Could we squeeze a bit more performance out of it by also switching to a field and reusing the default media type instance instead of having the lambda?
private static readonly MediaTypeHeaderValue s_defaultMediaType
= new MediaTypeHeaderValue(JsonMediaType) { CharSet = "utf-8" };
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.
I did consider that as well, but as per @scalablecory's, we must not re-use this, it could be modified or disposed on a request, or it can be intercepted by a DelegatingHandler
.
Fixes #34440
There was a considerable performance regression compared against
Microsoft.AspNetCore.Blazor.HttpClient
caused by always instantiating aJsonSerializerOptions
object when not provided, which means thatSystem.Net.Http.Json
was not actively using the caching mechanism of theJsonSerializer
for types and converters.I got the following results using the aspnet/Benchmarks repo to measure the amount of requests made to a simple WeatherForecast app that returns a
List<WeatherForecast>
.The scenario keeps a thread sending Get requests for the specified duration (15s) after a specified warm-up time lapse (15s).
The benchmarks used can be found in https://github.com/Jozkee/HttpJsonBenchmarks.
I ran the scenarios 5 times each in order to get a larger sample.
I also ensured that the same version of System.Text.Json was being used (4.7.1 latest stable).
Results
Microsoft.AspNetCore.Blazor.HttpClient Version 3.2.0-preview3.20168.3
[client] 55458 iterations in 15s
[client] 53794 iterations in 15s
[client] 52779 iterations in 15s
[client] 53429 iterations in 15s
[client] 51582 iterations in 15s
System.Net.Http.Json Version 3.2.0-preview3.20175.8
Before fix[client] 13159 iterations in 15s
[client] 12441 iterations in 15s
System.Net.Http.Json 3.2.0-dev
(Private build) After fix[client] 55352 iterations in 15s
[client] 51981 iterations in 15s
[client] 53777 iterations in 15s
[client] 52569 iterations in 15s
[client] 52655 iterations in 15s
Note: There are further optimizations that can be made/be explored, specially on the Post/Put methods but those transcend the quick gain of using the static options for cache on serialization.
cc @ericstj @joperezr @layomia