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

[System.Text.Json] Unexpected performance penalty when passing options inline #42167

Closed
bjowes opened this issue Sep 12, 2020 · 1 comment
Closed
Labels
Milestone

Comments

@bjowes
Copy link

bjowes commented Sep 12, 2020

Description

Calling JsonSerializer.Serilaize with an options object instantiated inline (or from a local variable) results in an unexpectedly large performance penalty, compared to passing the options object from a class member. The performance penalty is in the order of 1 ms.
Lots of example code in migrating to System.Text.Json uses inline instantiation of the options object, meaning that many developers will get this performance hit without expecting it.

I have written a blog post with a more detailed description of the issue, including benchmarks.
https://dev.to/bjowes/keeping-system-text-json-lean-12jc

The source code for all the benchmarks can be found here
https://github.com/bjowes/SystemTextJson-Benchmark

Configuration

Has been tested on .Net Core 3.1.5 and 3.1.7
Windows 10, x64
See benchmarks below for specs

Regression?

Not known

Data

Benchmark of serialization of a small object (6 properties, where two of them contain objects with more properties):

BenchmarkDotNet=v0.12.1, OS=Windows 10.0.17763.1397 (1809/October2018Update/Redstone5)
Intel Core i7-8700 CPU 3.20GHz (Coffee Lake), 1 CPU, 12 logical and 6 physical cores
.NET Core SDK=3.1.101
  [Host]     : .NET Core 3.1.5 (CoreCLR 4.700.20.26901, CoreFX 4.700.20.27001), X64 RyuJIT
  DefaultJob : .NET Core 3.1.5 (CoreCLR 4.700.20.26901, CoreFX 4.700.20.27001), X64 RyuJIT
Method Mean Error StdDev
Serialize 4.479 μs 0.0123 μs 0.0103 μs
Serialize_InlineOptions_Default 839.179 μs 7.5675 μs 7.0786 μs
Serialize_LocalOptions_Default 842.628 μs 4.0955 μs 3.4199 μs
Serialize_StaticMemberOptions_Default 4.464 μs 0.0144 μs 0.0120 μs
Serialize_MemberOptions_Default 4.499 μs 0.0311 μs 0.0259 μs

*_Default means that the options object was created with the default constructor, no properties where changed that could affect the serialization performance.

Just to make sure, I did a benchmark on instantiation of the options object too:

BenchmarkDotNet=v0.12.1, OS=Windows 10.0.17763.1397 (1809/October2018Update/Redstone5)
Intel Core i7-8700 CPU 3.20GHz (Coffee Lake), 1 CPU, 12 logical and 6 physical cores
.NET Core SDK=3.1.101
  [Host]     : .NET Core 3.1.5 (CoreCLR 4.700.20.26901, CoreFX 4.700.20.27001), X64 RyuJIT
  DefaultJob : .NET Core 3.1.5 (CoreCLR 4.700.20.26901, CoreFX 4.700.20.27001), X64 RyuJIT
Method Mean Error StdDev
CreateOptions 337.1717 ns 3.2470 ns 2.5351 ns

Since instantiating the options object with the default constructor takes well below 1 μs, it surely cannot account for the performance penalty in the benchmark above.

@bjowes bjowes added the tenet-performance Performance related issue label Sep 12, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Sep 13, 2020
@layomia
Copy link
Contributor

layomia commented Sep 13, 2020

This behavior is understood. Please see #38982 and #40072. Will close as dup.

TL;DR (from #38982 (comment))

The serializer undergoes a warm-up phase during the first (de)serialization of every type in the object graph when a new options instance is passed to it. This warm-up includes creating a cache of metadata it needs to perform (de)serialization: funcs to property getters, setters, ctor arguments, specified attributes etc. This metadata caches is stored in the options instance. This process is not cheap, and it is recommended to cache options instances for reuse on subsequent calls to the serializer to avoid unnecessarily undergoing the warm-up repeatedly.

Triage (from #40072 (comment))

For 5.0 we should document best practices for using JsonSerializerOptions and potential effects of not doing so: dotnet/docs#19878.

Mitigations to consider in 6.0/future:

@layomia layomia closed this as completed Sep 13, 2020
@layomia layomia removed the untriaged New issue has not been triaged by the area owner label Sep 13, 2020
@layomia layomia added this to the 5.0.0 milestone Sep 13, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants