From 76c90692025cadb5cea41fc2aff9a3a3047cb9f3 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Tue, 1 Feb 2022 20:36:44 +0000 Subject: [PATCH 01/10] Use caching in Reflection.Emit member accessors --- .../src/System.Text.Json.csproj | 2 + .../Serialization/JsonSerializerOptions.cs | 4 +- .../JsonSerializerOptionsUpdateHandler.cs | 4 + ...flectionEmitCachingMemberAccessor.Cache.cs | 87 +++++++++++++++++++ .../ReflectionEmitCachingMemberAccessor.cs | 64 ++++++++++++++ 5 files changed, 159 insertions(+), 2 deletions(-) create mode 100644 src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.Cache.cs create mode 100644 src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.cs diff --git a/src/libraries/System.Text.Json/src/System.Text.Json.csproj b/src/libraries/System.Text.Json/src/System.Text.Json.csproj index d24617f74a3d31..0dfd2c6b7f7292 100644 --- a/src/libraries/System.Text.Json/src/System.Text.Json.csproj +++ b/src/libraries/System.Text.Json/src/System.Text.Json.csproj @@ -247,6 +247,8 @@ System.Text.Json.Nodes.JsonValue + + diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs index fb5495a44c31ba..9ecfa5f1880233 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs @@ -568,10 +568,10 @@ internal MemberAccessor MemberAccessorStrategy #if NETCOREAPP // if dynamic code isn't supported, fallback to reflection _memberAccessorStrategy = RuntimeFeature.IsDynamicCodeSupported ? - new ReflectionEmitMemberAccessor() : + new ReflectionEmitCachingMemberAccessor() : new ReflectionMemberAccessor(); #elif NETFRAMEWORK - _memberAccessorStrategy = new ReflectionEmitMemberAccessor(); + _memberAccessorStrategy = new ReflectionEmitCachingMemberAccessor(); #else _memberAccessorStrategy = new ReflectionMemberAccessor(); #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 b79e417ec10d4b..da7637435c54eb 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 @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Reflection.Metadata; using System.Text.Json; +using System.Text.Json.Serialization.Metadata; [assembly: MetadataUpdateHandler(typeof(JsonSerializerOptionsUpdateHandler))] @@ -19,6 +20,9 @@ public static void ClearCache(Type[]? types) { options.Key.ClearClasses(); } + + // Flush the dynamic method cache + ReflectionEmitCachingMemberAccessor.Clear(); } } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.Cache.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.Cache.cs new file mode 100644 index 00000000000000..36de149b0ddebb --- /dev/null +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.Cache.cs @@ -0,0 +1,87 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#if NETFRAMEWORK || NETCOREAPP +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Diagnostics; +using System.Threading; + +namespace System.Text.Json.Serialization.Metadata +{ + internal sealed partial class ReflectionEmitCachingMemberAccessor + { + private sealed class Cache where TKey : notnull + { + private int _lock; + private long _lastEvictedTicks; // tracks total number of invocations to the cache; can be allowed to overflow. + private readonly long _evictionIntervalTicks; // number of cache invocations needed before triggering an eviction run. + private readonly long _slidingExpirationTicks; // max timespan allowed for cache entries to remain inactive. + private readonly ConcurrentDictionary _cache = new(); + + public Cache(TimeSpan slidingExpiration, TimeSpan evictionInterval) + { + _slidingExpirationTicks = slidingExpiration.Ticks; + _evictionIntervalTicks = evictionInterval.Ticks; + _lastEvictedTicks = DateTime.UtcNow.Ticks; + } + + public TValue GetOrAdd(TKey key, Func valueFactory) where TValue : class? + { + CacheEntry entry = _cache.GetOrAdd(key, +#if NETCOREAPP + static (TKey key, Func valueFactory) => new(valueFactory(key)), + valueFactory); +#else + key => new(valueFactory(key))); +#endif + long utcNowTicks = DateTime.UtcNow.Ticks; + Volatile.Write(ref entry.LastUsedTicks, utcNowTicks); + + if (utcNowTicks - Volatile.Read(ref _lastEvictedTicks) > _evictionIntervalTicks) + { + if (Interlocked.CompareExchange(ref _lock, 1, 0) == 0) + { + if (utcNowTicks - _lastEvictedTicks >= _evictionIntervalTicks) + { + EvictStaleCacheEntries(utcNowTicks); + Volatile.Write(ref _lastEvictedTicks, utcNowTicks); + Volatile.Write(ref _lock, 0); + } + } + } + + return (TValue)entry.Value!; + } + + public void Clear() + { + _cache.Clear(); + _lastEvictedTicks = DateTime.UtcNow.Ticks; + } + + private void EvictStaleCacheEntries(long utcNowTicks) + { + foreach (KeyValuePair kvp in _cache) + { + if (utcNowTicks - Volatile.Read(ref kvp.Value.LastUsedTicks) >= _slidingExpirationTicks) + { + _cache.TryRemove(kvp.Key, out _); + } + } + } + + private class CacheEntry + { + public readonly object? Value; + public long LastUsedTicks; + + public CacheEntry(object? value) + { + Value = value; + } + } + } + } +} +#endif diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.cs new file mode 100644 index 00000000000000..d72d98284a0dea --- /dev/null +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.cs @@ -0,0 +1,64 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#if NETFRAMEWORK || NETCOREAPP +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Reflection; + +namespace System.Text.Json.Serialization.Metadata +{ + internal sealed partial class ReflectionEmitCachingMemberAccessor : MemberAccessor + { + private static readonly ReflectionEmitMemberAccessor s_sourceAccessor = new(); + private static readonly Cache<(string id, Type declaringType, MemberInfo? member)> s_cache = + new(slidingExpiration: TimeSpan.FromSeconds(5), evictionInterval: TimeSpan.FromSeconds(1)); + + public static void Clear() => s_cache.Clear(); + + public override Action CreateAddMethodDelegate<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TCollection>() + => s_cache.GetOrAdd((nameof(CreateAddMethodDelegate), typeof(TCollection), null), + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2091:UnrecognizedReflectionPattern", + Justification = "DynamicallyAccessedMembersAttribute is not inherited by scoped generic parameter in lambda body.")] + static (_) => s_sourceAccessor.CreateAddMethodDelegate()); + + public override JsonTypeInfo.ConstructorDelegate? CreateConstructor([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type classType) + => s_cache.GetOrAdd((nameof(CreateConstructor), classType, null), + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2077:UnrecognizedReflectionPattern", + Justification = "Cannot apply DynamicallyAccessedMembersAttribute to tuple properties.")] + static (key) => s_sourceAccessor.CreateConstructor(key.declaringType)); + + public override Func CreateFieldGetter(FieldInfo fieldInfo) + => s_cache.GetOrAdd((nameof(CreateFieldGetter), typeof(TProperty), fieldInfo), static key => s_sourceAccessor.CreateFieldGetter((FieldInfo)key.member!)); + + public override Action CreateFieldSetter(FieldInfo fieldInfo) + => s_cache.GetOrAdd((nameof(CreateFieldSetter), typeof(TProperty), fieldInfo), static key => s_sourceAccessor.CreateFieldSetter((FieldInfo)key.member!)); + + [RequiresUnreferencedCode(IEnumerableConverterFactoryHelpers.ImmutableConvertersUnreferencedCodeMessage)] + public override Func>, TCollection> CreateImmutableDictionaryCreateRangeDelegate() + => s_cache.GetOrAdd((nameof(CreateImmutableDictionaryCreateRangeDelegate), typeof((TCollection, TKey, TValue)), null), + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", + Justification = "Factory method calls into chained RequiresUnreferencedCode method.")] + static (_) => s_sourceAccessor.CreateImmutableDictionaryCreateRangeDelegate()); + + [RequiresUnreferencedCode(IEnumerableConverterFactoryHelpers.ImmutableConvertersUnreferencedCodeMessage)] + public override Func, TCollection> CreateImmutableEnumerableCreateRangeDelegate() + => s_cache.GetOrAdd((nameof(CreateImmutableEnumerableCreateRangeDelegate), typeof((TCollection, TElement)), null), + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", + Justification = "Factory method calls into chained RequiresUnreferencedCode method.")] + static (_) => s_sourceAccessor.CreateImmutableEnumerableCreateRangeDelegate()); + + public override Func? CreateParameterizedConstructor(ConstructorInfo constructor) + => s_cache.GetOrAdd((nameof(CreateParameterizedConstructor), typeof(T), constructor), static key => s_sourceAccessor.CreateParameterizedConstructor((ConstructorInfo)key.member!)); + + public override JsonTypeInfo.ParameterizedConstructorDelegate? CreateParameterizedConstructor(ConstructorInfo constructor) + => s_cache.GetOrAdd((nameof(CreateParameterizedConstructor), typeof(T), constructor), static key => s_sourceAccessor.CreateParameterizedConstructor((ConstructorInfo)key.member!)); + + public override Func CreatePropertyGetter(PropertyInfo propertyInfo) + => s_cache.GetOrAdd((nameof(CreatePropertyGetter), typeof(TProperty), propertyInfo), static key => s_sourceAccessor.CreatePropertyGetter((PropertyInfo)key.member!)); + + public override Action CreatePropertySetter(PropertyInfo propertyInfo) + => s_cache.GetOrAdd((nameof(CreatePropertySetter), typeof(TProperty), propertyInfo), static key => s_sourceAccessor.CreatePropertySetter((PropertyInfo)key.member!)); + } +} +#endif From d1ffeafc7ae7f1579d1123a1488cf747db9b3099 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Fri, 4 Feb 2022 19:06:40 +0000 Subject: [PATCH 02/10] Refactor JsonSerializerOptions caching & use shared caching contexts --- .../src/System.Text.Json.csproj | 1 + .../Serialization/JsonSerializer.Helpers.cs | 6 +- .../JsonSerializer.Read.Stream.cs | 4 +- .../JsonSerializerOptions.Caching.cs | 321 ++++++++++++++++++ .../JsonSerializerOptions.Converters.cs | 34 +- .../Serialization/JsonSerializerOptions.cs | 87 +---- .../JsonSerializerOptionsUpdateHandler.cs | 2 +- .../Metadata/JsonParameterInfo.cs | 4 +- .../Metadata/JsonPropertyInfo.cs | 2 +- .../Serialization/Metadata/JsonTypeInfo.cs | 8 +- .../Text/Json/Serialization/ReadStack.cs | 2 +- .../Text/Json/Serialization/WriteStack.cs | 2 +- .../Json/Serialization/WriteStackFrame.cs | 2 +- .../JsonSerializerContextTests.cs | 4 - .../Serialization/CacheTests.cs | 225 ++++++++++-- .../System.Text.Json.Tests.csproj | 1 + 16 files changed, 574 insertions(+), 131 deletions(-) create mode 100644 src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs diff --git a/src/libraries/System.Text.Json/src/System.Text.Json.csproj b/src/libraries/System.Text.Json/src/System.Text.Json.csproj index 0dfd2c6b7f7292..be653156bb277e 100644 --- a/src/libraries/System.Text.Json/src/System.Text.Json.csproj +++ b/src/libraries/System.Text.Json/src/System.Text.Json.csproj @@ -121,6 +121,7 @@ System.Text.Json.Nodes.JsonValue + diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs index 40649ec846660a..fa39f8daaf871b 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Helpers.cs @@ -18,12 +18,12 @@ private static JsonTypeInfo GetTypeInfo(JsonSerializerOptions? options, Type run Debug.Assert(runtimeType != null); options ??= JsonSerializerOptions.Default; - if (!options.IsInitializedForReflectionSerializer) + if (!JsonSerializerOptions.IsInitializedForReflectionSerializer) { - options.InitializeForReflectionSerializer(); + JsonSerializerOptions.InitializeForReflectionSerializer(); } - return options.GetOrAddClassForRootType(runtimeType); + return options.GetOrAddJsonTypeInfoForRootType(runtimeType); } private static JsonTypeInfo GetTypeInfo(JsonSerializerContext context, Type type) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Stream.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Stream.cs index 46963245c89af2..c13ce252fee16a 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Stream.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Stream.cs @@ -287,9 +287,9 @@ public static partial class JsonSerializer CancellationToken cancellationToken = default) { options ??= JsonSerializerOptions.Default; - if (!options.IsInitializedForReflectionSerializer) + if (!JsonSerializerOptions.IsInitializedForReflectionSerializer) { - options.InitializeForReflectionSerializer(); + JsonSerializerOptions.InitializeForReflectionSerializer(); } return CreateAsyncEnumerableDeserializer(utf8Json, options, cancellationToken); 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 new file mode 100644 index 00000000000000..8756f0c68ea53b --- /dev/null +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs @@ -0,0 +1,321 @@ +// Licensed to the .NET Foundation under one or more agreements. +// 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; +using System.Text.Json.Serialization; +using System.Text.Json.Serialization.Metadata; + +namespace System.Text.Json +{ + public sealed partial class JsonSerializerOptions + { + /// + /// Encapsulates all cached metadata referenced by the current instance. + /// Context can be shared across multiple equivalent options instances. + /// + private CachingContext? _cachingContext; + + // Simple LRU cache for the public (de)serialize entry points that avoid some lookups in _cachingContext. + // Although this may be written by multiple threads, 'volatile' was not added since any local affinity is fine. + private JsonTypeInfo? _lastTypeInfo; + + internal JsonTypeInfo GetOrAddJsonTypeInfo(Type type) + { + if (_cachingContext == null) + { + InitializeCachingContext(); + Debug.Assert(_cachingContext != null); + } + + return _cachingContext.GetOrAddJsonTypeInfo(type); + } + + internal bool TryGetJsonTypeInfo(Type type, [NotNullWhen(true)] out JsonTypeInfo? typeInfo) + { + if (_cachingContext == null) + { + typeInfo = null; + return false; + } + + return _cachingContext.TryGetJsonTypeInfo(type, out typeInfo); + } + + internal bool IsJsonTypeInfoCached(Type type) => _cachingContext?.IsJsonTypeInfoCached(type) == true; + + /// + /// Return the TypeInfo for root API calls. + /// This has an LRU cache that is intended only for public API calls that specify the root type. + /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal JsonTypeInfo GetOrAddJsonTypeInfoForRootType(Type type) + { + JsonTypeInfo? jsonTypeInfo = _lastTypeInfo; + + if (jsonTypeInfo?.Type != type) + { + jsonTypeInfo = GetOrAddJsonTypeInfo(type); + _lastTypeInfo = jsonTypeInfo; + } + + return jsonTypeInfo; + } + + internal void ClearCaches() + { + _cachingContext?.Clear(); + _lastTypeInfo = null; + } + + private void InitializeCachingContext() + { +#if NETCOREAPP + _cachingContext = TrackedCachingContexts.GetOrCreate(this); +#else + _cachingContext = new CachingContext(this); +#endif + } + + /// + /// Stores and manages all reflection caches for one or more instances. + /// NB the type encapsulates the original options instance and only consults that one when building new types; + /// this is to prevent multiple options instances from leaking into the object graphs of converters which + /// could break user invariants. + /// + private sealed class CachingContext + { + private readonly ConcurrentDictionary _converterCache = new(); + private readonly ConcurrentDictionary _jsonTypeInfoCache = new(); + + public CachingContext(JsonSerializerOptions options) + { + Options = options; + _ = Count; + } + + public JsonSerializerOptions Options { get; } + public int Count => _converterCache.Count + _jsonTypeInfoCache.Count; + public JsonConverter GetOrAddConverter(Type type) => _converterCache.GetOrAdd(type, Options.GetConverterFromType); + public JsonTypeInfo GetOrAddJsonTypeInfo(Type type) => _jsonTypeInfoCache.GetOrAdd(type, Options.GetJsonTypeInfoFromContextOrCreate); + public bool TryGetJsonTypeInfo(Type type, [NotNullWhen(true)] out JsonTypeInfo? typeInfo) => _jsonTypeInfoCache.TryGetValue(type, out typeInfo); + public bool IsJsonTypeInfoCached(Type type) => _jsonTypeInfoCache.ContainsKey(type); + + public void Clear() + { + _converterCache.Clear(); + _jsonTypeInfoCache.Clear(); + } + } + +#if NETCOREAPP + /// + /// Defines a cache of CachingContexts; instead of using a ConditionalWeakTable which can be slow to traverse + /// this approach uses a regular dictionary pointing to weak references of . + /// Relevant caching contexts are looked up using the equality comparison defined by . + /// + private static class TrackedCachingContexts + { + private const int MaxTrackedContexts = 64; + private static readonly ConcurrentDictionary> s_cache = + new(concurrencyLevel: 1, capacity: MaxTrackedContexts, new EqualityComparer()); + + private const int EvictionCountHistory = 10; + private static Queue s_recentEvictionCounts = new(EvictionCountHistory); + private static int s_evictionRunsToSkip; + + public static CachingContext GetOrCreate(JsonSerializerOptions options) + { + if (s_cache.TryGetValue(options, out WeakReference? wr) && wr.TryGetTarget(out CachingContext? ctx)) + { + return ctx; + } + + lock (s_cache) + { + if (s_cache.TryGetValue(options, out wr)) + { + if (!wr.TryGetTarget(out ctx)) + { + // Found a dangling weak reference; replenish with a fresh instance. + ctx = new CachingContext(options); + wr.SetTarget(ctx); + } + + return ctx; + } + + if (s_cache.Count == MaxTrackedContexts) + { + if (!TryEvictDanglingEntries()) + { + // Cache is full; return a fresh instance. + return new CachingContext(options); + } + } + + Debug.Assert(s_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) { _context = options._context }; + Debug.Assert(key._cachingContext == null); + + ctx = new CachingContext(options); + bool success = s_cache.TryAdd(key, new(ctx)); + Debug.Assert(success); + + return ctx; + } + } + + private static bool TryEvictDanglingEntries() + { + // Worst case scenario, the cache has been saturated with permanent entries. + // We want to avoid iterating needlessly through the entire cache every time + // a new options instance is created. For this reason we implement a backoff + // strategy to amortize the cost of eviction across multiple cache initializations. + // The backoff count is determined by the eviction rates of the most recent eviction runs. + + 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++; + } + } + + s_evictionRunsToSkip = EstimateEvictionRunsToSkip(currentEvictions); + return currentEvictions > 0; + + // Estimate the number of eviction runs to skip based on recent eviction rates. + static int EstimateEvictionRunsToSkip(int latestEvictionCount) + { + if (s_recentEvictionCounts.Count < EvictionCountHistory) + { + // Insufficient data to determine a skip count. + s_recentEvictionCounts.Enqueue(latestEvictionCount); + return 0; + } + else + { + s_recentEvictionCounts.Dequeue(); + s_recentEvictionCounts.Enqueue(latestEvictionCount); + + // Calculate the average evictions per run. + double avgEvictionsPerRun = 0; + + foreach (int rate in s_recentEvictionCounts) + { + avgEvictionsPerRun += rate; + } + + avgEvictionsPerRun /= EvictionCountHistory; + + // - If avgEvictionsPerRun >= 1 we skip no subsequent eviction calls. + // - If avgEvictionsPerRun < 1 we skip ~ `1 / avgEvictionsPerRun` calls. + int runsPerEviction = (int)Math.Round(1 / Math.Clamp(avgEvictionsPerRun, 0.1, 1)); + Debug.Assert(runsPerEviction >= 1 && runsPerEviction <= 10); + return runsPerEviction - 1; + } + } + } + } + + /// + /// Provides a conservative equality comparison for JsonSerializerOptions instances. + /// If two instances are equivalent, they should generate identical metadata caches; + /// the converse however does not necessarily hold. + /// + private class EqualityComparer : IEqualityComparer + { + 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._context == right._context && + CompareConverters(left.Converters, right.Converters); + + static bool CompareConverters(IList left, IList right) + { + 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) + { + 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._context); + + foreach (JsonConverter converter in options.Converters) + { + hc.Add(converter); + } + + return hc.ToHashCode(); + } + } +#endif + } +} diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs index 4276d374058934..f8787a2eba8fdd 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // 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; @@ -24,11 +23,8 @@ public sealed partial class JsonSerializerOptions // The global list of built-in converters that override CanConvert(). private static JsonConverter[]? s_defaultFactoryConverters; - // The cached converters (custom or built-in). - private readonly ConcurrentDictionary _converters = new ConcurrentDictionary(); - [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)] - private void RootBuiltInConverters() + private static void RootBuiltInConverters() { s_defaultSimpleConverters = GetDefaultSimpleConverters(); s_defaultFactoryConverters = new JsonConverter[] @@ -97,7 +93,7 @@ void Add(JsonConverter converter) => /// public IList Converters { get; } - internal JsonConverter DetermineConverter(Type? parentClassType, Type runtimePropertyType, MemberInfo? memberInfo) + internal JsonConverter GetConverterForMember(Type? parentClassType, Type runtimePropertyType, MemberInfo? memberInfo) { JsonConverter converter = null!; @@ -174,16 +170,21 @@ public JsonConverter GetConverter(Type typeToConvert) internal JsonConverter GetConverterInternal(Type typeToConvert) { - Debug.Assert(typeToConvert != null); - - if (_converters.TryGetValue(typeToConvert, out JsonConverter? converter)) + // Only cache the value once (de)serialization has occurred since new converters can be added that may change the result. + if (_cachingContext != null) { - Debug.Assert(converter != null); - return converter; + return _cachingContext.GetOrAddConverter(typeToConvert); } + return GetConverterFromType(typeToConvert); + } + + private JsonConverter GetConverterFromType(Type typeToConvert) + { + Debug.Assert(typeToConvert != null); + // Priority 1: If there is a JsonSerializerContext, fetch the converter from there. - converter = _context?.GetTypeInfo(typeToConvert)?.PropertyInfoForTypeInfo?.ConverterBase; + JsonConverter? converter = _context?.GetTypeInfo(typeToConvert)?.PropertyInfoForTypeInfo?.ConverterBase; // Priority 2: Attempt to get custom converter added at runtime. // Currently there is not a way at runtime to override the [JsonConverter] when applied to a property. @@ -259,15 +260,6 @@ internal JsonConverter GetConverterInternal(Type typeToConvert) ThrowHelper.ThrowInvalidOperationException_SerializationConverterNotCompatible(converter.GetType(), typeToConvert); } - // Only cache the value once (de)serialization has occurred since new converters can be added that may change the result. - if (_haveTypesBeenCreated) - { - // A null converter is allowed here and cached. - - // Ignore failure case here in multi-threaded cases since the cached item will be equivalent. - _converters.TryAdd(typeToConvert, converter); - } - return converter; } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs index 9ecfa5f1880233..46e1725fd89de9 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Collections.Concurrent; using System.ComponentModel; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; @@ -32,17 +31,12 @@ public sealed partial class JsonSerializerOptions /// so using fresh default instances every time one is needed can result in redundant recomputation of converters. /// This property provides a shared instance that can be consumed by any number of components without necessitating any converter recomputation. /// - public static JsonSerializerOptions Default { get; } = new JsonSerializerOptions { _haveTypesBeenCreated = true }; - - private readonly ConcurrentDictionary _classes = new ConcurrentDictionary(); - - // Simple LRU cache for the public (de)serialize entry points that avoid some lookups in _classes. - // Although this may be written by multiple threads, 'volatile' was not added since any local affinity is fine. - private JsonTypeInfo? _lastClass { get; set; } + public static JsonSerializerOptions Default { get; } = CreateDefaultImmutableInstance(); internal JsonSerializerContext? _context; - private Func? _typeInfoCreationFunc; + // Stores the JsonTypeInfo factory, which requires unreferenced code and must be rooted by the reflection-based serializer. + private static Func? s_typeInfoCreationFunc; // For any new option added, adding it to the options copied in the copy constructor below must be considered. @@ -59,7 +53,6 @@ public sealed partial class JsonSerializerOptions private int _defaultBufferSize = BufferSizeDefault; private int _maxDepth; private bool _allowTrailingCommas; - private bool _haveTypesBeenCreated; private bool _ignoreNullValues; private bool _ignoreReadOnlyProperties; private bool _ignoreReadonlyFields; @@ -584,37 +577,26 @@ internal MemberAccessor MemberAccessorStrategy /// /// Whether needs to be called. /// - internal bool IsInitializedForReflectionSerializer { get; set; } + internal static bool IsInitializedForReflectionSerializer { get; set; } /// /// Initializes the converters for the reflection-based serializer. /// must be checked before calling. /// [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)] - internal void InitializeForReflectionSerializer() + internal static void InitializeForReflectionSerializer() { // For threading cases, the state that is set here can be overwritten. RootBuiltInConverters(); - _typeInfoCreationFunc = CreateJsonTypeInfo; + s_typeInfoCreationFunc = CreateJsonTypeInfo; IsInitializedForReflectionSerializer = true; [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)] static JsonTypeInfo CreateJsonTypeInfo(Type type, JsonSerializerOptions options) => new JsonTypeInfo(type, options); } - internal JsonTypeInfo GetOrAddClass(Type type) - { - _haveTypesBeenCreated = true; - - if (!TryGetClass(type, out JsonTypeInfo? result)) - { - result = _classes.GetOrAdd(type, GetClassFromContextOrCreate(type)); - } - - return result; - } - internal JsonTypeInfo GetClassFromContextOrCreate(Type type) + private JsonTypeInfo GetJsonTypeInfoFromContextOrCreate(Type type) { JsonTypeInfo? info = _context?.GetTypeInfo(type); if (info != null) @@ -622,55 +604,13 @@ internal JsonTypeInfo GetClassFromContextOrCreate(Type type) return info; } - if (_typeInfoCreationFunc == null) + if (s_typeInfoCreationFunc == null) { ThrowHelper.ThrowNotSupportedException_NoMetadataForType(type); return null!; } - return _typeInfoCreationFunc(type, this); - } - - /// - /// Return the TypeInfo for root API calls. - /// This has a LRU cache that is intended only for public API calls that specify the root type. - /// - [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal JsonTypeInfo GetOrAddClassForRootType(Type type) - { - JsonTypeInfo? jsonTypeInfo = _lastClass; - if (jsonTypeInfo?.Type != type) - { - jsonTypeInfo = GetOrAddClass(type); - _lastClass = jsonTypeInfo; - } - - return jsonTypeInfo; - } - - internal bool TryGetClass(Type type, [NotNullWhen(true)] out JsonTypeInfo? jsonTypeInfo) - { - // todo: for performance and reduced instances, consider using the converters and JsonTypeInfo from s_defaultOptions by cloning (or reference directly if no changes). - // https://github.com/dotnet/runtime/issues/32357 - if (!_classes.TryGetValue(type, out JsonTypeInfo? result)) - { - jsonTypeInfo = null; - return false; - } - - jsonTypeInfo = result; - return true; - } - - internal bool TypeIsCached(Type type) - { - return _classes.ContainsKey(type); - } - - internal void ClearClasses() - { - _classes.Clear(); - _lastClass = null; + return s_typeInfoCreationFunc(type, this); } internal JsonDocumentOptions GetDocumentOptions() @@ -716,10 +656,17 @@ internal JsonWriterOptions GetWriterOptions() internal void VerifyMutable() { - if (_haveTypesBeenCreated || _context != null) + if (_cachingContext != null || _context != null) { ThrowHelper.ThrowInvalidOperationException_SerializerOptionsImmutable(_context); } } + + private static JsonSerializerOptions CreateDefaultImmutableInstance() + { + var options = new JsonSerializerOptions(); + options.InitializeCachingContext(); // eagerly initialize caching context to close type for modification. + return options; + } } } 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 da7637435c54eb..ee80e36205f908 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 @@ -18,7 +18,7 @@ public static void ClearCache(Type[]? types) // Ignore the types, and just clear out all reflection caches from serializer options. foreach (KeyValuePair options in JsonSerializerOptions.TrackedOptionsInstances.All) { - options.Key.ClearClasses(); + options.Key.ClearCaches(); } // Flush the dynamic method cache diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonParameterInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonParameterInfo.cs index 9139a027c5ea4f..aea214b684969b 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonParameterInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonParameterInfo.cs @@ -41,7 +41,7 @@ public JsonTypeInfo RuntimeTypeInfo if (_runtimeTypeInfo == null) { Debug.Assert(Options != null); - _runtimeTypeInfo = Options!.GetOrAddClass(RuntimePropertyType); + _runtimeTypeInfo = Options!.GetOrAddJsonTypeInfo(RuntimePropertyType); } return _runtimeTypeInfo; @@ -102,7 +102,7 @@ public static JsonParameterInfo CreateIgnoredParameterPlaceholder( Type parameterType = parameterInfo.ParameterType; DefaultValueHolder holder; - if (matchingProperty.Options.TryGetClass(parameterType, out JsonTypeInfo? typeInfo)) + if (matchingProperty.Options.TryGetJsonTypeInfo(parameterType, out JsonTypeInfo? typeInfo)) { holder = typeInfo.DefaultValueHolder; } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs index 5e2258ec6dde83..03d0e4fe6b84e5 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs @@ -473,7 +473,7 @@ internal JsonTypeInfo RuntimeTypeInfo { if (_runtimeTypeInfo == null) { - _runtimeTypeInfo = Options.GetOrAddClass(RuntimePropertyType!); + _runtimeTypeInfo = Options.GetOrAddJsonTypeInfo(RuntimePropertyType!); } return _runtimeTypeInfo; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs index 547173ba0f39e3..64a88d4249402d 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs @@ -52,7 +52,7 @@ internal JsonTypeInfo? ElementTypeInfo { if (_elementTypeInfo == null && ElementType != null) { - _elementTypeInfo = Options.GetOrAddClass(ElementType); + _elementTypeInfo = Options.GetOrAddJsonTypeInfo(ElementType); } return _elementTypeInfo; @@ -85,7 +85,7 @@ internal JsonTypeInfo? KeyTypeInfo { Debug.Assert(PropertyInfoForTypeInfo.ConverterStrategy == ConverterStrategy.Dictionary); - _keyTypeInfo = Options.GetOrAddClass(KeyType); + _keyTypeInfo = Options.GetOrAddJsonTypeInfo(KeyType); } return _keyTypeInfo; @@ -600,7 +600,7 @@ private static JsonConverter GetConverter( Debug.Assert(type != null); ValidateType(type, parentClassType, memberInfo, options); - JsonConverter converter = options.DetermineConverter(parentClassType, type, memberInfo); + JsonConverter converter = options.GetConverterForMember(parentClassType, type, memberInfo); // The runtimeType is the actual value being assigned to the property. // There are three types to consider for the runtimeType: @@ -649,7 +649,7 @@ private static JsonConverter GetConverter( private static void ValidateType(Type type, Type? parentClassType, MemberInfo? memberInfo, JsonSerializerOptions options) { - if (!options.TypeIsCached(type) && IsInvalidForSerialization(type)) + if (!options.IsJsonTypeInfoCached(type) && IsInvalidForSerialization(type)) { ThrowHelper.ThrowInvalidOperationException_CannotSerializeInvalidType(type, parentClassType, memberInfo); } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs index 06c09b9cf07e91..9e011a5f8ed234 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs @@ -83,7 +83,7 @@ private void EnsurePushCapacity() public void Initialize(Type type, JsonSerializerOptions options, bool supportContinuation) { - JsonTypeInfo jsonTypeInfo = options.GetOrAddClassForRootType(type); + JsonTypeInfo jsonTypeInfo = options.GetOrAddJsonTypeInfoForRootType(type); Initialize(jsonTypeInfo, supportContinuation); } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStack.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStack.cs index e5042d9894769c..c5062bef1235e4 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStack.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStack.cs @@ -98,7 +98,7 @@ private void EnsurePushCapacity() /// public JsonConverter Initialize(Type type, JsonSerializerOptions options, bool supportContinuation) { - JsonTypeInfo jsonTypeInfo = options.GetOrAddClassForRootType(type); + JsonTypeInfo jsonTypeInfo = options.GetOrAddJsonTypeInfoForRootType(type); Debug.Assert(options == jsonTypeInfo.Options); return Initialize(jsonTypeInfo, supportContinuation); } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs index fdd4972f9f1d99..4379faf8723300 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs @@ -114,7 +114,7 @@ public JsonConverter InitializeReEntry(Type type, JsonSerializerOptions options) // if the current element is the same type as the previous element. if (PolymorphicJsonPropertyInfo?.RuntimePropertyType != type) { - JsonTypeInfo typeInfo = options.GetOrAddClass(type); + JsonTypeInfo typeInfo = options.GetOrAddJsonTypeInfo(type); PolymorphicJsonPropertyInfo = typeInfo.PropertyInfoForTypeInfo; } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs index a9fa26e3279f11..741c73e9bb0bc6 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs @@ -38,7 +38,6 @@ public static void Converters_AndTypeInfoCreator_NotRooted_WhenMetadataNotPresen // This test uses reflection to: // - Access JsonSerializerOptions.s_defaultSimpleConverters // - Access JsonSerializerOptions.s_defaultFactoryConverters - // - Access JsonSerializerOptions._typeInfoCreationFunc // // If any of them changes, this test will need to be kept in sync. @@ -46,9 +45,6 @@ public static void Converters_AndTypeInfoCreator_NotRooted_WhenMetadataNotPresen AssertFieldNull("s_defaultSimpleConverters", optionsInstance: null); AssertFieldNull("s_defaultFactoryConverters", optionsInstance: null); - // Confirm type info dynamic creator not set. - AssertFieldNull("_typeInfoCreationFunc", MetadataContext.Default.Options); - static void AssertFieldNull(string fieldName, JsonSerializerOptions? optionsInstance) { BindingFlags bindingFlags = BindingFlags.NonPublic | (optionsInstance == null ? BindingFlags.Static : BindingFlags.Instance); 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 e233a3440a4b3e..8b4bd83ff46804 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 @@ -1,12 +1,15 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Collections; using System.Collections.Generic; using System.IO; +using System.Linq; using System.Reflection; +using System.Text.Encodings.Web; +using System.Text.Json.Serialization.Metadata; using System.Threading.Tasks; using Xunit; +using Microsoft.DotNet.RemoteExecutor; namespace System.Text.Json.Serialization.Tests { @@ -205,36 +208,218 @@ void Deserialize() await Task.WhenAll(tasks); } - [Fact] [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] - public static async Task JsonSerializerOptionsUpdateHandler_ClearingDoesntPreventSerialization() + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public static void JsonSerializerOptionsUpdateHandler_ClearingDoesntPreventSerialization() { // This test uses reflection to: - // - Access JsonSerializerOptions._classes + // - Access JsonSerializerOptions._cachingContext.Count // - Access JsonSerializerOptionsUpdateHandler.ClearCache // // If either of them changes, this test will need to be kept in sync. - var options = new JsonSerializerOptions(); + RemoteExecutor.Invoke(() => + { + var options = new JsonSerializerOptions(); - FieldInfo classesField = options.GetType().GetField("_classes", BindingFlags.NonPublic | BindingFlags.Instance); - Assert.NotNull(classesField); - IDictionary classes = (IDictionary)classesField.GetValue(options); - Assert.Equal(0, classes.Count); + Func getCount = CreateCacheCountAccessor(); - SimpleTestClass testObj = new SimpleTestClass(); - testObj.Initialize(); - await JsonSerializer.SerializeAsync(new MemoryStream(), testObj, options); - Assert.NotEqual(0, classes.Count); + Assert.Equal(0, getCount(options)); + + SimpleTestClass testObj = new SimpleTestClass(); + testObj.Initialize(); + JsonSerializer.Serialize(testObj, options); + Assert.NotEqual(0, getCount(options)); + + Type updateHandler = typeof(JsonSerializerOptions).Assembly.GetType("System.Text.Json.JsonSerializerOptionsUpdateHandler", throwOnError: true, ignoreCase: false); + MethodInfo clearCache = updateHandler.GetMethod("ClearCache"); + Assert.NotNull(clearCache); + clearCache.Invoke(null, new object[] { null }); + Assert.Equal(0, getCount(options)); + + JsonSerializer.Serialize(testObj, options); + Assert.NotEqual(0, getCount(options)); + }).Dispose(); + + static Func CreateCacheCountAccessor() + { + FieldInfo cacheField = typeof(JsonSerializerOptions).GetField("_cachingContext", BindingFlags.NonPublic | BindingFlags.Instance); + Assert.NotNull(cacheField); + PropertyInfo countProperty = cacheField.FieldType.GetProperty("Count", BindingFlags.Public | BindingFlags.Instance); + Assert.NotNull(countProperty); + return options => + { + object? cache = cacheField.GetValue(options); + return cache is null ? 0 : (int)countProperty.GetValue(cache); + }; + } + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [MemberData(nameof(GetJsonSerializerOptions))] + [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] + public static void JsonSerializerOptions_ReuseConverterCaches() + { + // This test uses reflection to: + // - Access JsonSerializerOptions._cachingContext._options + // - Access JsonSerializerOptions.EqualityComparer.AreEquivalent + // + // If either of them changes, this test will need to be kept in sync. + + RemoteExecutor.Invoke(static () => + { + Func getCacheOptions = CreateCacheOptionsAccessor(); + IEqualityComparer equalityComparer = CreateEqualityComparerAccessor(); + + foreach (var args in GetJsonSerializerOptions()) + { + var options = (JsonSerializerOptions)args[0]; + Assert.Null(getCacheOptions(options)); + + JsonSerializer.Serialize(42, options); + + JsonSerializerOptions originalCacheOptions = getCacheOptions(options); + Assert.NotNull(originalCacheOptions); + Assert.True(equalityComparer.Equals(options, originalCacheOptions)); + Assert.Equal(equalityComparer.GetHashCode(options), equalityComparer.GetHashCode(originalCacheOptions)); + + for (int i = 0; i < 5; i++) + { + var options2 = new JsonSerializerOptions(options); + Assert.True(equalityComparer.Equals(options2, originalCacheOptions)); + Assert.Equal(equalityComparer.GetHashCode(options2), equalityComparer.GetHashCode(originalCacheOptions)); + Assert.Null(getCacheOptions(options2)); + JsonSerializer.Serialize(42, options2); + Assert.Same(originalCacheOptions, getCacheOptions(options2)); + } + } + }).Dispose(); + + static Func CreateCacheOptionsAccessor() + { + FieldInfo cacheField = typeof(JsonSerializerOptions).GetField("_cachingContext", BindingFlags.NonPublic | BindingFlags.Instance); + Assert.NotNull(cacheField); + PropertyInfo optionsField = cacheField.FieldType.GetProperty("Options", BindingFlags.Public | BindingFlags.Instance); + Assert.NotNull(optionsField); + return options => + { + object? cache = cacheField.GetValue(options); + return cache is null ? null : (JsonSerializerOptions)optionsField.GetValue(cache); + }; + } + } + + public static IEnumerable GetJsonSerializerOptions() + { + yield return new[] { new JsonSerializerOptions() }; + yield return new[] { new JsonSerializerOptions(JsonSerializerDefaults.Web) }; + yield return new[] { new JsonSerializerOptions { WriteIndented = true } }; + yield return new[] { new JsonSerializerOptions { Converters = { new JsonStringEnumConverter() } } }; + } + + [Fact] + [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] + public static void JsonSerializerOptions_EqualityComparer_ChangingAnySettingShouldReturnFalse() + { + // This test uses reflection to: + // - Access JsonSerializerOptions.EqualityComparer.AreEquivalent + // - All public setters in JsonSerializerOptions + // + // If either of them changes, this test will need to be kept in sync. + IEqualityComparer equalityComparer = CreateEqualityComparerAccessor(); - Type updateHandler = typeof(JsonSerializerOptions).Assembly.GetType("System.Text.Json.JsonSerializerOptionsUpdateHandler", throwOnError: true, ignoreCase: false); - MethodInfo clearCache = updateHandler.GetMethod("ClearCache"); - Assert.NotNull(clearCache); - clearCache.Invoke(null, new object[] { null }); - Assert.Equal(0, classes.Count); + (PropertyInfo prop, object value)[] propertySettersAndValues = GetPropertiesWithSettersAndNonDefaultValues().ToArray(); - await JsonSerializer.SerializeAsync(new MemoryStream(), testObj, options); - Assert.NotEqual(0, classes.Count); + // Ensure we're testing equality for all JsonSerializerOptions settings + foreach (PropertyInfo prop in GetAllPublicPropertiesWithSetters().Except(propertySettersAndValues.Select(x => x.prop))) + { + 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)); + + 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.False(equalityComparer.Equals(JsonSerializerOptions.Default, options)); + Assert.NotEqual(equalityComparer.GetHashCode(JsonSerializerOptions.Default), equalityComparer.GetHashCode(options)); + } + + static IEnumerable<(PropertyInfo, object)> GetPropertiesWithSettersAndNonDefaultValues() + { + yield return (GetProp(nameof(JsonSerializerOptions.AllowTrailingCommas)), true); + yield return (GetProp(nameof(JsonSerializerOptions.DefaultBufferSize)), 42); + yield return (GetProp(nameof(JsonSerializerOptions.Encoder)), JavaScriptEncoder.UnsafeRelaxedJsonEscaping); + yield return (GetProp(nameof(JsonSerializerOptions.DictionaryKeyPolicy)), JsonNamingPolicy.CamelCase); + yield return (GetProp(nameof(JsonSerializerOptions.IgnoreNullValues)), true); + yield return (GetProp(nameof(JsonSerializerOptions.DefaultIgnoreCondition)), JsonIgnoreCondition.WhenWritingDefault); + yield return (GetProp(nameof(JsonSerializerOptions.NumberHandling)), JsonNumberHandling.AllowReadingFromString); + yield return (GetProp(nameof(JsonSerializerOptions.IgnoreReadOnlyProperties)), true); + yield return (GetProp(nameof(JsonSerializerOptions.IgnoreReadOnlyFields)), true); + yield return (GetProp(nameof(JsonSerializerOptions.IncludeFields)), true); + yield return (GetProp(nameof(JsonSerializerOptions.MaxDepth)), 11); + yield return (GetProp(nameof(JsonSerializerOptions.PropertyNamingPolicy)), JsonNamingPolicy.CamelCase); + yield return (GetProp(nameof(JsonSerializerOptions.PropertyNameCaseInsensitive)), true); + yield return (GetProp(nameof(JsonSerializerOptions.ReadCommentHandling)), JsonCommentHandling.Skip); + yield return (GetProp(nameof(JsonSerializerOptions.UnknownTypeHandling)), JsonUnknownTypeHandling.JsonNode); + yield return (GetProp(nameof(JsonSerializerOptions.WriteIndented)), true); + yield return (GetProp(nameof(JsonSerializerOptions.ReferenceHandler)), ReferenceHandler.Preserve); + + static PropertyInfo GetProp(string name) + { + PropertyInfo property = typeof(JsonSerializerOptions).GetProperty(name, BindingFlags.Public | BindingFlags.Instance); + Assert.True(property.CanWrite); + return property; + } + } + + static IEnumerable GetAllPublicPropertiesWithSetters() + => typeof(JsonSerializerOptions) + .GetProperties(BindingFlags.Public | BindingFlags.Instance) + .Where(p => p.CanWrite); + } + + [Fact] + [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] + public static void JsonSerializerOptions_EqualityComparer_ApplyingJsonSerializerContextShouldReturnFalse() + { + // This test uses reflection to: + // - Access JsonSerializerOptions.EqualityComparer + // + // If either of them changes, this test will need to be kept in sync. + + IEqualityComparer 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)); + + _ = new MyJsonContext(options1); // Associate copy with a JsonSerializerContext + Assert.False(equalityComparer.Equals(options1, options2)); + Assert.NotEqual(equalityComparer.GetHashCode(options1), equalityComparer.GetHashCode(options2)); + } + + private class MyJsonContext : JsonSerializerContext + { + public MyJsonContext(JsonSerializerOptions options) : base(options) { } + + public override JsonTypeInfo? GetTypeInfo(Type _) => null; + + protected override JsonSerializerOptions? GeneratedSerializerOptions => Options; + } + + public static IEqualityComparer CreateEqualityComparerAccessor() + { + Type equalityComparerType = typeof(JsonSerializerOptions).GetNestedType("EqualityComparer", BindingFlags.NonPublic); + Assert.NotNull(equalityComparerType); + return (IEqualityComparer)Activator.CreateInstance(equalityComparerType, nonPublic: true); } public static IEnumerable WriteSuccessCases diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/System.Text.Json.Tests.csproj b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/System.Text.Json.Tests.csproj index 037d2b234e8c3c..ebc4668c96492a 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/System.Text.Json.Tests.csproj +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/System.Text.Json.Tests.csproj @@ -11,6 +11,7 @@ true $(WasmXHarnessArgs) --timeout=1800 + true From fea55935e51464ef99f4dc8dfaeac3f093deed4c Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Mon, 7 Feb 2022 17:06:53 +0000 Subject: [PATCH 03/10] Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.Cache.cs --- .../Metadata/ReflectionEmitCachingMemberAccessor.Cache.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.Cache.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.Cache.cs index 36de149b0ddebb..3e9a6d0f7aef34 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.Cache.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.Cache.cs @@ -38,7 +38,7 @@ public TValue GetOrAdd(TKey key, Func valueFactory) where long utcNowTicks = DateTime.UtcNow.Ticks; Volatile.Write(ref entry.LastUsedTicks, utcNowTicks); - if (utcNowTicks - Volatile.Read(ref _lastEvictedTicks) > _evictionIntervalTicks) + if (utcNowTicks - Volatile.Read(ref _lastEvictedTicks) >= _evictionIntervalTicks) { if (Interlocked.CompareExchange(ref _lock, 1, 0) == 0) { From 0e049e78a47956960a5652de91fec1176d18bf80 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Thu, 10 Feb 2022 13:04:10 +0000 Subject: [PATCH 04/10] address feedback --- .../JsonSerializerOptions.Caching.cs | 20 ++++++++++++------- .../JsonSerializerOptions.Converters.cs | 2 +- .../Serialization/Metadata/JsonTypeInfo.cs | 2 +- ...flectionEmitCachingMemberAccessor.Cache.cs | 14 +++++++------ .../ReflectionEmitCachingMemberAccessor.cs | 6 +++--- .../JsonSerializerContextTests.cs | 3 +++ 6 files changed, 29 insertions(+), 18 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 8756f0c68ea53b..ac770218e9fd8e 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 @@ -73,11 +73,7 @@ internal void ClearCaches() private void InitializeCachingContext() { -#if NETCOREAPP _cachingContext = TrackedCachingContexts.GetOrCreate(this); -#else - _cachingContext = new CachingContext(this); -#endif } /// @@ -111,7 +107,6 @@ public void Clear() } } -#if NETCOREAPP /// /// Defines a cache of CachingContexts; instead of using a ConditionalWeakTable which can be slow to traverse /// this approach uses a regular dictionary pointing to weak references of . @@ -226,7 +221,7 @@ static int EstimateEvictionRunsToSkip(int latestEvictionCount) // - If avgEvictionsPerRun >= 1 we skip no subsequent eviction calls. // - If avgEvictionsPerRun < 1 we skip ~ `1 / avgEvictionsPerRun` calls. - int runsPerEviction = (int)Math.Round(1 / Math.Clamp(avgEvictionsPerRun, 0.1, 1)); + int runsPerEviction = (int)Math.Round(1 / Math.Min(Math.Max(avgEvictionsPerRun, 0.1), 1)); Debug.Assert(runsPerEviction >= 1 && runsPerEviction <= 10); return runsPerEviction - 1; } @@ -315,7 +310,18 @@ public int GetHashCode(JsonSerializerOptions options) 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/JsonSerializerOptions.Converters.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs index f8787a2eba8fdd..751e9f8503cbb4 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs @@ -93,7 +93,7 @@ void Add(JsonConverter converter) => /// public IList Converters { get; } - internal JsonConverter GetConverterForMember(Type? parentClassType, Type runtimePropertyType, MemberInfo? memberInfo) + internal JsonConverter GetConverterFromMember(Type? parentClassType, Type runtimePropertyType, MemberInfo? memberInfo) { JsonConverter converter = null!; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs index 64a88d4249402d..c309c3bcd8a0bf 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs @@ -600,7 +600,7 @@ private static JsonConverter GetConverter( Debug.Assert(type != null); ValidateType(type, parentClassType, memberInfo, options); - JsonConverter converter = options.GetConverterForMember(parentClassType, type, memberInfo); + JsonConverter converter = options.GetConverterFromMember(parentClassType, type, memberInfo); // The runtimeType is the actual value being assigned to the property. // There are three types to consider for the runtimeType: diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.Cache.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.Cache.cs index 3e9a6d0f7aef34..c452977da0dcca 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.Cache.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.Cache.cs @@ -13,9 +13,9 @@ internal sealed partial class ReflectionEmitCachingMemberAccessor { private sealed class Cache where TKey : notnull { - private int _lock; - private long _lastEvictedTicks; // tracks total number of invocations to the cache; can be allowed to overflow. - private readonly long _evictionIntervalTicks; // number of cache invocations needed before triggering an eviction run. + private int _evictLock; + private long _lastEvictedTicks; // timestamp of latest eviction operation. + private readonly long _evictionIntervalTicks; // min timespan needed to trigger a new evict operation. private readonly long _slidingExpirationTicks; // max timespan allowed for cache entries to remain inactive. private readonly ConcurrentDictionary _cache = new(); @@ -28,7 +28,8 @@ public Cache(TimeSpan slidingExpiration, TimeSpan evictionInterval) public TValue GetOrAdd(TKey key, Func valueFactory) where TValue : class? { - CacheEntry entry = _cache.GetOrAdd(key, + CacheEntry entry = _cache.GetOrAdd( + key, #if NETCOREAPP static (TKey key, Func valueFactory) => new(valueFactory(key)), valueFactory); @@ -40,14 +41,15 @@ public TValue GetOrAdd(TKey key, Func valueFactory) where if (utcNowTicks - Volatile.Read(ref _lastEvictedTicks) >= _evictionIntervalTicks) { - if (Interlocked.CompareExchange(ref _lock, 1, 0) == 0) + if (Interlocked.CompareExchange(ref _evictLock, 1, 0) == 0) { if (utcNowTicks - _lastEvictedTicks >= _evictionIntervalTicks) { EvictStaleCacheEntries(utcNowTicks); Volatile.Write(ref _lastEvictedTicks, utcNowTicks); - Volatile.Write(ref _lock, 0); } + + Volatile.Write(ref _evictLock, 0); } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.cs index d72d98284a0dea..7531e558bce05f 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.cs @@ -19,7 +19,7 @@ internal sealed partial class ReflectionEmitCachingMemberAccessor : MemberAccess public override Action CreateAddMethodDelegate<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] TCollection>() => s_cache.GetOrAdd((nameof(CreateAddMethodDelegate), typeof(TCollection), null), [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2091:UnrecognizedReflectionPattern", - Justification = "DynamicallyAccessedMembersAttribute is not inherited by scoped generic parameter in lambda body.")] + Justification = "Parent method annotation does not flow to lambda method, cf. https://github.com/dotnet/roslyn/issues/46646")] static (_) => s_sourceAccessor.CreateAddMethodDelegate()); public override JsonTypeInfo.ConstructorDelegate? CreateConstructor([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type classType) @@ -38,14 +38,14 @@ public override Action CreateFieldSetter(FieldInfo public override Func>, TCollection> CreateImmutableDictionaryCreateRangeDelegate() => s_cache.GetOrAdd((nameof(CreateImmutableDictionaryCreateRangeDelegate), typeof((TCollection, TKey, TValue)), null), [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", - Justification = "Factory method calls into chained RequiresUnreferencedCode method.")] + Justification = "Parent method annotation does not flow to lambda method, cf. https://github.com/dotnet/roslyn/issues/46646")] static (_) => s_sourceAccessor.CreateImmutableDictionaryCreateRangeDelegate()); [RequiresUnreferencedCode(IEnumerableConverterFactoryHelpers.ImmutableConvertersUnreferencedCodeMessage)] public override Func, TCollection> CreateImmutableEnumerableCreateRangeDelegate() => s_cache.GetOrAdd((nameof(CreateImmutableEnumerableCreateRangeDelegate), typeof((TCollection, TElement)), null), [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", - Justification = "Factory method calls into chained RequiresUnreferencedCode method.")] + Justification = "Parent method annotation does not flow to lambda method, cf. https://github.com/dotnet/roslyn/issues/46646")] static (_) => s_sourceAccessor.CreateImmutableEnumerableCreateRangeDelegate()); public override Func? CreateParameterizedConstructor(ConstructorInfo constructor) diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs index 741c73e9bb0bc6..6442dbb6de2c4e 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs @@ -45,6 +45,9 @@ public static void Converters_AndTypeInfoCreator_NotRooted_WhenMetadataNotPresen AssertFieldNull("s_defaultSimpleConverters", optionsInstance: null); AssertFieldNull("s_defaultFactoryConverters", optionsInstance: null); + // Confirm type info dynamic creator not set. + AssertFieldNull("s_typeInfoCreationFunc", optionsInstance: null); + static void AssertFieldNull(string fieldName, JsonSerializerOptions? optionsInstance) { BindingFlags bindingFlags = BindingFlags.NonPublic | (optionsInstance == null ? BindingFlags.Static : BindingFlags.Instance); From a2b897563a5265ea4243fa42a689ddb5537fe02b Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Thu, 10 Feb 2022 13:05:19 +0000 Subject: [PATCH 05/10] tweak cache eviction constants --- .../Metadata/ReflectionEmitCachingMemberAccessor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.cs index 7531e558bce05f..61f9a8a3c9c02a 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitCachingMemberAccessor.cs @@ -12,7 +12,7 @@ internal sealed partial class ReflectionEmitCachingMemberAccessor : MemberAccess { private static readonly ReflectionEmitMemberAccessor s_sourceAccessor = new(); private static readonly Cache<(string id, Type declaringType, MemberInfo? member)> s_cache = - new(slidingExpiration: TimeSpan.FromSeconds(5), evictionInterval: TimeSpan.FromSeconds(1)); + new(slidingExpiration: TimeSpan.FromMilliseconds(1000), evictionInterval: TimeSpan.FromMilliseconds(200)); public static void Clear() => s_cache.Clear(); From f9efaf954c522e64472a29e101aefaa8fb896314 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Fri, 11 Feb 2022 22:36:16 +0000 Subject: [PATCH 06/10] Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs --- .../Text/Json/Serialization/JsonSerializerOptions.Caching.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 ac770218e9fd8e..87f1580584bf23 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 @@ -172,7 +172,7 @@ private static bool TryEvictDanglingEntries() // Worst case scenario, the cache has been saturated with permanent entries. // We want to avoid iterating needlessly through the entire cache every time // a new options instance is created. For this reason we implement a backoff - // strategy to amortize the cost of eviction across multiple cache initializations. + // strategy to average out the cost of eviction across multiple cache initializations. // The backoff count is determined by the eviction rates of the most recent eviction runs. if (s_evictionRunsToSkip > 0) From 801bf0bf948d7f609090c182803cc59dee98296d Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Mon, 14 Feb 2022 19:24:24 +0000 Subject: [PATCH 07/10] minor refinements to cache eviction algorithm --- .../JsonSerializerOptions.Caching.cs | 70 +++++++++++-------- 1 file changed, 39 insertions(+), 31 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 87f1580584bf23..837ef8b1957e3e 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 @@ -118,20 +118,22 @@ private static class TrackedCachingContexts private static readonly ConcurrentDictionary> s_cache = new(concurrencyLevel: 1, capacity: MaxTrackedContexts, new EqualityComparer()); - private const int EvictionCountHistory = 10; + private const int EvictionCountHistory = 16; private static Queue s_recentEvictionCounts = new(EvictionCountHistory); private static int s_evictionRunsToSkip; public static CachingContext GetOrCreate(JsonSerializerOptions options) { - if (s_cache.TryGetValue(options, out WeakReference? wr) && wr.TryGetTarget(out CachingContext? ctx)) + ConcurrentDictionary> cache = s_cache; + + if (cache.TryGetValue(options, out WeakReference? wr) && wr.TryGetTarget(out CachingContext? ctx)) { return ctx; } - lock (s_cache) + lock (cache) { - if (s_cache.TryGetValue(options, out wr)) + if (cache.TryGetValue(options, out wr)) { if (!wr.TryGetTarget(out ctx)) { @@ -143,7 +145,7 @@ public static CachingContext GetOrCreate(JsonSerializerOptions options) return ctx; } - if (s_cache.Count == MaxTrackedContexts) + if (cache.Count == MaxTrackedContexts) { if (!TryEvictDanglingEntries()) { @@ -152,7 +154,7 @@ public static CachingContext GetOrCreate(JsonSerializerOptions options) } } - Debug.Assert(s_cache.Count < MaxTrackedContexts); + Debug.Assert(cache.Count < MaxTrackedContexts); // Use a defensive copy of the options instance as key to // avoid capturing references to any caching contexts. @@ -160,7 +162,7 @@ public static CachingContext GetOrCreate(JsonSerializerOptions options) Debug.Assert(key._cachingContext == null); ctx = new CachingContext(options); - bool success = s_cache.TryAdd(key, new(ctx)); + bool success = cache.TryAdd(key, new(ctx)); Debug.Assert(success); return ctx; @@ -169,11 +171,12 @@ public static CachingContext GetOrCreate(JsonSerializerOptions options) private static bool TryEvictDanglingEntries() { - // Worst case scenario, the cache has been saturated with permanent entries. - // We want to avoid iterating needlessly through the entire cache every time - // a new options instance is created. For this reason we implement a backoff - // strategy to average out the cost of eviction across multiple cache initializations. - // The backoff count is determined by the eviction rates of the most recent eviction runs. + // 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. if (s_evictionRunsToSkip > 0) { @@ -198,33 +201,38 @@ private static bool TryEvictDanglingEntries() // Estimate the number of eviction runs to skip based on recent eviction rates. static int EstimateEvictionRunsToSkip(int latestEvictionCount) { - if (s_recentEvictionCounts.Count < EvictionCountHistory) + Queue recentEvictionCounts = s_recentEvictionCounts; + + if (recentEvictionCounts.Count < EvictionCountHistory - 1) { - // Insufficient data to determine a skip count. - s_recentEvictionCounts.Enqueue(latestEvictionCount); + // Insufficient data points to determine a skip count. + recentEvictionCounts.Enqueue(latestEvictionCount); return 0; } - else + else if (recentEvictionCounts.Count == EvictionCountHistory) { - s_recentEvictionCounts.Dequeue(); - s_recentEvictionCounts.Enqueue(latestEvictionCount); + recentEvictionCounts.Dequeue(); + } - // Calculate the average evictions per run. - double avgEvictionsPerRun = 0; + recentEvictionCounts.Enqueue(latestEvictionCount); - foreach (int rate in s_recentEvictionCounts) - { - avgEvictionsPerRun += rate; - } + // 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. - avgEvictionsPerRun /= EvictionCountHistory; - - // - If avgEvictionsPerRun >= 1 we skip no subsequent eviction calls. - // - If avgEvictionsPerRun < 1 we skip ~ `1 / avgEvictionsPerRun` calls. - int runsPerEviction = (int)Math.Round(1 / Math.Min(Math.Max(avgEvictionsPerRun, 0.1), 1)); - Debug.Assert(runsPerEviction >= 1 && runsPerEviction <= 10); - return runsPerEviction - 1; + int totalEvictions = 0; + foreach (int evictionCount in recentEvictionCounts) + { + totalEvictions += evictionCount; } + + int evictionRunsToSkip = + totalEvictions >= EvictionCountHistory ? 0 : + (int)Math.Round((double)EvictionCountHistory / Math.Max(totalEvictions, 1)); + + Debug.Assert(0 <= evictionRunsToSkip && evictionRunsToSkip <= EvictionCountHistory); + return evictionRunsToSkip; } } } From 5b917e2195853a411263ba000aad5686c4d00462 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Mon, 14 Feb 2022 19:26:28 +0000 Subject: [PATCH 08/10] rename JsonSerializerOptions._context to _serializerContext --- .../Converters/JsonMetadataServicesConverter.cs | 2 +- .../Serialization/JsonSerializer.Write.Helpers.cs | 6 +++--- .../Text/Json/Serialization/JsonSerializerContext.cs | 6 +++--- .../Serialization/JsonSerializerOptions.Caching.cs | 6 +++--- .../JsonSerializerOptions.Converters.cs | 4 ++-- .../Text/Json/Serialization/JsonSerializerOptions.cs | 12 ++++++------ .../Serialization/Metadata/JsonTypeInfo.Cache.cs | 4 ++-- .../MetadataTests/MetadataTests.Options.cs | 2 +- 8 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/JsonMetadataServicesConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/JsonMetadataServicesConverter.cs index 2596023f6b0dd8..3ab98e043385da 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/JsonMetadataServicesConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/JsonMetadataServicesConverter.cs @@ -76,7 +76,7 @@ internal override bool OnTryWrite(Utf8JsonWriter writer, T value, JsonSerializer if (!state.SupportContinuation && jsonTypeInfo is JsonTypeInfo info && info.SerializeHandler != null && - info.Options._context?.CanUseSerializationLogic == true) + info.Options._serializerContext?.CanUseSerializationLogic == true) { info.SerializeHandler(writer, value); return true; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.Helpers.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.Helpers.cs index 415f1417ae6071..45a39dc58511fe 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.Helpers.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.Helpers.cs @@ -41,7 +41,7 @@ private static void WriteUsingGeneratedSerializer(Utf8JsonWriter writer, if (jsonTypeInfo.HasSerialize && jsonTypeInfo is JsonTypeInfo typedInfo && - typedInfo.Options._context?.CanUseSerializationLogic == true) + typedInfo.Options._serializerContext?.CanUseSerializationLogic == true) { Debug.Assert(typedInfo.SerializeHandler != null); typedInfo.SerializeHandler(writer, value); @@ -59,8 +59,8 @@ private static void WriteUsingSerializer(Utf8JsonWriter writer, in TValu Debug.Assert(!jsonTypeInfo.HasSerialize || jsonTypeInfo is not JsonTypeInfo || - jsonTypeInfo.Options._context == null || - !jsonTypeInfo.Options._context.CanUseSerializationLogic, + jsonTypeInfo.Options._serializerContext == null || + !jsonTypeInfo.Options._serializerContext.CanUseSerializationLogic, "Incorrect method called. WriteUsingGeneratedSerializer() should have been called instead."); WriteStack state = default; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerContext.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerContext.cs index 0ce18180072c9e..7254cd6f2cf09e 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerContext.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerContext.cs @@ -30,7 +30,7 @@ public JsonSerializerOptions Options if (_options == null) { _options = new JsonSerializerOptions(); - _options._context = this; + _options._serializerContext = this; } return _options; @@ -97,13 +97,13 @@ protected JsonSerializerContext(JsonSerializerOptions? options) { if (options != null) { - if (options._context != null) + if (options._serializerContext != null) { ThrowHelper.ThrowInvalidOperationException_JsonSerializerOptionsAlreadyBoundToContext(); } _options = options; - options._context = this; + options._serializerContext = this; } } 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 837ef8b1957e3e..62350c1d08c81a 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 @@ -158,7 +158,7 @@ public static CachingContext GetOrCreate(JsonSerializerOptions options) // Use a defensive copy of the options instance as key to // avoid capturing references to any caching contexts. - var key = new JsonSerializerOptions(options) { _context = options._context }; + var key = new JsonSerializerOptions(options) { _serializerContext = options._serializerContext }; Debug.Assert(key._cachingContext == null); ctx = new CachingContext(options); @@ -265,7 +265,7 @@ public bool Equals(JsonSerializerOptions? left, JsonSerializerOptions? right) left._includeFields == right._includeFields && left._propertyNameCaseInsensitive == right._propertyNameCaseInsensitive && left._writeIndented == right._writeIndented && - left._context == right._context && + left._serializerContext == right._serializerContext && CompareConverters(left.Converters, right.Converters); static bool CompareConverters(IList left, IList right) @@ -309,7 +309,7 @@ public int GetHashCode(JsonSerializerOptions options) hc.Add(options._includeFields); hc.Add(options._propertyNameCaseInsensitive); hc.Add(options._writeIndented); - hc.Add(options._context); + hc.Add(options._serializerContext); foreach (JsonConverter converter in options.Converters) { diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs index 751e9f8503cbb4..fda10abb0c0752 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs @@ -184,7 +184,7 @@ private JsonConverter GetConverterFromType(Type typeToConvert) Debug.Assert(typeToConvert != null); // Priority 1: If there is a JsonSerializerContext, fetch the converter from there. - JsonConverter? converter = _context?.GetTypeInfo(typeToConvert)?.PropertyInfoForTypeInfo?.ConverterBase; + JsonConverter? converter = _serializerContext?.GetTypeInfo(typeToConvert)?.PropertyInfoForTypeInfo?.ConverterBase; // Priority 2: Attempt to get custom converter added at runtime. // Currently there is not a way at runtime to override the [JsonConverter] when applied to a property. @@ -311,7 +311,7 @@ private JsonConverter GetConverterFromAttribute(JsonConverterAttribute converter internal bool TryGetDefaultSimpleConverter(Type typeToConvert, [NotNullWhen(true)] out JsonConverter? converter) { - if (_context == null && // For consistency do not return any default converters for + if (_serializerContext == null && // For consistency do not return any default converters for // options instances linked to a JsonSerializerContext, // even if the default converters might have been rooted. s_defaultSimpleConverters != null && diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs index 46e1725fd89de9..34915078571813 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs @@ -33,7 +33,7 @@ public sealed partial class JsonSerializerOptions /// public static JsonSerializerOptions Default { get; } = CreateDefaultImmutableInstance(); - internal JsonSerializerContext? _context; + internal JsonSerializerContext? _serializerContext; // Stores the JsonTypeInfo factory, which requires unreferenced code and must be rooted by the reflection-based serializer. private static Func? s_typeInfoCreationFunc; @@ -150,13 +150,13 @@ public JsonSerializerOptions(JsonSerializerDefaults defaults) : this() /// public void AddContext() where TContext : JsonSerializerContext, new() { - if (_context != null) + if (_serializerContext != null) { ThrowHelper.ThrowInvalidOperationException_JsonSerializerOptionsAlreadyBoundToContext(); } TContext context = new(); - _context = context; + _serializerContext = context; context._options = this; } @@ -598,7 +598,7 @@ internal static void InitializeForReflectionSerializer() private JsonTypeInfo GetJsonTypeInfoFromContextOrCreate(Type type) { - JsonTypeInfo? info = _context?.GetTypeInfo(type); + JsonTypeInfo? info = _serializerContext?.GetTypeInfo(type); if (info != null) { return info; @@ -656,9 +656,9 @@ internal JsonWriterOptions GetWriterOptions() internal void VerifyMutable() { - if (_cachingContext != null || _context != null) + if (_cachingContext != null || _serializerContext != null) { - ThrowHelper.ThrowInvalidOperationException_SerializerOptionsImmutable(_context); + ThrowHelper.ThrowInvalidOperationException_SerializerOptionsImmutable(_serializerContext); } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs index f696cefe8f21cd..7cea26d207210f 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs @@ -580,7 +580,7 @@ internal void InitializePropCache() Debug.Assert(PropertyCache == null); Debug.Assert(PropertyInfoForTypeInfo.ConverterStrategy == ConverterStrategy.Object); - JsonSerializerContext? context = Options._context; + JsonSerializerContext? context = Options._serializerContext; Debug.Assert(context != null); JsonPropertyInfo[] array; @@ -636,7 +636,7 @@ internal void InitializeParameterCache() Debug.Assert(PropertyCache != null); Debug.Assert(PropertyInfoForTypeInfo.ConverterStrategy == ConverterStrategy.Object); - JsonSerializerContext? context = Options._context; + JsonSerializerContext? context = Options._serializerContext; Debug.Assert(context != null); JsonParameterInfoValues[] array; diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/MetadataTests.Options.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/MetadataTests.Options.cs index 567a2776b1e0c8..1a092379ee891d 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/MetadataTests.Options.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/MetadataTests.Options.cs @@ -55,7 +55,7 @@ public void AddContextOverwritesOptionsForFreshContext() // Those options are overwritten when context is binded via options.AddContext(); JsonSerializerOptions options = new(); options.AddContext(); // No error. - FieldInfo contextField = typeof(JsonSerializerOptions).GetField("_context", BindingFlags.NonPublic | BindingFlags.Instance); + FieldInfo contextField = typeof(JsonSerializerOptions).GetField("_serializerContext", BindingFlags.NonPublic | BindingFlags.Instance); Assert.NotNull(contextField); Assert.Same(options, ((JsonSerializerContext)contextField.GetValue(options)).Options); } From 761b2ad3b30ccb35594b3d863f1a364e521a5ac0 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Mon, 14 Feb 2022 20:02:47 +0000 Subject: [PATCH 09/10] ensure that the update handler clears the shared caching contexts --- .../Serialization/JsonSerializerOptions.Caching.cs | 14 ++++++++++++-- .../JsonSerializerOptionsUpdateHandler.cs | 3 +++ 2 files changed, 15 insertions(+), 2 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 62350c1d08c81a..833ebb910fb487 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 @@ -82,7 +82,7 @@ private void InitializeCachingContext() /// this is to prevent multiple options instances from leaking into the object graphs of converters which /// could break user invariants. /// - private sealed class CachingContext + internal sealed class CachingContext { private readonly ConcurrentDictionary _converterCache = new(); private readonly ConcurrentDictionary _jsonTypeInfoCache = new(); @@ -112,7 +112,7 @@ public void Clear() /// this approach uses a regular dictionary pointing to weak references of . /// Relevant caching contexts are looked up using the equality comparison defined by . /// - private static class TrackedCachingContexts + internal static class TrackedCachingContexts { private const int MaxTrackedContexts = 64; private static readonly ConcurrentDictionary> s_cache = @@ -169,6 +169,16 @@ public static CachingContext GetOrCreate(JsonSerializerOptions options) } } + public static void Clear() + { + 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. 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 ee80e36205f908..1b71a7c5f30d06 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 @@ -21,6 +21,9 @@ public static void ClearCache(Type[]? types) options.Key.ClearCaches(); } + // Flush the shared caching context cache + JsonSerializerOptions.TrackedCachingContexts.Clear(); + // Flush the dynamic method cache ReflectionEmitCachingMemberAccessor.Clear(); } From 3d9e9e2eb6eae0b60fdf49e4d1655ed90bcccf59 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Mon, 14 Feb 2022 20:05:09 +0000 Subject: [PATCH 10/10] fix remark --- .../Json/Serialization/JsonSerializerOptionsUpdateHandler.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 1b71a7c5f30d06..4fd3c66edae085 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 @@ -21,7 +21,7 @@ public static void ClearCache(Type[]? types) options.Key.ClearCaches(); } - // Flush the shared caching context cache + // Flush the shared caching contexts JsonSerializerOptions.TrackedCachingContexts.Clear(); // Flush the dynamic method cache