From fe921e0b727abfb0a1b7cdecd41c91bfb6dc1f7f Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Mon, 17 Oct 2022 14:18:00 +0100 Subject: [PATCH] Use hashcodes when looking up the JsonSerializerOptions global cache. (#76782) * Use hashcodes when looking up the JsonSerializerOptions global cache. * Address feedback. --- .../JsonSerializerOptions.Caching.cs | 153 +++++++++++++----- .../Serialization/CacheTests.cs | 35 ++-- 2 files changed, 132 insertions(+), 56 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs index 7fc4c800e7243..af1c610e3ee57 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs @@ -142,12 +142,14 @@ internal sealed class CachingContext { private readonly ConcurrentDictionary _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; @@ -164,37 +166,39 @@ public void Clear() /// /// 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 that can be looked up lock-free. - /// Relevant caching contexts are looked up by linear traversal using the equality comparison defined by - /// . + /// Relevant caching contexts are looked up by linear traversal using the equality comparison defined by . /// internal static class TrackedCachingContexts { private const int MaxTrackedContexts = 64; private static readonly WeakReference?[] s_trackedContexts = new WeakReference[MaxTrackedContexts]; + private static readonly EqualityComparer s_optionsComparer = new(); 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) { @@ -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) { @@ -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; @@ -252,52 +257,114 @@ private static bool TryGetContext( /// If two instances are equivalent, they should generate identical metadata caches; /// the converse however does not necessarily hold. /// - private static bool AreEquivalentOptions(JsonSerializerOptions left, JsonSerializerOptions right) + private sealed class EqualityComparer : IEqualityComparer { - 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 && - left._typeInfoResolver == right._typeInfoResolver && - CompareLists(left._converters, right._converters); - - static bool CompareLists(ConfigurationList left, ConfigurationList right) + public bool Equals(JsonSerializerOptions? left, JsonSerializerOptions? right) + { + 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 && + left._typeInfoResolver == right._typeInfoResolver && + CompareLists(left._converters, right._converters); + + static bool CompareLists(ConfigurationList left, ConfigurationList right) + where TValue : class? + { + int n; + if ((n = left.Count) != right.Count) + { + return false; + } + + for (int i = 0; i < n; i++) + { + if (left[i] != right[i]) + { + return false; + } + } + + return true; + } + } + + public int GetHashCode(JsonSerializerOptions options) { - int n; - if ((n = left.Count) != right.Count) + HashCode hc = default; + + AddHashCode(ref hc, options._dictionaryKeyPolicy); + AddHashCode(ref hc, options._jsonPropertyNamingPolicy); + AddHashCode(ref hc, options._readCommentHandling); + AddHashCode(ref hc, options._referenceHandler); + AddHashCode(ref hc, options._encoder); + AddHashCode(ref hc, options._defaultIgnoreCondition); + AddHashCode(ref hc, options._numberHandling); + AddHashCode(ref hc, options._unknownTypeHandling); + AddHashCode(ref hc, options._defaultBufferSize); + AddHashCode(ref hc, options._maxDepth); + AddHashCode(ref hc, options._allowTrailingCommas); + AddHashCode(ref hc, options._ignoreNullValues); + AddHashCode(ref hc, options._ignoreReadOnlyProperties); + AddHashCode(ref hc, options._ignoreReadonlyFields); + AddHashCode(ref hc, options._includeFields); + AddHashCode(ref hc, options._propertyNameCaseInsensitive); + AddHashCode(ref hc, options._writeIndented); + AddHashCode(ref hc, options._typeInfoResolver); + AddListHashCode(ref hc, options._converters); + + return hc.ToHashCode(); + + static void AddListHashCode(ref HashCode hc, ConfigurationList list) { - return false; + int n = list.Count; + for (int i = 0; i < n; i++) + { + AddHashCode(ref hc, list[i]); + } } - for (int i = 0; i < n; i++) + static void AddHashCode(ref HashCode hc, TValue? value) { - TValue? leftElem = left[i]; - TValue? rightElem = right[i]; - bool areEqual = leftElem is null ? rightElem is null : leftElem.Equals(rightElem); - if (!areEqual) + if (typeof(TValue).IsValueType) { - return false; + hc.Add(value); + } + else + { + Debug.Assert(!typeof(TValue).IsSealed, "Sealed reference types like string should not use this method."); + hc.Add(RuntimeHelpers.GetHashCode(value)); } } + } - return true; +#if !NETCOREAPP + /// + /// Polyfill for System.HashCode. + /// + private struct HashCode + { + private int _hashCode; + public void Add(T? value) => _hashCode = (_hashCode, value).GetHashCode(); + public int ToHashCode() => _hashCode; } +#endif } } } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CacheTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CacheTests.cs index 772074b7325d5..e53b33c3b4dda 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CacheTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CacheTests.cs @@ -257,6 +257,7 @@ static Func 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: @@ -268,7 +269,7 @@ public static void JsonSerializerOptions_ReuseConverterCaches() RemoteExecutor.Invoke(static () => { Func getCacheOptions = CreateCacheOptionsAccessor(); - Func equalityComparer = CreateEqualityComparerAccessor(); + IEqualityComparer equalityComparer = CreateEqualityComparerAccessor(); foreach (var args in GetJsonSerializerOptions()) { @@ -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++) { @@ -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)); } } @@ -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 equalityComparer = CreateEqualityComparerAccessor(); + IEqualityComparer equalityComparer = CreateEqualityComparerAccessor(); (PropertyInfo prop, object value)[] propertySettersAndValues = GetPropertiesWithSettersAndNonDefaultValues().ToArray(); @@ -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() @@ -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 equalityComparer = CreateEqualityComparerAccessor(); + IEqualityComparer 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 @@ -408,10 +416,11 @@ public MyJsonContext(JsonSerializerOptions options) : base(options) { } protected override JsonSerializerOptions? GeneratedSerializerOptions => Options; } - public static Func CreateEqualityComparerAccessor() + public static IEqualityComparer CreateEqualityComparerAccessor() { - MethodInfo equalityComparerMethod = typeof(JsonSerializerOptions).GetMethod("AreEquivalentOptions", BindingFlags.NonPublic | BindingFlags.Static); - return (Func)Delegate.CreateDelegate(typeof(Func), equalityComparerMethod); + Type equalityComparerType = typeof(JsonSerializerOptions).GetNestedType("EqualityComparer", BindingFlags.NonPublic); + Assert.NotNull(equalityComparerType); + return (IEqualityComparer)Activator.CreateInstance(equalityComparerType, nonPublic: true); } public static IEnumerable WriteSuccessCases