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

Use hashcodes when looking up the JsonSerializerOptions global cache. #76782

Merged
merged 2 commits into from
Oct 17, 2022
Merged

Use hashcodes when looking up the JsonSerializerOptions global cache. #76782

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4830043
Use hashcodes when looking up the JsonSerializerOptions global cache.
eiriktsarpalis Oct 8, 2022
1bc2d1c
Address feedback.
eiriktsarpalis Oct 9, 2022
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
141 changes: 98 additions & 43 deletions ...ries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,12 +142,14 @@ internal sealed class CachingContext
{
private readonly ConcurrentDictionary<Type, JsonTypeInfo?> _jsonTypeInfoCache = new();

public CachingContext(JsonSerializerOptions options)
public CachingContext(JsonSerializerOptions options, int hashCode)
{
Options = options;
HashCode = hashCode;
}

public JsonSerializerOptions Options { get; }
public int HashCode { get; }
// Property only accessed by reflection in testing -- do not remove.
// If changing please ensure that src/ILLink.Descriptors.LibraryBuild.xml is up-to-date.
public int Count => _jsonTypeInfoCache.Count;
Expand All @@ -164,37 +166,39 @@ public void Clear()
/// <summary>
/// Defines a cache of CachingContexts; instead of using a ConditionalWeakTable which can be slow to traverse
/// this approach uses a fixed-size array of weak references of <see cref="CachingContext"/> that can be looked up lock-free.
/// Relevant caching contexts are looked up by linear traversal using the equality comparison defined by
/// <see cref="AreEquivalentOptions(JsonSerializerOptions, JsonSerializerOptions)"/>.
/// Relevant caching contexts are looked up by linear traversal using the equality comparison defined by <see cref="EqualityComparer"/>.
/// </summary>
internal static class TrackedCachingContexts
{
private const int MaxTrackedContexts = 64;
private static readonly WeakReference<CachingContext>?[] s_trackedContexts = new WeakReference<CachingContext>[MaxTrackedContexts];
private static readonly EqualityComparer s_optionsComparer = new();
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved

public static CachingContext GetOrCreate(JsonSerializerOptions options)
{
Debug.Assert(options.IsReadOnly, "Cannot create caching contexts for mutable JsonSerializerOptions instances");
Debug.Assert(options._typeInfoResolver != null);

if (TryGetContext(options, out int firstUnpopulatedIndex, out CachingContext? result))
int hashCode = s_optionsComparer.GetHashCode(options);

if (TryGetContext(options, hashCode, out int firstUnpopulatedIndex, out CachingContext? result))
{
return result;
}
else if (firstUnpopulatedIndex < 0)
{
// Cache is full; return a fresh instance.
return new CachingContext(options);
return new CachingContext(options, hashCode);
}

lock (s_trackedContexts)
{
if (TryGetContext(options, out firstUnpopulatedIndex, out result))
if (TryGetContext(options, hashCode, out firstUnpopulatedIndex, out result))
{
return result;
}

var ctx = new CachingContext(options);
var ctx = new CachingContext(options, hashCode);

if (firstUnpopulatedIndex >= 0)
{
Expand All @@ -218,6 +222,7 @@ public static CachingContext GetOrCreate(JsonSerializerOptions options)

private static bool TryGetContext(
JsonSerializerOptions options,
int hashCode,
out int firstUnpopulatedIndex,
[NotNullWhen(true)] out CachingContext? result)
{
Expand All @@ -235,7 +240,7 @@ private static bool TryGetContext(
firstUnpopulatedIndex = i;
}
}
else if (AreEquivalentOptions(options, ctx.Options))
else if (hashCode == ctx.HashCode && s_optionsComparer.Equals(options, ctx.Options))
{
result = ctx;
return true;
Expand All @@ -252,52 +257,102 @@ private static bool TryGetContext(
/// If two instances are equivalent, they should generate identical metadata caches;
/// the converse however does not necessarily hold.
/// </summary>
private static bool AreEquivalentOptions(JsonSerializerOptions left, JsonSerializerOptions right)
private sealed class EqualityComparer : IEqualityComparer<JsonSerializerOptions>
{
Debug.Assert(left != null && right != null);

return
left._dictionaryKeyPolicy == right._dictionaryKeyPolicy &&
left._jsonPropertyNamingPolicy == right._jsonPropertyNamingPolicy &&
left._readCommentHandling == right._readCommentHandling &&
left._referenceHandler == right._referenceHandler &&
left._encoder == right._encoder &&
left._defaultIgnoreCondition == right._defaultIgnoreCondition &&
left._numberHandling == right._numberHandling &&
left._unknownTypeHandling == right._unknownTypeHandling &&
left._defaultBufferSize == right._defaultBufferSize &&
left._maxDepth == right._maxDepth &&
left._allowTrailingCommas == right._allowTrailingCommas &&
left._ignoreNullValues == right._ignoreNullValues &&
left._ignoreReadOnlyProperties == right._ignoreReadOnlyProperties &&
left._ignoreReadonlyFields == right._ignoreReadonlyFields &&
left._includeFields == right._includeFields &&
left._propertyNameCaseInsensitive == right._propertyNameCaseInsensitive &&
left._writeIndented == right._writeIndented &&
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
left._typeInfoResolver == right._typeInfoResolver &&
CompareLists(left._converters, right._converters);

static bool CompareLists<TValue>(ConfigurationList<TValue> left, ConfigurationList<TValue> right)
public bool Equals(JsonSerializerOptions? left, JsonSerializerOptions? right)
{
int n;
if ((n = left.Count) != right.Count)
Debug.Assert(left != null && right != null);

return
left._dictionaryKeyPolicy == right._dictionaryKeyPolicy &&
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
left._jsonPropertyNamingPolicy == right._jsonPropertyNamingPolicy &&
left._readCommentHandling == right._readCommentHandling &&
left._referenceHandler == right._referenceHandler &&
left._encoder == right._encoder &&
left._defaultIgnoreCondition == right._defaultIgnoreCondition &&
left._numberHandling == right._numberHandling &&
left._unknownTypeHandling == right._unknownTypeHandling &&
left._defaultBufferSize == right._defaultBufferSize &&
left._maxDepth == right._maxDepth &&
left._allowTrailingCommas == right._allowTrailingCommas &&
left._ignoreNullValues == right._ignoreNullValues &&
left._ignoreReadOnlyProperties == right._ignoreReadOnlyProperties &&
left._ignoreReadonlyFields == right._ignoreReadonlyFields &&
left._includeFields == right._includeFields &&
left._propertyNameCaseInsensitive == right._propertyNameCaseInsensitive &&
left._writeIndented == right._writeIndented &&
left._typeInfoResolver == right._typeInfoResolver &&
CompareLists(left._converters, right._converters);

static bool CompareLists<TValue>(ConfigurationList<TValue> left, ConfigurationList<TValue> right)
{
return false;
int n;
if ((n = left.Count) != right.Count)
{
return false;
}

for (int i = 0; i < n; i++)
{
TValue? leftElem = left[i];
TValue? rightElem = right[i];
bool areEqual = leftElem is null ? rightElem is null : leftElem.Equals(rightElem);
if (!areEqual)
{
return false;
}
}

return true;
}
}

for (int i = 0; i < n; i++)
public int GetHashCode(JsonSerializerOptions options)
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
{
HashCode hc = default;

hc.Add(options._dictionaryKeyPolicy);
hc.Add(options._jsonPropertyNamingPolicy);
hc.Add(options._readCommentHandling);
hc.Add(options._referenceHandler);
hc.Add(options._encoder);
hc.Add(options._defaultIgnoreCondition);
hc.Add(options._numberHandling);
hc.Add(options._unknownTypeHandling);
hc.Add(options._defaultBufferSize);
hc.Add(options._maxDepth);
hc.Add(options._allowTrailingCommas);
hc.Add(options._ignoreNullValues);
hc.Add(options._ignoreReadOnlyProperties);
hc.Add(options._ignoreReadonlyFields);
hc.Add(options._includeFields);
hc.Add(options._propertyNameCaseInsensitive);
hc.Add(options._writeIndented);
hc.Add(options._typeInfoResolver);
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
GetHashCode(ref hc, options._converters);

static void GetHashCode<TValue>(ref HashCode hc, ConfigurationList<TValue> list)
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
{
TValue? leftElem = left[i];
TValue? rightElem = right[i];
bool areEqual = leftElem is null ? rightElem is null : leftElem.Equals(rightElem);
if (!areEqual)
for (int i = 0; i < list.Count; i++)
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
{
return false;
hc.Add(list[i]);
}
}

return true;
return hc.ToHashCode();
}

#if !NETCOREAPP
/// <summary>
/// Polyfill for System.HashCode.
/// </summary>
private struct HashCode
{
private int _hashCode;
public void Add<T>(T? value) => _hashCode = (_hashCode, value).GetHashCode();
public int ToHashCode() => _hashCode;
}
#endif
}
}
}
35 changes: 22 additions & 13 deletions src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CacheTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ static Func<JsonSerializerOptions, int> CreateCacheCountAccessor()

[ActiveIssue("https://github.com/dotnet/runtime/issues/66232", TargetFrameworkMonikers.NetFramework)]
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
[MemberData(nameof(GetJsonSerializerOptions))]
public static void JsonSerializerOptions_ReuseConverterCaches()
{
// This test uses reflection to:
Expand All @@ -268,7 +269,7 @@ public static void JsonSerializerOptions_ReuseConverterCaches()
RemoteExecutor.Invoke(static () =>
{
Func<JsonSerializerOptions, JsonSerializerOptions?> getCacheOptions = CreateCacheOptionsAccessor();
Func<JsonSerializerOptions, JsonSerializerOptions, bool> equalityComparer = CreateEqualityComparerAccessor();
IEqualityComparer<JsonSerializerOptions> equalityComparer = CreateEqualityComparerAccessor();

foreach (var args in GetJsonSerializerOptions())
{
Expand All @@ -279,7 +280,8 @@ public static void JsonSerializerOptions_ReuseConverterCaches()

JsonSerializerOptions originalCacheOptions = getCacheOptions(options);
Assert.NotNull(originalCacheOptions);
Assert.True(equalityComparer(options, originalCacheOptions));
Assert.True(equalityComparer.Equals(options, originalCacheOptions));
Assert.Equal(equalityComparer.GetHashCode(options), equalityComparer.GetHashCode(originalCacheOptions));

for (int i = 0; i < 5; i++)
{
Expand All @@ -288,7 +290,8 @@ public static void JsonSerializerOptions_ReuseConverterCaches()

JsonSerializer.Serialize(42, options2);

Assert.True(equalityComparer(options2, originalCacheOptions));
Assert.True(equalityComparer.Equals(options2, originalCacheOptions));
Assert.Equal(equalityComparer.GetHashCode(options2), equalityComparer.GetHashCode(originalCacheOptions));
Assert.Same(originalCacheOptions, getCacheOptions(options2));
}
}
Expand Down Expand Up @@ -324,7 +327,7 @@ public static void JsonSerializerOptions_EqualityComparer_ChangingAnySettingShou
// - All public setters in JsonSerializerOptions
//
// If either of them changes, this test will need to be kept in sync.
Func<JsonSerializerOptions, JsonSerializerOptions, bool> equalityComparer = CreateEqualityComparerAccessor();
IEqualityComparer<JsonSerializerOptions> equalityComparer = CreateEqualityComparerAccessor();

(PropertyInfo prop, object value)[] propertySettersAndValues = GetPropertiesWithSettersAndNonDefaultValues().ToArray();

Expand All @@ -334,16 +337,19 @@ public static void JsonSerializerOptions_EqualityComparer_ChangingAnySettingShou
Assert.Fail($"{nameof(GetPropertiesWithSettersAndNonDefaultValues)} missing property declaration for {prop.Name}, please update the method.");
}

Assert.True(equalityComparer(JsonSerializerOptions.Default, JsonSerializerOptions.Default));
Assert.True(equalityComparer.Equals(JsonSerializerOptions.Default, JsonSerializerOptions.Default));
Assert.Equal(equalityComparer.GetHashCode(JsonSerializerOptions.Default), equalityComparer.GetHashCode(JsonSerializerOptions.Default));

foreach ((PropertyInfo prop, object? value) in propertySettersAndValues)
{
var options = new JsonSerializerOptions();
prop.SetValue(options, value);

Assert.True(equalityComparer(options, options));
Assert.True(equalityComparer.Equals(options, options));
Assert.Equal(equalityComparer.GetHashCode(options), equalityComparer.GetHashCode(options));

Assert.False(equalityComparer(JsonSerializerOptions.Default, options));
Assert.False(equalityComparer.Equals(JsonSerializerOptions.Default, options));
Assert.NotEqual(equalityComparer.GetHashCode(JsonSerializerOptions.Default), equalityComparer.GetHashCode(options));
}

static IEnumerable<(PropertyInfo, object)> GetPropertiesWithSettersAndNonDefaultValues()
Expand Down Expand Up @@ -389,14 +395,16 @@ public static void JsonSerializerOptions_EqualityComparer_ApplyingJsonSerializer
//
// If either of them changes, this test will need to be kept in sync.

Func<JsonSerializerOptions, JsonSerializerOptions, bool> equalityComparer = CreateEqualityComparerAccessor();
IEqualityComparer<JsonSerializerOptions> equalityComparer = CreateEqualityComparerAccessor();
var options1 = new JsonSerializerOptions { WriteIndented = true };
var options2 = new JsonSerializerOptions { WriteIndented = true };

Assert.True(equalityComparer(options1, options2));
Assert.True(equalityComparer.Equals(options1, options2));
Assert.Equal(equalityComparer.GetHashCode(options1), equalityComparer.GetHashCode(options2));

_ = new MyJsonContext(options1); // Associate copy with a JsonSerializerContext
Assert.False(equalityComparer(options1, options2));
Assert.False(equalityComparer.Equals(options1, options2));
Assert.NotEqual(equalityComparer.GetHashCode(options1), equalityComparer.GetHashCode(options2));
}

private class MyJsonContext : JsonSerializerContext
Expand All @@ -408,10 +416,11 @@ public MyJsonContext(JsonSerializerOptions options) : base(options) { }
protected override JsonSerializerOptions? GeneratedSerializerOptions => Options;
}

public static Func<JsonSerializerOptions, JsonSerializerOptions, bool> CreateEqualityComparerAccessor()
public static IEqualityComparer<JsonSerializerOptions> CreateEqualityComparerAccessor()
{
MethodInfo equalityComparerMethod = typeof(JsonSerializerOptions).GetMethod("AreEquivalentOptions", BindingFlags.NonPublic | BindingFlags.Static);
return (Func<JsonSerializerOptions, JsonSerializerOptions, bool>)Delegate.CreateDelegate(typeof(Func<JsonSerializerOptions, JsonSerializerOptions, bool>), equalityComparerMethod);
Type equalityComparerType = typeof(JsonSerializerOptions).GetNestedType("EqualityComparer", BindingFlags.NonPublic);
Assert.NotNull(equalityComparerType);
return (IEqualityComparer<JsonSerializerOptions>)Activator.CreateInstance(equalityComparerType, nonPublic: true);
}

public static IEnumerable<object[]> WriteSuccessCases
Expand Down