Skip to content

Commit

Permalink
Refactored the bugfix in accordance to the comments (dotnet#47765)
Browse files Browse the repository at this point in the history
  • Loading branch information
SkiFoD committed Jul 27, 2021
1 parent a6aebec commit 235c7ef
Showing 1 changed file with 45 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,12 @@ internal sealed class EnumConverter<T> : JsonConverter<T>

private readonly ConcurrentDictionary<ulong, JsonEncodedText> _nameCache;

private readonly Lazy<ConcurrentDictionary<ulong, JsonEncodedText>> _dictionaryKeyPolicyCache = new Lazy<ConcurrentDictionary<ulong, JsonEncodedText>>();
private ConcurrentDictionary<ulong, JsonEncodedText>? _dictionaryKeyPolicyCache;

// This is used to prevent flooding the cache due to exponential bitwise combinations of flags.
// Since multiple threads can add to the cache, a few more values might be added.
private const int NameCacheSizeSoftLimit = 64;

private const int DictionaryKeyPolicyCacheSizeSoftLimit = 64;

public override bool CanConvert(Type type)
{
return type.IsEnum;
Expand Down Expand Up @@ -329,20 +327,31 @@ internal override void WriteWithQuotes(Utf8JsonWriter writer, T value, JsonSeria

ulong key = ConvertToUInt64(value);

JsonEncodedText formatted;
string original;
//Try to obtain values from caches
if (options.DictionaryKeyPolicy != null)
{
Debug.Assert(!state.Current.IgnoreDictionaryKeyPolicy);

if (options.DictionaryKeyPolicy != null && !state.Current.IgnoreDictionaryKeyPolicy)
if (_dictionaryKeyPolicyCache != null && _dictionaryKeyPolicyCache.TryGetValue(key, out JsonEncodedText formatted))
{
writer.WritePropertyName(formatted);
return;
}
}
else
{
if (_dictionaryKeyPolicyCache.Value.TryGetValue(key, out formatted))
if (_nameCache.TryGetValue(key, out JsonEncodedText formatted))
{
writer.WritePropertyName(formatted);
return;
}
}

original = value.ToString();

if (IsValidIdentifier(original))
//if there are not cached values
string original = value.ToString();
if (IsValidIdentifier(original))
{
if (options.DictionaryKeyPolicy != null)
{
original = options.DictionaryKeyPolicy.ConvertName(original);

Expand All @@ -351,15 +360,20 @@ internal override void WriteWithQuotes(Utf8JsonWriter writer, T value, JsonSeria
ThrowHelper.ThrowInvalidOperationException_NamingPolicyReturnNull(options.DictionaryKeyPolicy);
}

if (_dictionaryKeyPolicyCache.Value.Count < DictionaryKeyPolicyCacheSizeSoftLimit)
if (_dictionaryKeyPolicyCache == null)
{
_dictionaryKeyPolicyCache = new ConcurrentDictionary<ulong, JsonEncodedText>();
}

if (_dictionaryKeyPolicyCache.Count < NameCacheSizeSoftLimit)
{
JavaScriptEncoder? encoder = options.Encoder;

formatted = JsonEncodedText.Encode(original, encoder);
JsonEncodedText formatted = JsonEncodedText.Encode(original, encoder);

writer.WritePropertyName(formatted);

_dictionaryKeyPolicyCache.Value.TryAdd(key, formatted);
_dictionaryKeyPolicyCache.TryAdd(key, formatted);
}
else
{
Expand All @@ -370,37 +384,29 @@ internal override void WriteWithQuotes(Utf8JsonWriter writer, T value, JsonSeria

return;
}
}

if (_nameCache.TryGetValue(key, out formatted))
{
writer.WritePropertyName(formatted);
return;
}
else
{
// We are dealing with a combination of flag constants since
// all constant values were cached during warm-up.
JavaScriptEncoder? encoder = options.Encoder;

original = value.ToString();
if (IsValidIdentifier(original))
{
// We are dealing with a combination of flag constants since
// all constant values were cached during warm-up.
JavaScriptEncoder? encoder = options.Encoder;
if (_nameCache.Count < NameCacheSizeSoftLimit)
{
JsonEncodedText formatted = JsonEncodedText.Encode(original, encoder);

if (_nameCache.Count < NameCacheSizeSoftLimit)
{
formatted = JsonEncodedText.Encode(original, encoder);
writer.WritePropertyName(formatted);

writer.WritePropertyName(formatted);
_nameCache.TryAdd(key, formatted);
}
else
{
// We also do not create a JsonEncodedText instance here because passing the string
// directly to the writer is cheaper than creating one and not caching it for reuse.
writer.WritePropertyName(original);
}

_nameCache.TryAdd(key, formatted);
}
else
{
// We also do not create a JsonEncodedText instance here because passing the string
// directly to the writer is cheaper than creating one and not caching it for reuse.
writer.WritePropertyName(original);
return;
}

return;
}

switch (s_enumTypeCode)
Expand Down

0 comments on commit 235c7ef

Please sign in to comment.