This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
Add support for immutable dictionaries #37710
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
danmoseley
reviewed
May 16, 2019
src/System.Text.Json/src/System/Text/Json/Serialization/ClassMaterializer.cs
Show resolved
Hide resolved
danmoseley
reviewed
May 16, 2019
src/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleArray.cs
Outdated
Show resolved
Hide resolved
danmoseley
reviewed
May 16, 2019
danmoseley
reviewed
May 16, 2019
|
cc @rynowak |
rynowak
reviewed
May 17, 2019
src/System.Text.Json/src/System/Text/Json/Serialization/ClassMaterializer.cs
Outdated
Show resolved
Hide resolved
rynowak
reviewed
May 17, 2019
src/System.Text.Json/src/System/Text/Json/Serialization/ClassMaterializer.cs
Show resolved
Hide resolved
rynowak
reviewed
May 17, 2019
...on/src/System/Text/Json/Serialization/Converters/DefaultIEnumerableConstructibleConverter.cs
Outdated
Show resolved
Hide resolved
rynowak
reviewed
May 17, 2019
...on/src/System/Text/Json/Serialization/Converters/DefaultIEnumerableConstructibleConverter.cs
Outdated
Show resolved
Hide resolved
src/System.Text.Json/src/System/Text/Json/Serialization/Converters/DefaultImmutableConverter.cs
Outdated
Show resolved
Hide resolved
src/System.Text.Json/src/System/Text/Json/Serialization/Converters/DefaultImmutableConverter.cs
Show resolved
Hide resolved
src/System.Text.Json/src/System/Text/Json/Serialization/Converters/DefaultImmutableConverter.cs
Outdated
Show resolved
Hide resolved
src/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs
Show resolved
Hide resolved
src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs
Outdated
Show resolved
Hide resolved
src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoCommon.cs
Outdated
Show resolved
Hide resolved
src/System.Text.Json/src/System/Text/Json/Serialization/ReflectionEmitMaterializer.cs
Outdated
Show resolved
Hide resolved
steveharter
reviewed
May 17, 2019
src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoCommon.cs
Outdated
Show resolved
Hide resolved
src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoCommon.cs
Outdated
Show resolved
Hide resolved
src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoCommon.cs
Outdated
Show resolved
Hide resolved
src/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleArray.cs
Outdated
Show resolved
Hide resolved
steveharter
reviewed
May 22, 2019
...on/src/System/Text/Json/Serialization/Converters/DefaultIEnumerableConstructibleConverter.cs
Outdated
Show resolved
Hide resolved
|
@layomia was there a before\after benchmark performed? With a large feature like this we should verify there are no surprises. Thanks. |
steveharter
reviewed
May 22, 2019
src/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs
Outdated
Show resolved
Hide resolved
steveharter
reviewed
May 22, 2019
|
Some performance notes. TLDR
Hot-path deserialization and serialization use cases (Primitives, enumerables, objects)These benchmarks live in the perf repo. Before this change[IndexViewModel]
[Location]
[LoginViewModel]
[MyEventsListerViewModel]
After this change[IndexViewModel]
[Location]
[LoginViewModel]
[MyEventsListerViewModel]
Deserialization and serialization of
|
| Method | Mean | Error | StdDev | Median | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|
| DeserializeDict | 698.0 ns | 13.884 ns | 22.811 ns | 685.8 ns | 0.0992 | - | - | 416 B |
| DeserializeIDict | 689.9 ns | 6.155 ns | 4.805 ns | 690.4 ns | 0.0992 | - | - | 416 B |
| DeserializeIReadOnlyDict | 689.1 ns | 6.550 ns | 5.470 ns | 689.1 ns | 0.0992 | - | - | 416 B |
| SerializeDict | 824.9 ns | 18.945 ns | 18.607 ns | 821.0 ns | 0.0954 | - | - | 400 B |
| SerializeIDict | 790.2 ns | 15.665 ns | 19.812 ns | 784.8 ns | 0.0954 | - | - | 400 B |
| SerializeIReadOnlyDict | 826.1 ns | 19.380 ns | 28.406 ns | 820.8 ns | 0.0954 | - | - | 400 B |
After this change
| Method | Mean | Error | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|
| DeserializeDict | 731.5 ns | 14.684 ns | 21.524 ns | 0.0992 | - | - | 416 B |
| DeserializeIDict | 719.8 ns | 14.314 ns | 20.066 ns | 0.0992 | - | - | 416 B |
| DeserializeIReadOnlyDict | 721.6 ns | 14.251 ns | 24.582 ns | 0.0992 | - | - | 416 B |
| SerializeDict | 817.5 ns | 16.344 ns | 23.957 ns | 0.0954 | - | - | 400 B |
| SerializeIDict | 802.7 ns | 9.360 ns | 8.755 ns | 0.0954 | - | - | 400 B |
| SerializeIReadOnlyDict | 839.9 ns | 16.863 ns | 26.253 ns | 0.0954 | - | - | 400 B |
[Added by this PR] Deserialization and serialization of ImmutableDictionary, IImmutableDictionary, ImmutableSortedDictionary
The benchmarks are being added to the performance repo: dotnet/performance#530.
| Method | Mean | Error | StdDev | Median | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|
| DeserializeImmutableDict | 1.947 us | 0.0700 us | 0.1974 us | 1.876 us | 0.6332 | - | - | 2649 B |
| DeserializeIImmutableDict | 1.949 us | 0.0404 us | 0.1147 us | 1.895 us | 0.6332 | - | - | 2649 B |
| DeserializeImmutableSortedDict | 2.532 us | 0.0504 us | 0.1283 us | 2.527 us | 0.7591 | - | - | 3177 B |
| SerializeImmutableDict_ToBytes | 1.606 us | 0.0081 us | 0.0071 us | 1.606 us | 0.1793 | - | - | 752 B |
| SerializeIImmutableDict_ToBytes | 1.685 us | 0.0338 us | 0.0473 us | 1.677 us | 0.1793 | - | - | 752 B |
| SerializeImmutableSortedDict_ToBytes | 1.501 us | 0.0299 us | 0.0265 us | 1.507 us | 0.1564 | - | - | 656 B |
| SerializeImmutableDict_ToString | 1.721 us | 0.0271 us | 0.0290 us | 1.711 us | 0.1869 | - | - | 784 B |
| SerializeIImmutableDict_ToString | 1.709 us | 0.0234 us | 0.0182 us | 1.707 us | 0.1869 | - | - | 784 B |
| SerializeImmutableSortedDict_ToString | 1.357 us | 0.0112 us | 0.0094 us | 1.358 us | 0.1640 | - | - | 688 B |
Json.NET perf
| Method | Mean | Error | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|
| DeserializeDict | 948.3 ns | 5.016 ns | 4.447 ns | 0.7010 | - | - | 2.87 KB |
| DeserializeIDict | 959.0 ns | 6.703 ns | 5.942 ns | 0.7000 | - | - | 2.87 KB |
| DeserializeIReadOnlyDict | 1,010.8 ns | 2.997 ns | 2.804 ns | 0.7172 | - | - | 2.94 KB |
| SerializeDict_ToString | 593.5 ns | 1.524 ns | 1.351 ns | 0.3443 | - | - | 1.41 KB |
| SerializeIDict_ToString | 607.2 ns | 2.617 ns | 2.448 ns | 0.3443 | - | - | 1.41 KB |
| SerializeIReadOnlyDict_ToString | 598.5 ns | 2.760 ns | 2.581 ns | 0.3443 | - | - | 1.41 KB |
| DeserializeImmutableDict | 1,426.5 ns | 6.333 ns | 5.924 ns | 0.7629 | - | - | 3.12 KB |
| DeserializeIImmutableDict | 1,839.7 ns | 17.861 ns | 14.915 ns | 0.8793 | - | - | 3.59 KB |
| DeserializeImmutableSortedDict | 1,726.2 ns | 7.363 ns | 6.527 ns | 0.8793 | - | - | 3.59 KB |
| SerializeImmutableDict_ToString | 1,196.9 ns | 6.363 ns | 5.640 ns | 0.3967 | - | - | 1.63 KB |
| SerializeIImmutableDict_ToString | 1,192.0 ns | 3.545 ns | 3.316 ns | 0.3967 | - | - | 1.63 KB |
| SerializeImmutableSortedDict_ToString | 948.9 ns | 4.334 ns | 3.842 ns | 0.3748 | - | - | 1.53 KB |
[JsonExporterAttribute.Full]
[MemoryDiagnoser]
public class JsonDotNet_DictionaryPerf
{
private const string _jsonString = @"{""Hello"":""World"",""Hello2"":""World2""}";
private Dictionary<string, string> _dict = new Dictionary<string, string>() { { "Hello", "World" }, { "Hello2", "World2" } };
private static IDictionary<string, string> _iDict = new Dictionary<string, string>() { { "Hello", "World" }, { "Hello2", "World2" } };
private IReadOnlyDictionary<string, string> _iReadOnlyDict = new Dictionary<string, string>() { { "Hello", "World" }, { "Hello2", "World2" } };
private static ImmutableDictionary<string, string> _immutableDict;
private static IImmutableDictionary<string, string> _iimmutableDict;
private static ImmutableSortedDictionary<string, string> _immutableSortedDict;
[GlobalSetup]
public void Setup()
{
Dictionary<string, string> _dict = new Dictionary<string, string>() { { "Hello", "World" }, { "Hello2", "World2" } };
_immutableDict = ImmutableDictionary.CreateRange(_dict);
_iimmutableDict = ImmutableDictionary.CreateRange(_dict);
_immutableSortedDict = ImmutableSortedDictionary.CreateRange(_dict);
}
[Benchmark]
public Dictionary<string, string> DeserializeDict()
{
return JsonConvert.DeserializeObject<Dictionary<string, string>>(_jsonString);
}
[Benchmark]
public IDictionary<string, string> DeserializeIDict()
{
return JsonConvert.DeserializeObject<IDictionary<string, string>>(_jsonString);
}
[Benchmark]
public IReadOnlyDictionary<string, string> DeserializeIReadOnlyDict()
{
return JsonConvert.DeserializeObject<IReadOnlyDictionary<string, string>>(_jsonString);
}
[Benchmark]
public string SerializeDict_ToString()
{
return JsonConvert.SerializeObject(_dict);
}
[Benchmark]
public string SerializeIDict_ToString()
{
return JsonConvert.SerializeObject(_iDict);
}
[Benchmark]
public string SerializeIReadOnlyDict_ToString()
{
return JsonConvert.SerializeObject(_iReadOnlyDict);
}
[Benchmark]
public ImmutableDictionary<string, string> DeserializeImmutableDict()
{
return JsonConvert.DeserializeObject<ImmutableDictionary<string, string>>(_jsonString);
}
[Benchmark]
public IImmutableDictionary<string, string> DeserializeIImmutableDict()
{
return JsonConvert.DeserializeObject<IImmutableDictionary<string, string>>(_jsonString);
}
[Benchmark]
public ImmutableSortedDictionary<string, string> DeserializeImmutableSortedDict()
{
return JsonConvert.DeserializeObject<ImmutableSortedDictionary<string, string>>(_jsonString);
}
[Benchmark]
public string SerializeImmutableDict_ToString()
{
return JsonConvert.SerializeObject(_immutableDict);
}
[Benchmark]
public string SerializeIImmutableDict_ToString()
{
return JsonConvert.SerializeObject(_iimmutableDict);
}
[Benchmark]
public string SerializeImmutableSortedDict_ToString()
{
return JsonConvert.SerializeObject(_immutableSortedDict);
}
}
steveharter
approved these changes
May 31, 2019
|
The failing CI legs are unrelated: System.Text.RegularExpressions.Tests System.Net.Sockets.Tests - Windows.10.Amd64.ClientRS4.ES.Open-x64:Debug |
ahsonkhan
approved these changes
May 31, 2019
wtgodbe
pushed a commit
to dotnet-maestro-bot/corefx
that referenced
this pull request
May 31, 2019
* Add support for immutable dictionaries * Some clean up * Address review comments * Address review comments * Remove commented out code * Move object property infos to JsonSerializerOptions * Re-add immutable test * Cache IsImmutableDict bool * More immutable support * Modify processing enumerable or dictionary check * Correct handle end dictionary checks * Re-add dict tests * React to changes from master
picenka21
pushed a commit
to picenka21/runtime
that referenced
this pull request
Feb 18, 2022
* Add support for immutable dictionaries * Some clean up * Address review comments * Address review comments * Remove commented out code * Move object property infos to JsonSerializerOptions * Re-add immutable test * Cache IsImmutableDict bool * More immutable support * Modify processing enumerable or dictionary check * Correct handle end dictionary checks * Re-add dict tests * React to changes from master Commit migrated from dotnet/corefx@616b849
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Partially addresses https://github.com/dotnet/corefx/issues/36643.
ImmutableDictionary<TKey, TValue>IImmutableDictionary<TKey, TValue>ImmutableSortedDictionary<TKey, TValue>