From e584ac7056962bdb13eb4d76494161401a62ca71 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Tue, 4 Oct 2022 13:21:18 +0100 Subject: [PATCH 1/3] Rewrite the shared JsonSerializerOptions cache implementation. --- .../JsonSerializerOptions.Caching.cs | 224 ++++++------------ .../JsonSerializerOptionsUpdateHandler.cs | 3 - .../Serialization/CacheTests.cs | 35 ++- 3 files changed, 92 insertions(+), 170 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 24152a5c716aec..5af18441d8f54c 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 @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Concurrent; -using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; @@ -163,146 +162,124 @@ public void Clear() /// /// Defines a cache of CachingContexts; instead of using a ConditionalWeakTable which can be slow to traverse - /// this approach uses a concurrent dictionary pointing to weak references of . - /// Relevant caching contexts are looked up using the equality comparison defined by . + /// 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 . /// internal static class TrackedCachingContexts { private const int MaxTrackedContexts = 64; - private static readonly ConcurrentDictionary> s_cache = - new(concurrencyLevel: 1, capacity: MaxTrackedContexts, new EqualityComparer()); + private static readonly WeakReference?[] s_trackedContexts = new WeakReference[MaxTrackedContexts]; + private static int s_size; - private const int EvictionCountHistory = 16; - private static readonly Queue s_recentEvictionCounts = new(EvictionCountHistory); - private static int s_evictionRunsToSkip; + private const int DanglingEntryEvictInterval = 8; + private static int s_lookupsWithDanglingEntries; public static CachingContext GetOrCreate(JsonSerializerOptions options) { Debug.Assert(options.IsReadOnly, "Cannot create caching contexts for mutable JsonSerializerOptions instances"); Debug.Assert(options._typeInfoResolver != null); - ConcurrentDictionary> cache = s_cache; + if (TryGetSharedContext(options, out bool foundDanglingEntries, out CachingContext? result)) + { + // Periodically evict dangling entries in the case of successful lookups + if (foundDanglingEntries && Interlocked.Increment(ref s_lookupsWithDanglingEntries) == DanglingEntryEvictInterval) + { + lock (s_trackedContexts) + { + EvictDanglingEntries(); + } + } + + return result; + } - if (cache.TryGetValue(options, out WeakReference? wr) && wr.TryGetTarget(out CachingContext? ctx)) + if (s_size == MaxTrackedContexts && !foundDanglingEntries) { - return ctx; + // Cache is full; return a fresh instance. + return new CachingContext(options); } - lock (cache) + lock (s_trackedContexts) { - if (cache.TryGetValue(options, out wr)) + if (TryGetSharedContext(options, out foundDanglingEntries, out result)) { - if (!wr.TryGetTarget(out ctx)) - { - // Found a dangling weak reference; replenish with a fresh instance. - ctx = new CachingContext(options); - wr.SetTarget(ctx); - } - - return ctx; + return result; } - if (cache.Count == MaxTrackedContexts) + if (foundDanglingEntries) { - if (!TryEvictDanglingEntries()) - { - // Cache is full; return a fresh instance. - return new CachingContext(options); - } + // Always run eviction if writing to the cache. + EvictDanglingEntries(); } - Debug.Assert(cache.Count < MaxTrackedContexts); - - // Use a defensive copy of the options instance as key to - // avoid capturing references to any caching contexts. - var key = new JsonSerializerOptions(options); - Debug.Assert(key._cachingContext == null); - - ctx = new CachingContext(options); - bool success = cache.TryAdd(key, new WeakReference(ctx)); - Debug.Assert(success); + if (s_size == MaxTrackedContexts) + { + // Cache is full; return a fresh instance. + return new CachingContext(options); + } + var ctx = new CachingContext(options); + s_trackedContexts[s_size++] = new WeakReference(ctx); return ctx; } } - public static void Clear() + private static bool TryGetSharedContext( + JsonSerializerOptions options, + out bool foundDanglingEntries, + [NotNullWhen(true)] out CachingContext? result) { - lock (s_cache) - { - s_cache.Clear(); - s_recentEvictionCounts.Clear(); - s_evictionRunsToSkip = 0; - } - } - private static bool TryEvictDanglingEntries() - { - // Worst case scenario, the cache has been filled with permanent entries. - // Evictions are synchronized and each run is in the order of microseconds, - // so we want to avoid triggering runs every time an instance is initialized, - // For this reason we use a backoff strategy to average out the cost of eviction - // across multiple initializations. The backoff count is determined by the eviction - // rates of the most recent runs. - - Debug.Assert(Monitor.IsEntered(s_cache)); - - if (s_evictionRunsToSkip > 0) - { - --s_evictionRunsToSkip; - return false; - } - - int currentEvictions = 0; - foreach (KeyValuePair> kvp in s_cache) - { - if (!kvp.Value.TryGetTarget(out _)) - { - bool result = s_cache.TryRemove(kvp.Key, out _); - Debug.Assert(result); - currentEvictions++; - } - } + WeakReference?[] trackedContexts = s_trackedContexts; + int size = s_size; - s_evictionRunsToSkip = EstimateEvictionRunsToSkip(currentEvictions); - return currentEvictions > 0; + foundDanglingEntries = false; - // Estimate the number of eviction runs to skip based on recent eviction rates. - static int EstimateEvictionRunsToSkip(int latestEvictionCount) + for (int i = 0; i < size; i++) { - Queue recentEvictionCounts = s_recentEvictionCounts; - - if (recentEvictionCounts.Count < EvictionCountHistory - 1) + if (trackedContexts[i] is WeakReference weakRef && + weakRef.TryGetTarget(out CachingContext? ctx)) { - // Insufficient data points to determine a skip count. - recentEvictionCounts.Enqueue(latestEvictionCount); - return 0; + if (EqualityComparer.Equals(options, ctx.Options)) + { + result = ctx; + return true; + } } - else if (recentEvictionCounts.Count == EvictionCountHistory) + else { - recentEvictionCounts.Dequeue(); + foundDanglingEntries = true; } + } - recentEvictionCounts.Enqueue(latestEvictionCount); + result = null; + return false; + } + + private static void EvictDanglingEntries() + { + Monitor.IsEntered(s_trackedContexts); - // Calculate the total number of eviction in the latest runs - // - If we have at least one eviction per run, on average, - // do not skip any future eviction runs. - // - Otherwise, skip ~the number of runs needed per one eviction. + WeakReference?[] trackedOptions = s_trackedContexts; + int size = s_size; - int totalEvictions = 0; - foreach (int evictionCount in recentEvictionCounts) + int nextAvailable = 0; + for (int i = 0; i < size; i++) + { + if (trackedOptions[i] is WeakReference weakRef && + weakRef.TryGetTarget(out _)) { - totalEvictions += evictionCount; + trackedOptions[nextAvailable++] = weakRef; } + } - int evictionRunsToSkip = - totalEvictions >= EvictionCountHistory ? 0 : - (int)Math.Round((double)EvictionCountHistory / Math.Max(totalEvictions, 1)); - - Debug.Assert(0 <= evictionRunsToSkip && evictionRunsToSkip <= EvictionCountHistory); - return evictionRunsToSkip; + for (int i = nextAvailable; i < size; i++) + { + trackedOptions[i] = null; } + + Volatile.Write(ref s_size, nextAvailable); + Volatile.Write(ref s_lookupsWithDanglingEntries, 0); } } @@ -311,9 +288,9 @@ static int EstimateEvictionRunsToSkip(int latestEvictionCount) /// If two instances are equivalent, they should generate identical metadata caches; /// the converse however does not necessarily hold. /// - private sealed class EqualityComparer : IEqualityComparer + private static class EqualityComparer { - public bool Equals(JsonSerializerOptions? left, JsonSerializerOptions? right) + public static bool Equals(JsonSerializerOptions left, JsonSerializerOptions right) { Debug.Assert(left != null && right != null); @@ -357,53 +334,6 @@ static bool CompareLists(ConfigurationList left, ConfigurationLi return true; } } - - public int GetHashCode(JsonSerializerOptions options) - { - 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); - GetHashCode(ref hc, options._converters); - - static void GetHashCode(ref HashCode hc, ConfigurationList list) - { - for (int i = 0; i < list.Count; i++) - { - hc.Add(list[i]); - } - } - - return hc.ToHashCode(); - } - -#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/src/System/Text/Json/Serialization/JsonSerializerOptionsUpdateHandler.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptionsUpdateHandler.cs index 8f7f94140f0c48..53031f1a3c34eb 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptionsUpdateHandler.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptionsUpdateHandler.cs @@ -23,9 +23,6 @@ public static void ClearCache(Type[]? types) options.Key.ClearCaches(); } - // Flush the shared caching contexts - JsonSerializerOptions.TrackedCachingContexts.Clear(); - // Flush the dynamic method cache ReflectionEmitCachingMemberAccessor.Clear(); } 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 e53b33c3b4dda8..a0b5e15713cd7b 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,7 +257,6 @@ 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: @@ -269,7 +268,7 @@ public static void JsonSerializerOptions_ReuseConverterCaches() RemoteExecutor.Invoke(static () => { Func getCacheOptions = CreateCacheOptionsAccessor(); - IEqualityComparer equalityComparer = CreateEqualityComparerAccessor(); + Func equalityComparer = CreateEqualityComparerAccessor(); foreach (var args in GetJsonSerializerOptions()) { @@ -280,8 +279,7 @@ public static void JsonSerializerOptions_ReuseConverterCaches() JsonSerializerOptions originalCacheOptions = getCacheOptions(options); Assert.NotNull(originalCacheOptions); - Assert.True(equalityComparer.Equals(options, originalCacheOptions)); - Assert.Equal(equalityComparer.GetHashCode(options), equalityComparer.GetHashCode(originalCacheOptions)); + Assert.True(equalityComparer(options, originalCacheOptions)); for (int i = 0; i < 5; i++) { @@ -290,8 +288,7 @@ public static void JsonSerializerOptions_ReuseConverterCaches() JsonSerializer.Serialize(42, options2); - Assert.True(equalityComparer.Equals(options2, originalCacheOptions)); - Assert.Equal(equalityComparer.GetHashCode(options2), equalityComparer.GetHashCode(originalCacheOptions)); + Assert.True(equalityComparer(options2, originalCacheOptions)); Assert.Same(originalCacheOptions, getCacheOptions(options2)); } } @@ -327,7 +324,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. - IEqualityComparer equalityComparer = CreateEqualityComparerAccessor(); + Func equalityComparer = CreateEqualityComparerAccessor(); (PropertyInfo prop, object value)[] propertySettersAndValues = GetPropertiesWithSettersAndNonDefaultValues().ToArray(); @@ -337,19 +334,16 @@ public static void JsonSerializerOptions_EqualityComparer_ChangingAnySettingShou Assert.Fail($"{nameof(GetPropertiesWithSettersAndNonDefaultValues)} missing property declaration for {prop.Name}, please update the method."); } - Assert.True(equalityComparer.Equals(JsonSerializerOptions.Default, JsonSerializerOptions.Default)); - Assert.Equal(equalityComparer.GetHashCode(JsonSerializerOptions.Default), equalityComparer.GetHashCode(JsonSerializerOptions.Default)); + Assert.True(equalityComparer(JsonSerializerOptions.Default, JsonSerializerOptions.Default)); foreach ((PropertyInfo prop, object? value) in propertySettersAndValues) { var options = new JsonSerializerOptions(); prop.SetValue(options, value); - Assert.True(equalityComparer.Equals(options, options)); - Assert.Equal(equalityComparer.GetHashCode(options), equalityComparer.GetHashCode(options)); + Assert.True(equalityComparer(options, options)); - Assert.False(equalityComparer.Equals(JsonSerializerOptions.Default, options)); - Assert.NotEqual(equalityComparer.GetHashCode(JsonSerializerOptions.Default), equalityComparer.GetHashCode(options)); + Assert.False(equalityComparer(JsonSerializerOptions.Default, options)); } static IEnumerable<(PropertyInfo, object)> GetPropertiesWithSettersAndNonDefaultValues() @@ -395,16 +389,14 @@ public static void JsonSerializerOptions_EqualityComparer_ApplyingJsonSerializer // // If either of them changes, this test will need to be kept in sync. - IEqualityComparer equalityComparer = CreateEqualityComparerAccessor(); + Func equalityComparer = CreateEqualityComparerAccessor(); var options1 = new JsonSerializerOptions { WriteIndented = true }; var options2 = new JsonSerializerOptions { WriteIndented = true }; - Assert.True(equalityComparer.Equals(options1, options2)); - Assert.Equal(equalityComparer.GetHashCode(options1), equalityComparer.GetHashCode(options2)); + Assert.True(equalityComparer(options1, options2)); _ = new MyJsonContext(options1); // Associate copy with a JsonSerializerContext - Assert.False(equalityComparer.Equals(options1, options2)); - Assert.NotEqual(equalityComparer.GetHashCode(options1), equalityComparer.GetHashCode(options2)); + Assert.False(equalityComparer(options1, options2)); } private class MyJsonContext : JsonSerializerContext @@ -416,11 +408,14 @@ public MyJsonContext(JsonSerializerOptions options) : base(options) { } protected override JsonSerializerOptions? GeneratedSerializerOptions => Options; } - public static IEqualityComparer CreateEqualityComparerAccessor() + public static Func CreateEqualityComparerAccessor() { Type equalityComparerType = typeof(JsonSerializerOptions).GetNestedType("EqualityComparer", BindingFlags.NonPublic); Assert.NotNull(equalityComparerType); - return (IEqualityComparer)Activator.CreateInstance(equalityComparerType, nonPublic: true); + MethodInfo equalityComparerMethod = equalityComparerType.GetMethod("Equals", new[] { typeof(JsonSerializerOptions), typeof(JsonSerializerOptions) }); + Assert.NotNull(equalityComparerMethod); + + return (Func)Delegate.CreateDelegate(typeof(Func), equalityComparerMethod); } public static IEnumerable WriteSuccessCases From 28e917208fd8158230c97ed0e92b253af71f869c Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Tue, 4 Oct 2022 14:31:23 +0100 Subject: [PATCH 2/3] address feedback --- .../Json/Serialization/JsonSerializerOptions.Caching.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 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 5af18441d8f54c..38547640f6bc84 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 @@ -229,12 +229,14 @@ private static bool TryGetSharedContext( out bool foundDanglingEntries, [NotNullWhen(true)] out CachingContext? result) { + // When called outside of the lock this method can be subject to races. + // We're fine with this since worst-case scenario the lookup will report + // a false negative and trigger a further lookup after the lock has been acquired. WeakReference?[] trackedContexts = s_trackedContexts; int size = s_size; foundDanglingEntries = false; - for (int i = 0; i < size; i++) { if (trackedContexts[i] is WeakReference weakRef && @@ -273,10 +275,7 @@ private static void EvictDanglingEntries() } } - for (int i = nextAvailable; i < size; i++) - { - trackedOptions[i] = null; - } + Array.Clear(trackedOptions, nextAvailable, size - nextAvailable); Volatile.Write(ref s_size, nextAvailable); Volatile.Write(ref s_lookupsWithDanglingEntries, 0); From 4a26440ec822f397e76c15494055de5b77412204 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Thu, 6 Oct 2022 00:10:27 +0100 Subject: [PATCH 3/3] Simplify cache implementation& address feedback. --- .../JsonSerializerOptions.Caching.cs | 175 +++++++----------- .../Serialization/CacheTests.cs | 6 +- 2 files changed, 71 insertions(+), 110 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 38547640f6bc84..7fc4c800e72435 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 @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Concurrent; +using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; @@ -163,37 +164,24 @@ 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 int s_size; - - private const int DanglingEntryEvictInterval = 8; - private static int s_lookupsWithDanglingEntries; public static CachingContext GetOrCreate(JsonSerializerOptions options) { Debug.Assert(options.IsReadOnly, "Cannot create caching contexts for mutable JsonSerializerOptions instances"); Debug.Assert(options._typeInfoResolver != null); - if (TryGetSharedContext(options, out bool foundDanglingEntries, out CachingContext? result)) + if (TryGetContext(options, out int firstUnpopulatedIndex, out CachingContext? result)) { - // Periodically evict dangling entries in the case of successful lookups - if (foundDanglingEntries && Interlocked.Increment(ref s_lookupsWithDanglingEntries) == DanglingEntryEvictInterval) - { - lock (s_trackedContexts) - { - EvictDanglingEntries(); - } - } - return result; } - - if (s_size == MaxTrackedContexts && !foundDanglingEntries) + else if (firstUnpopulatedIndex < 0) { // Cache is full; return a fresh instance. return new CachingContext(options); @@ -201,85 +189,62 @@ public static CachingContext GetOrCreate(JsonSerializerOptions options) lock (s_trackedContexts) { - if (TryGetSharedContext(options, out foundDanglingEntries, out result)) + if (TryGetContext(options, out firstUnpopulatedIndex, out result)) { return result; } - if (foundDanglingEntries) - { - // Always run eviction if writing to the cache. - EvictDanglingEntries(); - } + var ctx = new CachingContext(options); - if (s_size == MaxTrackedContexts) + if (firstUnpopulatedIndex >= 0) { - // Cache is full; return a fresh instance. - return new CachingContext(options); + // Cache has capacity -- store the context in the first available index. + ref WeakReference? weakRef = ref s_trackedContexts[firstUnpopulatedIndex]; + + if (weakRef is null) + { + weakRef = new(ctx); + } + else + { + Debug.Assert(weakRef.TryGetTarget(out _) is false); + weakRef.SetTarget(ctx); + } } - var ctx = new CachingContext(options); - s_trackedContexts[s_size++] = new WeakReference(ctx); return ctx; } } - private static bool TryGetSharedContext( + private static bool TryGetContext( JsonSerializerOptions options, - out bool foundDanglingEntries, + out int firstUnpopulatedIndex, [NotNullWhen(true)] out CachingContext? result) { - // When called outside of the lock this method can be subject to races. - // We're fine with this since worst-case scenario the lookup will report - // a false negative and trigger a further lookup after the lock has been acquired. - WeakReference?[] trackedContexts = s_trackedContexts; - int size = s_size; - foundDanglingEntries = false; - for (int i = 0; i < size; i++) + firstUnpopulatedIndex = -1; + for (int i = 0; i < trackedContexts.Length; i++) { - if (trackedContexts[i] is WeakReference weakRef && - weakRef.TryGetTarget(out CachingContext? ctx)) + WeakReference? weakRef = trackedContexts[i]; + + if (weakRef is null || !weakRef.TryGetTarget(out CachingContext? ctx)) { - if (EqualityComparer.Equals(options, ctx.Options)) + if (firstUnpopulatedIndex < 0) { - result = ctx; - return true; + firstUnpopulatedIndex = i; } } - else + else if (AreEquivalentOptions(options, ctx.Options)) { - foundDanglingEntries = true; + result = ctx; + return true; } } result = null; return false; } - - private static void EvictDanglingEntries() - { - Monitor.IsEntered(s_trackedContexts); - - WeakReference?[] trackedOptions = s_trackedContexts; - int size = s_size; - - int nextAvailable = 0; - for (int i = 0; i < size; i++) - { - if (trackedOptions[i] is WeakReference weakRef && - weakRef.TryGetTarget(out _)) - { - trackedOptions[nextAvailable++] = weakRef; - } - } - - Array.Clear(trackedOptions, nextAvailable, size - nextAvailable); - - Volatile.Write(ref s_size, nextAvailable); - Volatile.Write(ref s_lookupsWithDanglingEntries, 0); - } } /// @@ -287,51 +252,51 @@ private static void EvictDanglingEntries() /// If two instances are equivalent, they should generate identical metadata caches; /// the converse however does not necessarily hold. /// - private static class EqualityComparer + private static bool AreEquivalentOptions(JsonSerializerOptions left, JsonSerializerOptions right) { - public static 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) { - 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) + int n; + if ((n = left.Count) != right.Count) { - int n; - if ((n = left.Count) != right.Count) - { - return false; - } + return false; + } - for (int i = 0; i < n; i++) + 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) { - if (!left[i]!.Equals(right[i])) - { - return false; - } + return false; } - - return true; } + + return true; } } } 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 a0b5e15713cd7b..772074b7325d5b 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 @@ -410,11 +410,7 @@ public MyJsonContext(JsonSerializerOptions options) : base(options) { } public static Func CreateEqualityComparerAccessor() { - Type equalityComparerType = typeof(JsonSerializerOptions).GetNestedType("EqualityComparer", BindingFlags.NonPublic); - Assert.NotNull(equalityComparerType); - MethodInfo equalityComparerMethod = equalityComparerType.GetMethod("Equals", new[] { typeof(JsonSerializerOptions), typeof(JsonSerializerOptions) }); - Assert.NotNull(equalityComparerMethod); - + MethodInfo equalityComparerMethod = typeof(JsonSerializerOptions).GetMethod("AreEquivalentOptions", BindingFlags.NonPublic | BindingFlags.Static); return (Func)Delegate.CreateDelegate(typeof(Func), equalityComparerMethod); }