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.

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

Add support for multiple non-string TKey on dictionaries #38056

Merged
merged 18 commits into from
Jul 7, 2020
Merged

Add support for multiple non-string TKey on dictionaries #38056

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
f222f23
Add support for multiple non-string TKey on dictionaries
jozkee Jun 17, 2020
25e654b
Fix build errors
jozkee Jun 17, 2020
77b7a4a
Fix performance regression
jozkee Jun 19, 2020
225f27a
Merge branch 'master' of https://github.com/dotnet/runtime into tkey_…
jozkee Jun 19, 2020
6bcb25e
Add Read/WriteWithQuotes to the remaining supported types
jozkee Jun 22, 2020
c272684
Add support for a few more missing types
jozkee Jun 23, 2020
eb75a96
Add policy support for Enum keys
jozkee Jun 23, 2020
8d4fd95
Address performance regression.
jozkee Jun 24, 2020
f9b507c
Fix test error on netstandard
jozkee Jun 25, 2020
3f2a4e7
Merge branch 'master' of https://github.com/dotnet/runtime into tkey_…
jozkee Jun 25, 2020
578138e
1. Add support for escaped characters on Read and avoid escaping on W…
jozkee Jun 30, 2020
2d56bfb
Remove TryGet*Core methods to mitigate perf regression
jozkee Jun 30, 2020
637c68f
Remove duplicated escaping tests
jozkee Jun 30, 2020
7201ccb
Cache TKey and TValue converters
jozkee Jul 2, 2020
2b30299
Remove duplicated code used to read the dictionary key
jozkee Jul 2, 2020
ee17991
Bring back TryGet*Core
jozkee Jul 2, 2020
8cd787b
Fix test errors with floating point types
jozkee Jul 2, 2020
8b30e75
Address nits and feedback from steveharter
jozkee Jul 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/libraries/System.Text.Json/src/Resources/Strings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,4 +527,10 @@
<data name="DefaultIgnoreConditionInvalid" xml:space="preserve">
<value>The value cannot be 'JsonIgnoreCondition.Always'.</value>
</data>
</root>
<data name="FormatBoolean" xml:space="preserve">
<value>The JSON value is not in a supported Boolean format.</value>
</data>
<data name="DictionaryKeyTypeNotSupported" xml:space="preserve">
<value>The type '{0}' is not a supported Dictionary key type.</value>
</data>
</root>
jozkee marked this conversation as resolved.
Show resolved Hide resolved
8 changes: 4 additions & 4 deletions src/libraries/System.Text.Json/src/System.Text.Json.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,10 @@
<Compile Include="System\Text\Json\Serialization\Converters\Collection\ConcurrentQueueOfTConverter.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Collection\ConcurrentStackOfTConverter.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Collection\DictionaryDefaultConverter.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Collection\DictionaryOfStringTValueConverter.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Collection\DictionaryOfTKeyTValueConverter.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Collection\ICollectionOfTConverter.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Collection\IDictionaryConverter.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Collection\IDictionaryOfStringTValueConverter.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Collection\IDictionaryOfTKeyTValueConverter.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Collection\IEnumerableConverter.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Collection\IEnumerableConverterFactory.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Collection\IEnumerableConverterFactoryHelpers.cs" />
Expand All @@ -78,9 +78,9 @@
<Compile Include="System\Text\Json\Serialization\Converters\Collection\IEnumerableWithAddMethodConverter.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Collection\IListConverter.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Collection\IListOfTConverter.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Collection\ImmutableDictionaryOfStringTValueConverter.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Collection\ImmutableDictionaryOfTKeyTValueConverter.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Collection\ImmutableEnumerableOfTConverter.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Collection\IReadOnlyDictionaryOfStringTValueConverter.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Collection\IReadOnlyDictionaryOfTKeyTValueConverter.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Collection\ISetOfTConverter.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Collection\JsonCollectionConverter.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\Collection\JsonDictionaryConverter.cs" />
Expand Down
1 change: 1 addition & 0 deletions src/libraries/System.Text.Json/src/System/Text/Json/JsonConstants.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ internal static class JsonConstants
public const int MaxBase64ValueTokenSize = (MaxEscapedTokenSize >> 2) * 3 / MaxExpansionFactorWhileEscaping; // 125_000_000 bytes
public const int MaxCharacterTokenSize = MaxEscapedTokenSize / MaxExpansionFactorWhileEscaping; // 166_666_666 characters

public const int MaximumFormatBooleanLength = 5;
public const int MaximumFormatInt64Length = 20; // 19 + sign (i.e. -9223372036854775808)
public const int MaximumFormatUInt64Length = 20; // i.e. 18446744073709551615
public const int MaximumFormatDoubleLength = 128; // default (i.e. 'G'), using 128 (rather than say 32) to be future-proof.
Expand Down
71 changes: 71 additions & 0 deletions src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.TryGet.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,11 @@ public bool TryGetByte(out byte value)
throw ThrowHelper.GetInvalidOperationException_ExpectedNumber(TokenType);
}

return TryGetByteCore(out value);
}

internal bool TryGetByteCore(out byte value)
{
ReadOnlySpan<byte> span = HasValueSequence ? ValueSequence.ToArray() : ValueSpan;
if (Utf8Parser.TryParse(span, out byte tmp, out int bytesConsumed)
&& span.Length == bytesConsumed)
Expand Down Expand Up @@ -525,6 +530,11 @@ public bool TryGetSByte(out sbyte value)
throw ThrowHelper.GetInvalidOperationException_ExpectedNumber(TokenType);
}

return TryGetSByteCore(out value);
}

internal bool TryGetSByteCore(out sbyte value)
{
ReadOnlySpan<byte> span = HasValueSequence ? ValueSequence.ToArray() : ValueSpan;
if (Utf8Parser.TryParse(span, out sbyte tmp, out int bytesConsumed)
&& span.Length == bytesConsumed)
Expand Down Expand Up @@ -554,6 +564,11 @@ public bool TryGetInt16(out short value)
throw ThrowHelper.GetInvalidOperationException_ExpectedNumber(TokenType);
}

return TryGetInt16Core(out value);
}

internal bool TryGetInt16Core(out short value)
{
ReadOnlySpan<byte> span = HasValueSequence ? ValueSequence.ToArray() : ValueSpan;
if (Utf8Parser.TryParse(span, out short tmp, out int bytesConsumed)
&& span.Length == bytesConsumed)
Expand Down Expand Up @@ -583,6 +598,11 @@ public bool TryGetInt32(out int value)
throw ThrowHelper.GetInvalidOperationException_ExpectedNumber(TokenType);
}

return TryGetInt32Core(out value);
}

internal bool TryGetInt32Core(out int value)
{
ReadOnlySpan<byte> span = HasValueSequence ? ValueSequence.ToArray() : ValueSpan;
if (Utf8Parser.TryParse(span, out int tmp, out int bytesConsumed)
&& span.Length == bytesConsumed)
Expand Down Expand Up @@ -612,6 +632,11 @@ public bool TryGetInt64(out long value)
throw ThrowHelper.GetInvalidOperationException_ExpectedNumber(TokenType);
}

return TryGetInt64Core(out value);
}

internal bool TryGetInt64Core(out long value)
{
ReadOnlySpan<byte> span = HasValueSequence ? ValueSequence.ToArray() : ValueSpan;
if (Utf8Parser.TryParse(span, out long tmp, out int bytesConsumed)
&& span.Length == bytesConsumed)
Expand Down Expand Up @@ -642,6 +667,11 @@ public bool TryGetUInt16(out ushort value)
throw ThrowHelper.GetInvalidOperationException_ExpectedNumber(TokenType);
}

return TryGetUInt16Core(out value);
}

internal bool TryGetUInt16Core(out ushort value)
{
ReadOnlySpan<byte> span = HasValueSequence ? ValueSequence.ToArray() : ValueSpan;
if (Utf8Parser.TryParse(span, out ushort tmp, out int bytesConsumed)
&& span.Length == bytesConsumed)
Expand Down Expand Up @@ -672,6 +702,11 @@ public bool TryGetUInt32(out uint value)
throw ThrowHelper.GetInvalidOperationException_ExpectedNumber(TokenType);
}

return TryGetUInt32Core(out value);
}

internal bool TryGetUInt32Core(out uint value)
{
ReadOnlySpan<byte> span = HasValueSequence ? ValueSequence.ToArray() : ValueSpan;
if (Utf8Parser.TryParse(span, out uint tmp, out int bytesConsumed)
&& span.Length == bytesConsumed)
Expand Down Expand Up @@ -702,6 +737,11 @@ public bool TryGetUInt64(out ulong value)
throw ThrowHelper.GetInvalidOperationException_ExpectedNumber(TokenType);
}

return TryGetUInt64Core(out value);
}

internal bool TryGetUInt64Core(out ulong value)
{
ReadOnlySpan<byte> span = HasValueSequence ? ValueSequence.ToArray() : ValueSpan;
if (Utf8Parser.TryParse(span, out ulong tmp, out int bytesConsumed)
&& span.Length == bytesConsumed)
Expand Down Expand Up @@ -731,6 +771,11 @@ public bool TryGetSingle(out float value)
throw ThrowHelper.GetInvalidOperationException_ExpectedNumber(TokenType);
}

return TryGetSingleCore(out value);
}

internal bool TryGetSingleCore(out float value)
{
ReadOnlySpan<byte> span = HasValueSequence ? ValueSequence.ToArray() : ValueSpan;
if (Utf8Parser.TryParse(span, out float tmp, out int bytesConsumed, _numberFormat)
&& span.Length == bytesConsumed)
Expand Down Expand Up @@ -760,6 +805,11 @@ public bool TryGetDouble(out double value)
throw ThrowHelper.GetInvalidOperationException_ExpectedNumber(TokenType);
}

return TryGetDoubleCore(out value);
}

internal bool TryGetDoubleCore(out double value)
{
ReadOnlySpan<byte> span = HasValueSequence ? ValueSequence.ToArray() : ValueSpan;
if (Utf8Parser.TryParse(span, out double tmp, out int bytesConsumed, _numberFormat)
&& span.Length == bytesConsumed)
Expand Down Expand Up @@ -789,6 +839,11 @@ public bool TryGetDecimal(out decimal value)
throw ThrowHelper.GetInvalidOperationException_ExpectedNumber(TokenType);
}

return TryGetDecimalCore(out value);
}

internal bool TryGetDecimalCore(out decimal value)
{
ReadOnlySpan<byte> span = HasValueSequence ? ValueSequence.ToArray() : ValueSpan;
if (Utf8Parser.TryParse(span, out decimal tmp, out int bytesConsumed, _numberFormat)
&& span.Length == bytesConsumed)
Expand Down Expand Up @@ -818,6 +873,12 @@ public bool TryGetDateTime(out DateTime value)
throw ThrowHelper.GetInvalidOperationException_ExpectedString(TokenType);
}

return TryGetDateTimeCore(out value);
}

internal bool TryGetDateTimeCore(out DateTime value)
{

ReadOnlySpan<byte> span = stackalloc byte[0];

if (HasValueSequence)
Expand Down Expand Up @@ -881,6 +942,11 @@ public bool TryGetDateTimeOffset(out DateTimeOffset value)
throw ThrowHelper.GetInvalidOperationException_ExpectedString(TokenType);
}

return TryGetDateTimeOffsetCore(out value);
}

internal bool TryGetDateTimeOffsetCore(out DateTimeOffset value)
{
ReadOnlySpan<byte> span = stackalloc byte[0];

if (HasValueSequence)
Expand Down Expand Up @@ -945,6 +1011,11 @@ public bool TryGetGuid(out Guid value)
throw ThrowHelper.GetInvalidOperationException_ExpectedString(TokenType);
}

return TryGetGuidCore(out value);
}

internal bool TryGetGuidCore(out Guid value)
{
ReadOnlySpan<byte> span = stackalloc byte[0];

if (HasValueSequence)
Expand Down
93 changes: 68 additions & 25 deletions ...on/src/System/Text/Json/Serialization/Converters/Collection/DictionaryDefaultConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,14 @@ namespace System.Text.Json.Serialization.Converters
/// <summary>
/// Default base class implementation of <cref>JsonDictionaryConverter{TCollection}</cref> .
/// </summary>
internal abstract class DictionaryDefaultConverter<TCollection, TValue>
internal abstract class DictionaryDefaultConverter<TCollection, TKey, TValue>
: JsonDictionaryConverter<TCollection>
where TKey : notnull
{
/// <summary>
/// When overridden, adds the value to the collection.
/// </summary>
protected abstract void Add(in TValue value, JsonSerializerOptions options, ref ReadStack state);
protected abstract void Add(in TKey key, in TValue value, JsonSerializerOptions options, ref ReadStack state);

/// <summary>
/// When overridden, converts the temporary collection held in state.Current.ReturnValue to the final collection.
Expand All @@ -31,6 +32,11 @@ protected virtual void CreateCollection(ref Utf8JsonReader reader, ref ReadStack

internal override Type ElementType => typeof(TValue);

protected Type KeyType = typeof(TKey);
// For string keys we don't use a key converter
// in order to avoid performance regression on already supported types.
protected bool IsStringKey = typeof(TKey) == typeof(string);

protected static JsonConverter<TValue> GetElementConverter(ref ReadStack state)
{
JsonConverter<TValue> converter = (JsonConverter<TValue>)state.Current.JsonClassInfo.ElementClassInfo!.PropertyInfoForClassInfo.ConverterBase;
Expand All @@ -39,29 +45,17 @@ protected static JsonConverter<TValue> GetElementConverter(ref ReadStack state)
return converter;
}

protected string GetKeyName(string key, ref WriteStack state, JsonSerializerOptions options)
{
if (options.DictionaryKeyPolicy != null && !state.Current.IgnoreDictionaryKeyPolicy)
{
key = options.DictionaryKeyPolicy.ConvertName(key);

if (key == null)
{
ThrowHelper.ThrowInvalidOperationException_NamingPolicyReturnNull(options.DictionaryKeyPolicy);
}
}

return key;
}

protected static JsonConverter<TValue> GetValueConverter(ref WriteStack state)
jozkee marked this conversation as resolved.
Show resolved Hide resolved
{
JsonConverter<TValue> converter = (JsonConverter<TValue>)state.Current.DeclaredJsonPropertyInfo!.ConverterBase;
JsonConverter<TValue> converter = (JsonConverter<TValue>)state.Current.JsonClassInfo.ElementClassInfo!.PropertyInfoForClassInfo.ConverterBase;
Debug.Assert(converter != null); // It should not be possible to have a null converter at this point.

return converter;
}

protected JsonConverter<TKey> GetKeyConverter(JsonSerializerOptions options)
layomia marked this conversation as resolved.
Show resolved Hide resolved
=> (JsonConverter<TKey>)options.GetDictionaryKeyConverter(KeyType);

internal sealed override bool OnTryRead(
ref Utf8JsonReader reader,
Type typeToConvert,
Expand Down Expand Up @@ -97,12 +91,27 @@ internal sealed override bool OnTryRead(
// Read method would have thrown if otherwise.
Debug.Assert(reader.TokenType == JsonTokenType.PropertyName);

state.Current.JsonPropertyNameAsString = reader.GetString();
TKey key;
string unescapedPropertyNameAsString;
if (IsStringKey)
{
object keyAsObject = reader.GetString()!;
key = (TKey)keyAsObject;
unescapedPropertyNameAsString = (string)keyAsObject;
jozkee marked this conversation as resolved.
Show resolved Hide resolved
}
else
{
JsonConverter<TKey> keyConverter = GetKeyConverter(options);
key = keyConverter.ReadWithQuotes(ref reader);
unescapedPropertyNameAsString = reader.GetString()!;
}

// Copy key name for JSON Path support in case of error.
state.Current.JsonPropertyNameAsString = unescapedPropertyNameAsString;
// Read the value and add.
reader.ReadWithVerify();
TValue element = elementConverter.Read(ref reader, typeof(TValue), options);
Add(element!, options, ref state);
Add(in key, in element!, options, ref state);
}
}
else
Expand All @@ -121,13 +130,29 @@ internal sealed override bool OnTryRead(
// Read method would have thrown if otherwise.
Debug.Assert(reader.TokenType == JsonTokenType.PropertyName);

state.Current.JsonPropertyNameAsString = reader.GetString();
// Copy key name for JSON Path support in case of error.
string unescapedPropertyNameAsString;

TKey key;
if (IsStringKey)
{
object keyAsObject = reader.GetString()!;
key = (TKey)keyAsObject;
unescapedPropertyNameAsString = (string)keyAsObject;
}
else
{
JsonConverter<TKey> keyConverter = GetKeyConverter(options);
key = keyConverter.ReadWithQuotes(ref reader);
unescapedPropertyNameAsString = reader.GetString()!;
}

state.Current.JsonPropertyNameAsString = unescapedPropertyNameAsString;
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks identical to the block in the if case above. Can this be extracted into a method?

Copy link
Member Author

Choose a reason for hiding this comment

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

Extracting this into an instance method was causing a perf. decrease; I tried using AggresiveInlining but no luck.

Copy link
Member Author

Choose a reason for hiding this comment

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

For now I moved this code into a local method since that seems to regress less the performance.
There might be a reason why the method doesn't get inlined (even with AggressiveInlining) but for now these are the perf results:

worse: 8, geomean: 1.029
total diff: 8

Slower diff/base Base Median (ns) Diff Median (ns) Modality
System.Text.Json.Serialization.Tests.ReadJson<Dictionary<String, String>>.Deseri 1.05 21061.70 22172.59
System.Text.Json.Serialization.Tests.ReadJson<ImmutableSortedDictionary<String, 1.04 70776.79 73581.40
System.Text.Json.Serialization.Tests.ReadJson.DeserializeFromStream 1.04 62335.99 64625.97
System.Text.Json.Serialization.Tests.ReadJson<Dictionary<String, String>>.Deseri 1.03 19661.33 20254.28
System.Text.Json.Serialization.Tests.ReadJson<ImmutableSortedDictionary<String, 1.03 69778.78 71658.05
System.Text.Json.Serialization.Tests.ReadJson<ImmutableDictionary<String, String 1.03 46107.37 47298.85
System.Text.Json.Serialization.Tests.ReadJson<ImmutableDictionary<String, String 1.02 44138.14 44838.34
System.Text.Json.Serialization.Tests.ReadJson.DeserializeFromUtf8Byte 1.01 62344.90 62836.07

No Faster results for the provided threshold = 0% and noise filter = 0.3ns.

reader.ReadWithVerify();

// Get the value from the converter and add it.
elementConverter.TryRead(ref reader, typeof(TValue), options, ref state, out TValue element);
Add(element!, options, ref state);
Add(in key, in element!, options, ref state);
}
}
}
Expand Down Expand Up @@ -212,7 +237,6 @@ internal sealed override bool OnTryRead(

state.Current.PropertyState = StackFramePropertyState.Name;

// Verify property doesn't contain metadata.
if (preserveReferences)
{
ReadOnlySpan<byte> propertyName = reader.GetSpan();
Expand All @@ -222,7 +246,25 @@ internal sealed override bool OnTryRead(
}
}

state.Current.JsonPropertyNameAsString = reader.GetString();
// Copy key name for JSON Path support in case of error.
string unescapedPropertyNameAsString;

TKey key;
if (IsStringKey)
{
object keyAsObject = reader.GetString()!;
key = (TKey)keyAsObject;
unescapedPropertyNameAsString = (string)keyAsObject;
}
else
{
JsonConverter<TKey> keyConverter = GetKeyConverter(options);
key = keyConverter.ReadWithQuotes(ref reader);
unescapedPropertyNameAsString = reader.GetString()!;
}

state.Current.JsonPropertyNameAsString = unescapedPropertyNameAsString;
layomia marked this conversation as resolved.
Show resolved Hide resolved
state.Current.DictionaryKey = key;
}

if (state.Current.PropertyState < StackFramePropertyState.ReadValue)
Expand All @@ -246,7 +288,8 @@ internal sealed override bool OnTryRead(
return false;
}

Add(element!, options, ref state);
TKey key = (TKey)state.Current.DictionaryKey!;
Add(in key, in element!, options, ref state);
state.Current.EndElement();
}
}
Expand Down
Loading