Skip to content

Support deserialization for Struct properties that implements IList<T> #41223

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.

Sign up for GitHub

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

Jump to bottom
Merged
layomia merged 5 commits into from
Sep 27, 2020
Merged

Support deserialization for Struct properties that implements IList<T> #41223

layomia merged 5 commits into from
Sep 27, 2020

Conversation

@RaymondHuy
Copy link
Contributor

@RaymondHuy RaymondHuy commented Aug 23, 2020 •
edited by layomia
Loading

Fixes #1998
Add a new field into IListOfTConverter class to store the values to prevent unboxing when the TCollection is ValueType.

Support deserialization for Struct properties that implements IList<T>
ba01b03 
Add a new field into IListOfTConverter class to store the values to prevent unboxing when the TCollection is ValueType.
@Dotnet-GitSync-Bot
Copy link
Collaborator

Dotnet-GitSync-Bot commented Aug 23, 2020

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@dnfadmin
Copy link

dnfadmin commented Aug 23, 2020 •
edited
Loading

CLA assistant check
All CLA requirements met.

@layomia layomia added this to the 6.0.0 milestone Aug 24, 2020
HuyLuong added 2 commits August 26, 2020 23:24
Support deserialization for struct that implements IList<T>, ICollect…
30bc9d7 
…ion<T>, IList

Use constraint type instead of TCollection to avoid unboxing when TCollection is ValueType.
Add testcase Support deserialization for Struct that implements ISet<…
0b8260c 
…T>, IDictionary<TKey, TValue>, IDictionary

Use constraint type to cast to avoid unboxing when the target type is ValueType
@jaredpar jaredpar mentioned this pull request Aug 27, 2020
Closed
@RaymondHuy
Copy link
Contributor Author

RaymondHuy commented Sep 4, 2020

Hi @layomia @steveharter I have updated my PR to resolve your comments. Are there anything else I can do to get this PR approved ?

@RaymondHuy RaymondHuy closed this Sep 5, 2020
@RaymondHuy RaymondHuy reopened this Sep 5, 2020
layomia
layomia reviewed Sep 14, 2020
View reviewed changes
Copy link
Contributor

@layomia layomia left a comment

Choose a reason for hiding this comment

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

Thanks for the changes.

We also need to update the Add methods of the following converter types and add corresponding tests:

  • ICollectionOfTConverter
  • IDictionaryConverter
  • IDictionaryOfTKeyTValueConverter
  • IListConverter
  • IListOfTConverter
  • ISetOfTConverter

Add testcase for reading nullable struct and null json.
0eb3a88 
Modify the 'Add' method to use actual type casting.
@RaymondHuy
Copy link
Contributor Author

RaymondHuy commented Sep 25, 2020

@layomia I have just updated my PR.
Here is my benchmark result: The first table is before the change and the second table is after the change.

BenchmarkDotNet=v0.12.1.1405-nightly, OS=Windows 10.0.18363.1082 (1909/November2019Update/19H2)
Intel Core i7-9700 CPU 3.00GHz, 1 CPU, 8 logical and 8 physical cores
.NET Core SDK=5.0.100-preview.7.20366.6
  [Host]     : .NET Core 5.0.0 (CoreCLR 5.0.20.36411, CoreFX 5.0.20.36411), X64 RyuJIT
  Job-WVOCPJ : .NET Core 5.0 (CoreCLR 42.42.42.42424, CoreFX 42.42.42.42424), X64 RyuJIT

PowerPlanMode=00000000-0000-0000-0000-000000000000  Arguments=/p:DebugType=portable  Toolchain=CoreRun  
IterationTime=250.0000 ms  MaxIterationCount=20  MinIterationCount=15  
WarmupCount=1

ArrayList

Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 49.54 μs 0.439 μs 0.389 μs 49.55 μs 48.71 μs 50.26 μs 4.8905 0.5869 - 30.29 KB
DeserializeFromUtf8Bytes 50.77 μs 0.409 μs 0.363 μs 50.74 μs 50.38 μs 51.67 μs 4.8940 0.6117 - 30.29 KB
DeserializeFromStream 48.72 μs 0.374 μs 0.350 μs 48.78 μs 48.16 μs 49.33 μs 4.8704 0.5845 - 30.36 KB
Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 47.89 μs 0.524 μs 0.490 μs 47.91 μs 47.07 μs 48.71 μs 4.7701 0.5724 - 30.29 KB
DeserializeFromUtf8Bytes 48.35 μs 0.420 μs 0.372 μs 48.38 μs 47.57 μs 49.03 μs 4.7755 0.5731 - 30.29 KB
DeserializeFromStream 48.71 μs 0.375 μs 0.351 μs 48.82 μs 47.71 μs 49.08 μs 4.8132 0.5776 - 30.36 KB

BinaryData

Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 631.3 ns 6.61 ns 5.86 ns 630.5 ns 623.3 ns 642.4 ns 0.1691 - - 1.05 KB
DeserializeFromUtf8Bytes 494.2 ns 6.63 ns 6.21 ns 494.9 ns 483.8 ns 505.7 ns 0.1702 - - 1.05 KB
DeserializeFromStream 802.9 ns 5.92 ns 5.25 ns 802.5 ns 793.5 ns 811.6 ns 0.1805 - - 1.12 KB
Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 613.5 ns 5.62 ns 4.98 ns 614.1 ns 604.5 ns 621.6 ns 0.1708 - - 1.05 KB
DeserializeFromUtf8Bytes 474.7 ns 5.66 ns 5.30 ns 474.6 ns 465.5 ns 484.4 ns 0.1700 - - 1.05 KB
DeserializeFromStream 797.8 ns 8.74 ns 8.18 ns 798.8 ns 785.3 ns 811.6 ns 0.1823 - - 1.12 KB

Dictionary_string, string

Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 21.51 μs 0.140 μs 0.124 μs 21.51 μs 21.32 μs 21.73 μs 4.0650 0.5081 - 25.27 KB
DeserializeFromUtf8Bytes 21.27 μs 0.082 μs 0.072 μs 21.26 μs 21.16 μs 21.41 μs 4.0671 0.4980 - 25.27 KB
DeserializeFromStream 23.27 μs 0.309 μs 0.289 μs 23.17 μs 22.88 μs 23.92 μs 4.0980 0.4657 - 25.34 KB
Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 21.77 μs 0.277 μs 0.259 μs 21.71 μs 21.36 μs 22.22 μs 4.0461 0.5165 - 25.27 KB
DeserializeFromUtf8Bytes 21.25 μs 0.312 μs 0.292 μs 21.26 μs 20.83 μs 21.82 μs 4.1052 0.5027 - 25.27 KB
DeserializeFromStream 22.78 μs 0.129 μs 0.121 μs 22.78 μs 22.61 μs 23.06 μs 4.1248 0.4484 - 25.34 KB

HashSet_String

Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 12.47 μs 0.109 μs 0.096 μs 12.48 μs 12.33 μs 12.64 μs 2.3944 0.1955 - 14.87 KB
DeserializeFromUtf8Bytes 12.30 μs 0.104 μs 0.093 μs 12.33 μs 12.13 μs 12.43 μs 2.4171 0.1973 - 14.87 KB
DeserializeFromStream 13.76 μs 0.171 μs 0.160 μs 13.79 μs 13.49 μs 14.02 μs 2.4246 0.1616 - 14.94 KB
Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 12.80 μs 0.077 μs 0.072 μs 12.82 μs 12.67 μs 12.96 μs 2.4155 0.2013 - 14.87 KB
DeserializeFromUtf8Bytes 11.90 μs 0.113 μs 0.106 μs 11.87 μs 11.73 μs 12.12 μs 2.3819 0.1905 - 14.87 KB
DeserializeFromStream 13.69 μs 0.102 μs 0.096 μs 13.69 μs 13.52 μs 13.82 μs 2.4288 0.1619 - 14.94 KB

HashTable

Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 64.86 μs 0.419 μs 0.371 μs 64.95 μs 64.28 μs 65.30 μs 7.0699 1.3092 - 44.22 KB
DeserializeFromUtf8Bytes 65.87 μs 0.531 μs 0.497 μs 65.79 μs 64.76 μs 66.66 μs 7.0112 1.2984 - 44.22 KB
DeserializeFromStream 66.66 μs 0.435 μs 0.385 μs 66.60 μs 66.08 μs 67.32 μs 7.1485 1.0590 - 44.29 KB
Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 65.16 μs 0.761 μs 0.712 μs 64.97 μs 64.18 μs 66.29 μs 7.1203 1.3186 - 44.22 KB
DeserializeFromUtf8Bytes 66.34 μs 0.585 μs 0.547 μs 66.47 μs 65.38 μs 67.23 μs 7.1109 1.3168 - 44.22 KB
DeserializeFromStream 65.35 μs 0.448 μs 0.397 μs 65.37 μs 64.51 μs 66.08 μs 7.0057 1.0379 - 44.29 KB

ImmutableDictionary_String, String

Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 45.44 μs 0.361 μs 0.320 μs 45.44 μs 44.74 μs 46.04 μs 5.0074 0.5564 - 31.61 KB
DeserializeFromUtf8Bytes 45.73 μs 0.368 μs 0.345 μs 45.75 μs 45.16 μs 46.27 μs 5.0725 0.7246 - 31.61 KB
DeserializeFromStream 48.37 μs 0.398 μs 0.372 μs 48.44 μs 47.82 μs 49.08 μs 5.0633 0.5842 - 31.68 KB
Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 45.23 μs 0.361 μs 0.337 μs 45.16 μs 44.76 μs 45.79 μs 5.0578 0.5419 - 31.61 KB
DeserializeFromUtf8Bytes 46.00 μs 0.284 μs 0.252 μs 46.02 μs 45.44 μs 46.29 μs 5.1471 0.7353 - 31.61 KB
DeserializeFromStream 48.39 μs 0.340 μs 0.318 μs 48.36 μs 47.78 μs 48.97 μs 5.0096 0.5780 - 31.68 KB

ImmutableSortedDictionary_String, String

Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 76.58 μs 0.780 μs 0.730 μs 76.50 μs 75.42 μs 77.94 μs 6.0680 0.6068 - 38.29 KB
DeserializeFromUtf8Bytes 72.56 μs 0.859 μs 0.804 μs 72.54 μs 71.49 μs 74.19 μs 6.0484 0.8641 - 38.29 KB
DeserializeFromStream 74.30 μs 0.514 μs 0.481 μs 74.07 μs 73.67 μs 75.19 μs 5.9701 0.5970 - 38.36 KB
Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 72.86 μs 0.692 μs 0.647 μs 73.13 μs 71.68 μs 73.74 μs 6.0484 0.8641 - 38.29 KB
DeserializeFromUtf8Bytes 74.66 μs 0.408 μs 0.361 μs 74.69 μs 73.89 μs 75.17 μs 6.0386 0.6039 - 38.29 KB
DeserializeFromStream 73.85 μs 0.336 μs 0.297 μs 73.86 μs 73.37 μs 74.60 μs 5.9719 0.5972 - 38.36 KB

IndexViewModel

Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 28.99 μs 0.241 μs 0.213 μs 28.97 μs 28.60 μs 29.40 μs 3.5789 0.3463 - 22.01 KB
DeserializeFromUtf8Bytes 28.33 μs 0.226 μs 0.200 μs 28.32 μs 28.02 μs 28.76 μs 3.4882 0.3376 - 22.01 KB
DeserializeFromStream 31.05 μs 0.374 μs 0.350 μs 31.01 μs 30.55 μs 31.62 μs 3.4969 0.3747 - 22.08 KB
Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 29.42 μs 0.273 μs 0.242 μs 29.42 μs 29.00 μs 29.86 μs 3.5469 0.3547 - 22.01 KB
DeserializeFromUtf8Bytes 28.02 μs 0.172 μs 0.144 μs 28.02 μs 27.74 μs 28.24 μs 3.5147 0.3401 - 22.01 KB
DeserializeFromStream 30.78 μs 0.261 μs 0.244 μs 30.70 μs 30.42 μs 31.18 μs 3.5526 0.3675 - 22.08 KB

Int32

Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 133.65 ns 0.635 ns 0.594 ns 133.60 ns 132.38 ns 134.46 ns - - - -
DeserializeFromUtf8Bytes 81.61 ns 0.301 ns 0.281 ns 81.59 ns 81.09 ns 81.96 ns - - - -
DeserializeFromStream 262.62 ns 2.082 ns 1.948 ns 262.14 ns 259.80 ns 266.31 ns 0.0107 - - 72 B
Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 136.19 ns 0.877 ns 0.821 ns 136.16 ns 134.82 ns 137.77 ns - - - -
DeserializeFromUtf8Bytes 81.36 ns 0.343 ns 0.321 ns 81.29 ns 80.65 ns 82.00 ns - - - -
DeserializeFromStream 253.83 ns 2.359 ns 2.207 ns 253.43 ns 250.68 ns 257.52 ns 0.0113 - - 72 B

LargeStructWithProperties

Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 1.142 μs 0.0103 μs 0.0091 μs 1.140 μs 1.127 μs 1.160 μs 0.0279 - - 200 B
DeserializeFromUtf8Bytes 1.069 μs 0.0063 μs 0.0059 μs 1.069 μs 1.059 μs 1.081 μs 0.0298 - - 200 B
DeserializeFromStream 1.596 μs 0.0142 μs 0.0132 μs 1.590 μs 1.581 μs 1.620 μs 0.0499 - - 328 B
Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 1.157 μs 0.0076 μs 0.0071 μs 1.158 μs 1.147 μs 1.172 μs 0.0276 - - 200 B
DeserializeFromUtf8Bytes 1.081 μs 0.0065 μs 0.0058 μs 1.081 μs 1.071 μs 1.088 μs 0.0302 - - 200 B
DeserializeFromStream 1.498 μs 0.0094 μs 0.0088 μs 1.498 μs 1.483 μs 1.516 μs 0.0475 - - 328 B

Location

Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 1.259 μs 0.0092 μs 0.0086 μs 1.260 μs 1.246 μs 1.277 μs 0.0698 - - 448 B
DeserializeFromUtf8Bytes 1.153 μs 0.0112 μs 0.0105 μs 1.155 μs 1.135 μs 1.171 μs 0.0689 - - 448 B
DeserializeFromStream 1.550 μs 0.0104 μs 0.0098 μs 1.548 μs 1.534 μs 1.568 μs 0.0815 - - 520 B
Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 1.240 μs 0.0085 μs 0.0079 μs 1.240 μs 1.221 μs 1.253 μs 0.0695 - - 448 B
DeserializeFromUtf8Bytes 1.130 μs 0.0112 μs 0.0105 μs 1.133 μs 1.109 μs 1.143 μs 0.0682 - - 448 B
DeserializeFromStream 1.534 μs 0.0163 μs 0.0153 μs 1.538 μs 1.512 μs 1.563 μs 0.0797 - - 520 B

LoginViewModel

Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 468.6 ns 3.98 ns 3.72 ns 469.0 ns 464.0 ns 475.4 ns 0.0261 - - 168 B
DeserializeFromUtf8Bytes 444.6 ns 4.95 ns 4.63 ns 442.9 ns 439.1 ns 455.1 ns 0.0255 - - 168 B
DeserializeFromStream 727.3 ns 5.57 ns 5.21 ns 727.9 ns 719.8 ns 735.3 ns 0.0375 - - 240 B
Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 464.8 ns 1.69 ns 1.58 ns 464.9 ns 462.0 ns 467.4 ns 0.0257 - - 168 B
DeserializeFromUtf8Bytes 451.0 ns 3.38 ns 3.16 ns 451.4 ns 443.2 ns 454.8 ns 0.0266 - - 168 B
DeserializeFromStream 702.5 ns 4.70 ns 4.17 ns 701.3 ns 696.4 ns 710.7 ns 0.0364 - - 240 B

MyEventListerViewModel

Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 324.4 μs 2.10 μs 1.97 μs 324.2 μs 320.7 μs 328.0 μs 11.6129 2.5806 - 76.9 KB
DeserializeFromUtf8Bytes 304.2 μs 1.40 μs 1.24 μs 304.2 μs 302.5 μs 306.6 μs 12.1359 3.6408 - 76.9 KB
DeserializeFromStream 363.7 μs 1.48 μs 1.24 μs 363.6 μs 361.9 μs 366.4 μs 11.5774 2.8944 - 77.74 KB
Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 330.9 μs 2.66 μs 2.49 μs 331.2 μs 326.4 μs 335.2 μs 11.8577 2.6350 - 76.9 KB
DeserializeFromUtf8Bytes 306.6 μs 2.11 μs 1.76 μs 307.1 μs 302.6 μs 308.5 μs 11.6732 2.5940 - 76.9 KB
DeserializeFromStream 361.3 μs 3.36 μs 3.14 μs 360.7 μs 356.7 μs 366.3 μs 11.5607 2.8902 - 77.75 KB

SimpleStructWithProperties

Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 331.8 ns 2.86 ns 2.68 ns 332.3 ns 326.0 ns 335.3 ns 0.0092 - - 64 B
DeserializeFromUtf8Bytes 275.7 ns 1.93 ns 1.61 ns 276.2 ns 273.1 ns 278.4 ns 0.0091 - - 64 B
DeserializeFromStream 531.0 ns 4.21 ns 3.94 ns 531.4 ns 524.9 ns 538.0 ns 0.0212 - - 144 B
Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
DeserializeFromString 332.5 ns 2.25 ns 1.88 ns 332.3 ns 329.2 ns 335.4 ns 0.0095 - - 64 B
DeserializeFromUtf8Bytes 299.5 ns 1.80 ns 1.68 ns 299.1 ns 297.1 ns 302.1 ns 0.0097 - - 64 B
DeserializeFromStream 517.6 ns 5.21 ns 4.87 ns 516.2 ns 510.3 ns 528.9 ns 0.0226 - - 144 B

layomia
layomia reviewed Sep 25, 2020
View reviewed changes
Copy link
Contributor

@layomia layomia left a comment

Choose a reason for hiding this comment

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

Thanks for sharing the benchmarks. The change is close to ready, I just have a few comments on the tests.

...libraries/System.Text.Json/tests/Serialization/CollectionTests/CollectionTests.Dictionary.cs
}

{
GenericStructIDictionaryWrapper<string, string>? obj = JsonSerializer.Deserialize<GenericStructIDictionaryWrapper<string, string>?>("null");
Copy link
Contributor

@layomia layomia Sep 25, 2020

Choose a reason for hiding this comment

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

What happens when serializing a default instance of GenericStructIDictionaryWrapper<string, string>? Test this for GenericStructIListWrapper<int> as well.

@RaymondHuy
Add test cases for struct collection:
d935b04 
- Serializing a default instance.
- Deserializing a null json string.
@RaymondHuy
Copy link
Contributor Author

RaymondHuy commented Sep 27, 2020

@layomia Thanks for your comments. I've updated my PR, can you check if it is satisfied ?

layomia
layomia approved these changes Sep 27, 2020
View reviewed changes
Copy link
Contributor

@layomia layomia 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!

@layomia layomia merged commit 08472ef into dotnet:master Sep 27, 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.

Reviewers

@jozkee jozkee Awaiting requested review from jozkee

@steveharter steveharter Awaiting requested review from steveharter

2 more reviewers

@devsko devsko devsko left review comments

@layomia layomia layomia approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area-System.Text.Json

Projects

None yet

Milestone

6.0.0

Development

Successfully merging this pull request may close these issues.

Struct properties that implement IList<T> are unable to hold values on Deserialization

7 participants

@RaymondHuy @Dotnet-GitSync-Bot @dnfadmin @steveharter @layomia @devsko @jkotas