Improve deserialization perf for case-insensitive and missing-property cases by steveharter · Pull Request #35848 · dotnet/runtime

steveharter · 2020-05-05T17:37:24Z

~1.75x faster deserialization of a simple POCO when:

JsonSerializerOptions.PropertyNameCaseInsensitive is true AND
Property names in JSON do not match property name (specified by either the CLR property name, the value specified by [JsonPropertyName], or the property naming policy) AND
There is at least one property name > 7 bytes in length

In addition, similar perf results for property-misses cases (property names in JSON do not match any property).

Also there are less cache memory allocations for property names. This also has a startup CPU improvement - a simple POCO resulted in a .5ms startup improvements on first deserialization (~23.5ms to ~23.0ms). Private bytes savings were also verified using Process Explorer (very long property names were used).

Fixes #30789 and the increased startup perf and reduced memory consumption helps with 5.0 serialization goals per dotnet/designs#113.

In addition, there are less stack-based allocations for case-insensitive and missing-property scenarios (shown below in the benchmarks).

This may help with ASP.NET deserialization scenarios since they enable case-insensitive by default (cc @pranavkm). However, since ASP.NET also uses a camel-casing property naming policy, the benefit will not occur if the incoming JSON exactly matches what was produced by the camel-casing policy, as would be the case if all serialization occurs with System.Text.Json.

No measurable perf regressions on existing deserialization benchmarks (within +- 3%). Todo: add a case-insensitive and missing-property benchmark.

The main CPU benefit comes from avoiding the dictionary-based lookup for case-insensitive and missing-property scenarios and instead use the array-backed cache. The previous code would also unnecessarily grow and max out that array (64 elements) in certain cases before using the dictionary.

During testing, it was found that "missing properties" in JSON that are added to extension data did not properly support JsonPath semantics (used when a JsonException is thrown to report the invalid property). That was fixed and a test added.

Below are benchmarks for a simple 4-property POCO test class that has property names > 7 bytes.

3.1 performance
|                            Method |       Mean |   Error |  StdDev |     Median |        Min |        Max |  Gen 0 | Gen 1 | Gen 2 | Allocated |
|---------------------------------- |-----------:|--------:|--------:|-----------:|-----------:|-----------:|-------:|------:|------:|----------:|
| CaseSensitive_Matching            |   844.2 ns | 4.25 ns | 3.55 ns |   844.2 ns |   838.6 ns |   850.6 ns | 0.0342 |     - |     - |     224 B |
| CaseInsensitive_Matching          |   833.3 ns | 3.84 ns | 3.40 ns |   832.6 ns |   829.4 ns |   841.1 ns | 0.0504 |     - |     - |     328 B |
| CaseSensitive_NotMatching(Missing)| 1,007.7 ns | 9.40 ns | 8.79 ns | 1,005.1 ns |   997.3 ns | 1,023.3 ns | 0.0722 |     - |     - |     464 B |
| CaseInsensitive_NotMatching       | 1,405.6 ns | 8.35 ns | 7.40 ns | 1,405.1 ns | 1,397.1 ns | 1,423.6 ns | 0.0626 |     - |     - |     408 B |

5.0 performance (BEFORE)
|                            Method |       Mean |    Error |   StdDev |     Median |        Min |        Max |  Gen 0 | Gen 1 | Gen 2 | Allocated |
|---------------------------------- |-----------:|---------:|---------:|-----------:|-----------:|-----------:|-------:|------:|------:|----------:|
| CaseSensitive_Matching            |   826.0 ns |  6.41 ns |  5.99 ns |   827.1 ns |   815.7 ns |   836.1 ns | 0.0981 |     - |     - |     632 B |
| CaseInsensitive_Matching          |   844.7 ns |  2.95 ns |  2.46 ns |   845.1 ns |   841.2 ns |   848.5 ns | 0.1158 |     - |     - |     736 B |
| CaseSensitive_NotMatching(Missing)|   899.7 ns |  3.77 ns |  3.14 ns |   899.4 ns |   895.5 ns |   906.6 ns | 0.0182 |     - |     - |     120 B |
| CaseInsensitive_NotMatching       | 1,419.6 ns | 24.86 ns | 20.76 ns | 1,424.9 ns | 1,374.7 ns | 1,446.9 ns | 0.1263 |     - |     - |     816 B |

5.0 performance (AFTER)
|                            Method |     Mean |   Error |  StdDev |   Median |      Min |      Max |  Gen 0 | Gen 1 | Gen 2 | Allocated |
|---------------------------------- |---------:|--------:|--------:|---------:|---------:|---------:|-------:|------:|------:|----------:|
| CaseSensitive_Matching            | 799.2 ns | 4.59 ns | 4.29 ns | 801.0 ns | 790.5 ns | 803.9 ns | 0.0985 |     - |     - |     632 B |
| CaseInsensitive_Matching          | 789.2 ns | 6.62 ns | 5.53 ns | 790.3 ns | 776.0 ns | 794.4 ns | 0.1004 |     - |     - |     632 B |
| CaseSensitive_NotMatching(Missing)| 479.9 ns | 0.75 ns | 0.59 ns | 479.8 ns | 479.1 ns | 481.0 ns | 0.0059 |     - |     - |      40 B |
| CaseInsensitive_NotMatching       | 783.5 ns | 3.26 ns | 2.89 ns | 783.5 ns | 779.0 ns | 789.2 ns | 0.1004 |     - |     - |     632 B |

ghost · 2020-05-05T17:37:27Z

Tagging subscribers to this area: @jozkee
Notify danmosemsft if you want to be subscribed.

pranavkm

Nice!

src/libraries/System.Text.Json/src/System/Text/Json/JsonEncodedText.cs

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.Cache.cs

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/PropertyRef.cs

layomia · 2020-05-06T05:02:31Z

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReferenceHandling.cs

Could these be internal properties instead?

Yes; didn't change. cc @jozkee

...ystem.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.Cache.cs

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs

layomia · 2020-05-07T22:35:15Z

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs

nit: we should assert that jsonPropertyInfo.NameAsString and jsonParameterInfo.NameAsString are equal (I can do this in a follow up)

steveharter · 2020-05-11T15:54:27Z

Test failures in runtime Libraries Windows_NT x64 Debug due to infrastructure:
Restore) Cannot create a file when that file already exists.

steveharter added area-System.Text.Json tenet-performance Performance related issue labels May 5, 2020

steveharter added this to the 5.0 milestone May 5, 2020

steveharter requested review from jozkee and layomia May 5, 2020 17:37

steveharter self-assigned this May 5, 2020

pranavkm reviewed May 5, 2020

View reviewed changes

src/libraries/System.Text.Json/src/System/Text/Json/JsonEncodedText.cs Outdated Show resolved Hide resolved

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs Outdated Show resolved Hide resolved

layomia reviewed May 6, 2020

View reviewed changes

jaredpar mentioned this pull request May 6, 2020

OSX machines are de-provisioned during CI / PR runs leading to failures #34472

Closed

stephentoub reviewed May 7, 2020

View reviewed changes

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.Cache.cs Outdated Show resolved Hide resolved

stephentoub reviewed May 7, 2020

View reviewed changes

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.Cache.cs Outdated Show resolved Hide resolved

steveharter commented May 7, 2020

View reviewed changes

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs Outdated Show resolved Hide resolved

layomia approved these changes May 7, 2020

View reviewed changes

steveharter added 2 commits May 8, 2020 13:27

Improve perf of case-insensitive and missing properties

8883d89

Apply new prop lookup pattern to ctor param logic and other feedback

6264d70

steveharter force-pushed the PropertyPerf branch from d115a12 to 6264d70 Compare May 8, 2020 22:32

steveharter merged commit 230538b into dotnet:master May 11, 2020

steveharter deleted the PropertyPerf branch May 11, 2020 15:54

This was referenced May 11, 2020

Add JSON benchmarks for missing-property and case-insensitive dotnet/performance#1322

Merged

Normalizing casing for property names in JsonException.Path helps deserialization performance #30789

Closed

layomia mentioned this pull request Jun 19, 2020

System.Text.Json The JSON property name for collides with another property. #37769

Closed

layomia mentioned this pull request Aug 25, 2020

JsonSerializer improvements in .NET 5 #41313

Closed

ghost locked as resolved and limited conversation to collaborators Dec 9, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve deserialization perf for case-insensitive and missing-property cases#35848

Improve deserialization perf for case-insensitive and missing-property cases#35848
steveharter merged 2 commits intodotnet:masterfrom
steveharter:PropertyPerf

steveharter commented May 5, 2020 •

edited

Loading

Uh oh!

ghost commented May 5, 2020

Uh oh!

pranavkm left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

layomia May 6, 2020

Uh oh!

steveharter May 7, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

layomia May 7, 2020

Uh oh!

steveharter commented May 11, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

steveharter commented May 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented May 5, 2020

Uh oh!

pranavkm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

layomia May 6, 2020

Choose a reason for hiding this comment

Uh oh!

steveharter May 7, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

layomia May 7, 2020

Choose a reason for hiding this comment

Uh oh!

steveharter commented May 11, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

steveharter commented May 5, 2020 •

edited

Loading