From 53df6c07ba2a05655cacf5cf3cc8ad2a8fdef418 Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Wed, 5 Jul 2023 11:13:21 -0700 Subject: [PATCH 1/5] Add tests to check for ALC goodness with DCS. --- .../tests/DataContractSerializer.cs | 85 ++++++++++++++++++- ...ialization.Xml.ReflectionOnly.Tests.csproj | 6 +- ...tem.Runtime.Serialization.Xml.Tests.csproj | 6 +- 3 files changed, 94 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Runtime.Serialization.Xml/tests/DataContractSerializer.cs b/src/libraries/System.Runtime.Serialization.Xml/tests/DataContractSerializer.cs index 8484775ea230a0..0ae22abf361777 100644 --- a/src/libraries/System.Runtime.Serialization.Xml/tests/DataContractSerializer.cs +++ b/src/libraries/System.Runtime.Serialization.Xml/tests/DataContractSerializer.cs @@ -19,7 +19,8 @@ using System.Xml.Schema; using Xunit; using System.Runtime.Serialization.Tests; - +using System.Runtime.CompilerServices; +using System.Runtime.Loader; public static partial class DataContractSerializerTests { @@ -1115,6 +1116,88 @@ public static void DCS_TypeNamesWithSpecialCharacters() Assert.Equal(x.PropertyNameWithSpecialCharacters\u6F22\u00F1, y.PropertyNameWithSpecialCharacters\u6F22\u00F1); } + [Fact] +#if XMLSERIALIZERGENERATORTESTS + // Lack of AssemblyDependencyResolver results in assemblies that are not loaded by path to get + // loaded in the default ALC, which causes problems for this test. + [SkipOnPlatform(TestPlatforms.Browser, "AssemblyDependencyResolver not supported in wasm")] +#endif + [ActiveIssue("34072", TestRuntimes.Mono)] + public static void DCS_TypeInCollectibleALC() + { + ExecuteAndUnload("SerializableAssembly.dll", "SerializationTypes.SimpleType", makeCollection: false, out var weakRef); + + for (int i = 0; weakRef.IsAlive && i < 10; i++) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + } + Assert.True(!weakRef.IsAlive); + } + + [Fact] +#if XMLSERIALIZERGENERATORTESTS + // Lack of AssemblyDependencyResolver results in assemblies that are not loaded by path to get + // loaded in the default ALC, which causes problems for this test. + [SkipOnPlatform(TestPlatforms.Browser, "AssemblyDependencyResolver not supported in wasm")] +#endif + [ActiveIssue("34072", TestRuntimes.Mono)] + public static void DCS_CollectionTypeInCollectibleALC() + { + ExecuteAndUnload("SerializableAssembly.dll", "SerializationTypes.SimpleType", makeCollection: true, out var weakRef); + + for (int i = 0; weakRef.IsAlive && i < 10; i++) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + } + Assert.True(!weakRef.IsAlive); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void ExecuteAndUnload(string assemblyfile, string typename, bool makeCollection, out WeakReference wref) + { + var fullPath = Path.GetFullPath(assemblyfile); + var alc = new TestAssemblyLoadContext("DataContractSerializerTests", true, fullPath); + object obj; + wref = new WeakReference(alc); + + // Load assembly by path. By name, and it gets loaded in the default ALC. + var asm = alc.LoadFromAssemblyPath(fullPath); + + // Ensure the type loaded in the intended non-Default ALC + var type = asm.GetType(typename); + Assert.Equal(AssemblyLoadContext.GetLoadContext(type.Assembly), alc); + Assert.NotEqual(alc, AssemblyLoadContext.Default); + + if (makeCollection) + { + int arrayLength = 3; + var array = Array.CreateInstance(type, arrayLength); + for (int i = 0; i < arrayLength; i++) + { + array.SetValue(Activator.CreateInstance(type), i); + } + type = array.GetType(); + obj = array; + } + else + { + obj = Activator.CreateInstance(type); + } + + // Round-Trip the instance + var dcs = new DataContractSerializer(type); + var rtobj = DataContractSerializerHelper.SerializeAndDeserialize(obj, null, null, () => dcs, true, false); + Assert.NotNull(rtobj); + if (makeCollection) + Assert.Equal(obj, rtobj); + else + Assert.True(rtobj.Equals(obj)); + + alc.Unload(); + } + [Fact] public static void DCS_JaggedArrayAsRoot() { diff --git a/src/libraries/System.Runtime.Serialization.Xml/tests/ReflectionOnly/System.Runtime.Serialization.Xml.ReflectionOnly.Tests.csproj b/src/libraries/System.Runtime.Serialization.Xml/tests/ReflectionOnly/System.Runtime.Serialization.Xml.ReflectionOnly.Tests.csproj index bdc0d2c86c22c0..d128172dd2acc6 100644 --- a/src/libraries/System.Runtime.Serialization.Xml/tests/ReflectionOnly/System.Runtime.Serialization.Xml.ReflectionOnly.Tests.csproj +++ b/src/libraries/System.Runtime.Serialization.Xml/tests/ReflectionOnly/System.Runtime.Serialization.Xml.ReflectionOnly.Tests.csproj @@ -3,10 +3,14 @@ $(DefineConstants);ReflectionOnly $(NetCoreAppCurrent) + + + + - + diff --git a/src/libraries/System.Runtime.Serialization.Xml/tests/System.Runtime.Serialization.Xml.Tests.csproj b/src/libraries/System.Runtime.Serialization.Xml/tests/System.Runtime.Serialization.Xml.Tests.csproj index 1628ec4ae6ebd1..c299e17295d0a0 100644 --- a/src/libraries/System.Runtime.Serialization.Xml/tests/System.Runtime.Serialization.Xml.Tests.csproj +++ b/src/libraries/System.Runtime.Serialization.Xml/tests/System.Runtime.Serialization.Xml.Tests.csproj @@ -2,9 +2,13 @@ $(NetCoreAppCurrent) + + + + - + From a1478398d34fed9c8a3ac6fa514d21b6aec5e2ad Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Fri, 7 Jul 2023 18:28:30 -0700 Subject: [PATCH 2/5] Daniel-Svensson's proposed perf improvement... since it doesn't seem to interfere with what comes next. --- .../Runtime/Serialization/DataContract.cs | 70 ++++++------------- 1 file changed, 22 insertions(+), 48 deletions(-) diff --git a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/DataContract.cs b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/DataContract.cs index 49b2d19b83d772..d4ae8789709042 100644 --- a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/DataContract.cs +++ b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/DataContract.cs @@ -4,6 +4,7 @@ using System; using System.Buffers.Binary; using System.Collections; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Collections.ObjectModel; using System.Diagnostics; @@ -297,19 +298,16 @@ internal virtual bool IsValidContract() internal class DataContractCriticalHelper { - private static readonly Hashtable s_typeToIDCache = new Hashtable(new HashTableEqualityComparer()); + private static readonly ConcurrentDictionary s_typeToIDCache = new(); private static DataContract[] s_dataContractCache = new DataContract[32]; private static int s_dataContractID; - private static Dictionary? s_typeToBuiltInContract; + private static readonly ConcurrentDictionary s_typeToBuiltInContract = new(); private static Dictionary? s_nameToBuiltInContract; private static Dictionary? s_typeNameToBuiltInContract; private static readonly Hashtable s_namespaces = new Hashtable(); private static Dictionary? s_clrTypeStrings; private static XmlDictionary? s_clrTypeStringsDictionary; - [ThreadStatic] - private static TypeHandleRef? s_typeHandleRef; - private static readonly object s_cacheLock = new object(); private static readonly object s_createDataContractLock = new object(); private static readonly object s_initBuiltInContractsLock = new object(); @@ -393,36 +391,29 @@ private static bool ContractMatches(DataContract contract, DataContract cachedCo internal static int GetId(RuntimeTypeHandle typeHandle) { typeHandle = GetDataContractAdapterTypeHandle(typeHandle); - s_typeHandleRef ??= new TypeHandleRef(); - s_typeHandleRef.Value = typeHandle; - object? value = s_typeToIDCache[s_typeHandleRef]; - if (value != null) - return ((IntRef)value).Value; + if (s_typeToIDCache.TryGetValue(typeHandle.Value, out int id)) + return id; try { lock (s_cacheLock) { - value = s_typeToIDCache[s_typeHandleRef]; - if (value != null) - return ((IntRef)value).Value; - - int nextId = s_dataContractID++; - if (nextId >= s_dataContractCache.Length) + return s_typeToIDCache.GetOrAdd(typeHandle.Value, static _ => { - int newSize = (nextId < int.MaxValue / 2) ? nextId * 2 : int.MaxValue; - if (newSize <= nextId) + int nextId = s_dataContractID++; + if (nextId >= s_dataContractCache.Length) { - Debug.Fail("DataContract cache overflow"); - throw new SerializationException(SR.DataContractCacheOverflow); + int newSize = (nextId < int.MaxValue / 2) ? nextId * 2 : int.MaxValue; + if (newSize <= nextId) + { + Debug.Fail("DataContract cache overflow"); + throw new SerializationException(SR.DataContractCacheOverflow); + } + Array.Resize(ref s_dataContractCache, newSize); } - Array.Resize(ref s_dataContractCache, newSize); - } - IntRef id = new IntRef(nextId); - - s_typeToIDCache.Add(new TypeHandleRef(typeHandle), id); - return id.Value; + return nextId; + }); } } catch (Exception ex) when (!ExceptionUtility.IsFatal(ex)) @@ -589,17 +580,11 @@ private static RuntimeTypeHandle GetDataContractAdapterTypeHandle(RuntimeTypeHan if (type.IsInterface && !CollectionDataContract.IsCollectionInterface(type)) type = Globals.TypeOfObject; - lock (s_initBuiltInContractsLock) + return s_typeToBuiltInContract.GetOrAdd(type, static (Type key) => { - s_typeToBuiltInContract ??= new Dictionary(); - - if (!s_typeToBuiltInContract.TryGetValue(type, out DataContract? dataContract)) - { - TryCreateBuiltInDataContract(type, out dataContract); - s_typeToBuiltInContract.Add(type, dataContract); - } + TryCreateBuiltInDataContract(key, out DataContract? dataContract); return dataContract; - } + }); } [RequiresDynamicCode(DataContract.SerializerAOTWarning)] @@ -945,19 +930,8 @@ internal static void ThrowInvalidDataContractException(string? message, Type? ty { if (type != null) { - lock (s_cacheLock) - { - s_typeHandleRef ??= new TypeHandleRef(); - s_typeHandleRef.Value = GetDataContractAdapterTypeHandle(type.TypeHandle); - - if (s_typeToIDCache.ContainsKey(s_typeHandleRef)) - { - lock (s_cacheLock) - { - s_typeToIDCache.Remove(s_typeHandleRef); - } - } - } + RuntimeTypeHandle runtimeTypeHandle = GetDataContractAdapterTypeHandle(type.TypeHandle); + s_typeToIDCache.TryRemove(runtimeTypeHandle.Value, out _); } throw new InvalidDataContractException(message); From 1477cbe7b3b1dfbca7fdb2396d5edfe7ac341783 Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Fri, 7 Jul 2023 18:44:17 -0700 Subject: [PATCH 3/5] Apply same perf improvement to JDCS; Remove Int/TypeHandleRef silliness that we no longer need. --- .../Runtime/Serialization/DataContract.cs | 58 ------------------- .../Serialization/Json/JsonDataContract.cs | 32 ++++------ 2 files changed, 12 insertions(+), 78 deletions(-) diff --git a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/DataContract.cs b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/DataContract.cs index d4ae8789709042..78fe30328b8c74 100644 --- a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/DataContract.cs +++ b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/DataContract.cs @@ -2489,62 +2489,4 @@ public override int GetHashCode() return _object1.GetHashCode() ^ _object2.GetHashCode(); } } - - internal sealed class HashTableEqualityComparer : IEqualityComparer - { - bool IEqualityComparer.Equals(object? x, object? y) - { - return ((TypeHandleRef)x!).Value.Equals(((TypeHandleRef)y!).Value); - } - - public int GetHashCode(object obj) - { - return ((TypeHandleRef)obj).Value.GetHashCode(); - } - } - - internal sealed class TypeHandleRefEqualityComparer : IEqualityComparer - { - public bool Equals(TypeHandleRef? x, TypeHandleRef? y) - { - return x!.Value.Equals(y!.Value); - } - - public int GetHashCode(TypeHandleRef obj) - { - return obj.Value.GetHashCode(); - } - } - - internal sealed class TypeHandleRef - { - private RuntimeTypeHandle _value; - - public TypeHandleRef() - { - } - - public TypeHandleRef(RuntimeTypeHandle value) - { - _value = value; - } - - public RuntimeTypeHandle Value - { - get => _value; - set => _value = value; - } - } - - internal sealed class IntRef - { - private readonly int _value; - - public IntRef(int value) - { - _value = value; - } - - public int Value => _value; - } } diff --git a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/JsonDataContract.cs b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/JsonDataContract.cs index 50db0fd272948b..25654871992db8 100644 --- a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/JsonDataContract.cs +++ b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/Json/JsonDataContract.cs @@ -1,6 +1,7 @@ // 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; @@ -151,8 +152,7 @@ internal class JsonDataContractCriticalHelper private static JsonDataContract[] s_dataContractCache = new JsonDataContract[32]; private static int s_dataContractID; - private static readonly TypeHandleRef s_typeHandleRef = new TypeHandleRef(); - private static readonly Dictionary s_typeToIDCache = new Dictionary(new TypeHandleRefEqualityComparer()); + private static readonly ConcurrentDictionary s_typeToIDCache = new(); private DataContractDictionary? _knownDataContracts; private readonly DataContract _traditionalDataContract; private readonly string _typeName; @@ -188,34 +188,26 @@ public static JsonDataContract GetJsonDataContract(DataContract traditionalDataC internal static int GetId(RuntimeTypeHandle typeHandle) { + if (s_typeToIDCache.TryGetValue(typeHandle.Value, out int id)) + return id; + lock (s_cacheLock) { - IntRef? id; - s_typeHandleRef.Value = typeHandle; - if (!s_typeToIDCache.TryGetValue(s_typeHandleRef, out id)) + return s_typeToIDCache.GetOrAdd(typeHandle.Value, static _ => { - int value = s_dataContractID++; - if (value >= s_dataContractCache.Length) + int nextId = s_dataContractID++; + if (nextId >= s_dataContractCache.Length) { - int newSize = (value < int.MaxValue / 2) ? value * 2 : int.MaxValue; - if (newSize <= value) + int newSize = (nextId < int.MaxValue / 2) ? nextId * 2 : int.MaxValue; + if (newSize <= nextId) { Debug.Fail("DataContract cache overflow"); throw new SerializationException(SR.DataContractCacheOverflow); } Array.Resize(ref s_dataContractCache, newSize); } - id = new IntRef(value); - try - { - s_typeToIDCache.Add(new TypeHandleRef(typeHandle), id); - } - catch (Exception ex) when (!ExceptionUtility.IsFatal(ex)) - { - throw new Exception(ex.Message, ex); - } - } - return id.Value; + return nextId; + }); } } From 9f5e962db81f8e9a9a64d52187243abf39e65295 Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Tue, 11 Jul 2023 17:28:53 -0700 Subject: [PATCH 4/5] Add context-aware collections for caching/indexing DataContracts. --- ...m.Private.DataContractSerialization.csproj | 2 + .../Runtime/Serialization/ContextAware.cs | 149 ++++++++++++++++++ .../Runtime/Serialization/DataContract.cs | 26 +-- 3 files changed, 164 insertions(+), 13 deletions(-) create mode 100644 src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/ContextAware.cs diff --git a/src/libraries/System.Private.DataContractSerialization/src/System.Private.DataContractSerialization.csproj b/src/libraries/System.Private.DataContractSerialization/src/System.Private.DataContractSerialization.csproj index 8ee00c3c19581b..bd9a7cc8028184 100644 --- a/src/libraries/System.Private.DataContractSerialization/src/System.Private.DataContractSerialization.csproj +++ b/src/libraries/System.Private.DataContractSerialization/src/System.Private.DataContractSerialization.csproj @@ -145,6 +145,7 @@ + @@ -163,6 +164,7 @@ + diff --git a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/ContextAware.cs b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/ContextAware.cs new file mode 100644 index 00000000000000..3d0369cf0c566c --- /dev/null +++ b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/ContextAware.cs @@ -0,0 +1,149 @@ +// 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.Concurrent; +using System.Diagnostics.CodeAnalysis; +using System.Reflection; +using System.Runtime.CompilerServices; +using System.Runtime.Loader; + +namespace System.Runtime.Serialization +{ + internal sealed class ContextAwareIndex<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] T> where T : class? + { + private (T? strong, WeakReference? weak)[] _items; + private ConditionalWeakTable _keepAlive; + + public int Length => _items.Length; + + public ContextAwareIndex(int size) + { + _items = new (T?, WeakReference?)[size]; + _keepAlive = new ConditionalWeakTable(); + } + + public T? GetItem(int index) => _items[index].strong ?? (_items[index].weak?.TryGetTarget(out T? ret) == true ? ret : null); + + public void SetItem(int index, T value, Type keepAliveType) + { + // Check for unloadability to decide how to store the value + AssemblyLoadContext? alc = AssemblyLoadContext.GetLoadContext(keepAliveType.Assembly); + if (alc == null || !alc.IsCollectible) + { + _items[index].strong = value; + } + else + { + _items[index].weak = new WeakReference(value); + _keepAlive.Add(keepAliveType, value); + } + } + + public void Resize(int newSize) + { + Array.Resize<(T?, WeakReference?)>(ref _items, newSize); + } + } + + internal sealed class ContextAwareDictionary + where TType : Type + where T : class? + { + private readonly ConcurrentDictionary _fastDictionary = new(); + private readonly ConditionalWeakTable _collectibleTable = new(); + + + internal T GetOrAdd(TType t, Func f) + { + T? ret; + + // The fast and most common default case + if (_fastDictionary.TryGetValue(t, out ret)) + return ret; + + // Common case for collectible contexts + if (_collectibleTable.TryGetValue(t, out ret)) + return ret; + + // Not found. Do the slower work of creating the value in the correct collection. + AssemblyLoadContext? alc = AssemblyLoadContext.GetLoadContext(t.Assembly); + + // Null and non-collectible load contexts use the default table + if (alc == null || !alc.IsCollectible) + { + return _fastDictionary.GetOrAdd(t, f); + } + + // Collectible load contexts should use the ConditionalWeakTable so they can be unloaded + else + { + lock (_collectibleTable) + { + if (!_collectibleTable.TryGetValue(t, out ret)) + { + ret = f(t); + _collectibleTable.AddOrUpdate(t, ret); + } + } + } + + return ret; + } + } + + //internal sealed class ContextAwareTables<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]T> where T : class? + //{ + // private readonly Hashtable _defaultTable; + // private readonly ConditionalWeakTable _collectibleTable; + + // public ContextAwareTables() + // { + // _defaultTable = new Hashtable(); + // _collectibleTable = new ConditionalWeakTable(); + // } + + // internal T GetOrCreateValue(Type t, Func f) + // { + // // The fast and most common default case + // T? ret = (T?)_defaultTable[t]; + // if (ret != null) + // return ret; + + // // Common case for collectible contexts + // if (_collectibleTable.TryGetValue(t, out ret)) + // return ret; + + // // Not found. Do the slower work of creating the value in the correct collection. + // AssemblyLoadContext? alc = AssemblyLoadContext.GetLoadContext(t.Assembly); + + // // Null and non-collectible load contexts use the default table + // if (alc == null || !alc.IsCollectible) + // { + // lock (_defaultTable) + // { + // if ((ret = (T?)_defaultTable[t]) == null) + // { + // ret = f(t); + // _defaultTable[t] = ret; + // } + // } + // } + + // // Collectible load contexts should use the ConditionalWeakTable so they can be unloaded + // else + // { + // lock (_collectibleTable) + // { + // if (!_collectibleTable.TryGetValue(t, out ret)) + // { + // ret = f(t); + // _collectibleTable.AddOrUpdate(t, ret); + // } + // } + // } + + // return ret; + // } + //} +} diff --git a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/DataContract.cs b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/DataContract.cs index 78fe30328b8c74..e6b80eb4fbaf88 100644 --- a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/DataContract.cs +++ b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/DataContract.cs @@ -299,9 +299,9 @@ internal virtual bool IsValidContract() internal class DataContractCriticalHelper { private static readonly ConcurrentDictionary s_typeToIDCache = new(); - private static DataContract[] s_dataContractCache = new DataContract[32]; + private static readonly ContextAwareIndex s_dataContractCache = new(32); private static int s_dataContractID; - private static readonly ConcurrentDictionary s_typeToBuiltInContract = new(); + private static readonly ContextAwareDictionary s_typeToBuiltInContract = new(); private static Dictionary? s_nameToBuiltInContract; private static Dictionary? s_typeNameToBuiltInContract; private static readonly Hashtable s_namespaces = new Hashtable(); @@ -335,7 +335,7 @@ internal class DataContractCriticalHelper [RequiresUnreferencedCode(DataContract.SerializerTrimmerWarning)] internal static DataContract GetDataContractSkipValidation(int id, RuntimeTypeHandle typeHandle, Type? type) { - DataContract dataContract = s_dataContractCache[id]; + DataContract? dataContract = s_dataContractCache.GetItem(id); if (dataContract == null) { dataContract = CreateDataContract(id, typeHandle, type); @@ -351,13 +351,13 @@ internal static DataContract GetDataContractSkipValidation(int id, RuntimeTypeHa [RequiresUnreferencedCode(DataContract.SerializerTrimmerWarning)] internal static DataContract GetGetOnlyCollectionDataContractSkipValidation(int id, RuntimeTypeHandle typeHandle, Type? type) { - DataContract dataContract = s_dataContractCache[id] ?? CreateGetOnlyCollectionDataContract(id, typeHandle, type); + DataContract dataContract = s_dataContractCache.GetItem(id) ?? CreateGetOnlyCollectionDataContract(id, typeHandle, type); return dataContract; } internal static DataContract GetDataContractForInitialization(int id) { - DataContract dataContract = s_dataContractCache[id]; + DataContract? dataContract = s_dataContractCache.GetItem(id); if (dataContract == null) { throw new SerializationException(SR.DataContractCacheOverflow); @@ -368,14 +368,14 @@ internal static DataContract GetDataContractForInitialization(int id) internal static int GetIdForInitialization(ClassDataContract classContract) { int id = DataContract.GetId(classContract.TypeForInitialization.TypeHandle); - if (id < s_dataContractCache.Length && ContractMatches(classContract, s_dataContractCache[id])) + if (id < s_dataContractCache.Length && ContractMatches(classContract, s_dataContractCache.GetItem(id))) { return id; } int currentDataContractId = DataContractCriticalHelper.s_dataContractID; for (int i = 0; i < currentDataContractId; i++) { - if (ContractMatches(classContract, s_dataContractCache[i])) + if (ContractMatches(classContract, s_dataContractCache.GetItem(id))) { return i; } @@ -383,7 +383,7 @@ internal static int GetIdForInitialization(ClassDataContract classContract) throw new SerializationException(SR.DataContractCacheOverflow); } - private static bool ContractMatches(DataContract contract, DataContract cachedContract) + private static bool ContractMatches(DataContract contract, DataContract? cachedContract) { return (cachedContract != null && cachedContract.UnderlyingType == contract.UnderlyingType); } @@ -410,7 +410,7 @@ internal static int GetId(RuntimeTypeHandle typeHandle) Debug.Fail("DataContract cache overflow"); throw new SerializationException(SR.DataContractCacheOverflow); } - Array.Resize(ref s_dataContractCache, newSize); + s_dataContractCache.Resize(newSize); } return nextId; }); @@ -427,12 +427,12 @@ internal static int GetId(RuntimeTypeHandle typeHandle) [RequiresUnreferencedCode(DataContract.SerializerTrimmerWarning)] private static DataContract CreateDataContract(int id, RuntimeTypeHandle typeHandle, Type? type) { - DataContract? dataContract = s_dataContractCache[id]; + DataContract? dataContract = s_dataContractCache.GetItem(id); if (dataContract == null) { lock (s_createDataContractLock) { - dataContract = s_dataContractCache[id]; + dataContract = s_dataContractCache.GetItem(id); if (dataContract == null) { type ??= Type.GetTypeFromHandle(typeHandle)!; @@ -499,7 +499,7 @@ private static void AssignDataContractToId(DataContract dataContract, int id) { lock (s_cacheLock) { - s_dataContractCache[id] = dataContract; + s_dataContractCache.SetItem(id, dataContract, dataContract.UnderlyingType); } } @@ -510,7 +510,7 @@ private static DataContract CreateGetOnlyCollectionDataContract(int id, RuntimeT DataContract? dataContract = null; lock (s_createDataContractLock) { - dataContract = s_dataContractCache[id]; + dataContract = s_dataContractCache.GetItem(id); if (dataContract == null) { type ??= Type.GetTypeFromHandle(typeHandle)!; From a4104e5cd9c8c990b914e3eb96ac181e68e3da96 Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Tue, 11 Jul 2023 17:54:36 -0700 Subject: [PATCH 5/5] Refine the context-aware collection types. --- .../Runtime/Serialization/ContextAware.cs | 98 +++++-------------- .../Runtime/Serialization/DataContract.cs | 4 +- 2 files changed, 24 insertions(+), 78 deletions(-) diff --git a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/ContextAware.cs b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/ContextAware.cs index 3d0369cf0c566c..8134c5c3f82c70 100644 --- a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/ContextAware.cs +++ b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/ContextAware.cs @@ -7,56 +7,57 @@ using System.Reflection; using System.Runtime.CompilerServices; using System.Runtime.Loader; +using System.Runtime.Serialization.DataContracts; namespace System.Runtime.Serialization { - internal sealed class ContextAwareIndex<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] T> where T : class? + internal sealed class ContextAwareDataContractIndex { - private (T? strong, WeakReference? weak)[] _items; - private ConditionalWeakTable _keepAlive; + private (DataContract? strong, WeakReference? weak)[] _contracts; + private ConditionalWeakTable _keepAlive; - public int Length => _items.Length; + public int Length => _contracts.Length; - public ContextAwareIndex(int size) + public ContextAwareDataContractIndex(int size) { - _items = new (T?, WeakReference?)[size]; - _keepAlive = new ConditionalWeakTable(); + _contracts = new (DataContract?, WeakReference?)[size]; + _keepAlive = new ConditionalWeakTable(); } - public T? GetItem(int index) => _items[index].strong ?? (_items[index].weak?.TryGetTarget(out T? ret) == true ? ret : null); + public DataContract? GetItem(int index) => _contracts[index].strong ?? (_contracts[index].weak?.TryGetTarget(out DataContract? ret) == true ? ret : null); - public void SetItem(int index, T value, Type keepAliveType) + public void SetItem(int index, DataContract dataContract) { // Check for unloadability to decide how to store the value - AssemblyLoadContext? alc = AssemblyLoadContext.GetLoadContext(keepAliveType.Assembly); + AssemblyLoadContext? alc = AssemblyLoadContext.GetLoadContext(dataContract.UnderlyingType.Assembly); if (alc == null || !alc.IsCollectible) { - _items[index].strong = value; + _contracts[index].strong = dataContract; } else { - _items[index].weak = new WeakReference(value); - _keepAlive.Add(keepAliveType, value); + _contracts[index].weak = new WeakReference(dataContract); + _keepAlive.Add(dataContract.UnderlyingType, dataContract); } } public void Resize(int newSize) { - Array.Resize<(T?, WeakReference?)>(ref _items, newSize); + Array.Resize<(DataContract?, WeakReference?)>(ref _contracts, newSize); } } - internal sealed class ContextAwareDictionary - where TType : Type - where T : class? + internal sealed class ContextAwareDictionary + where TKey : Type + where TValue : class? { - private readonly ConcurrentDictionary _fastDictionary = new(); - private readonly ConditionalWeakTable _collectibleTable = new(); + private readonly ConcurrentDictionary _fastDictionary = new(); + private readonly ConditionalWeakTable _collectibleTable = new(); - internal T GetOrAdd(TType t, Func f) + internal TValue GetOrAdd(TKey t, Func f) { - T? ret; + TValue? ret; // The fast and most common default case if (_fastDictionary.TryGetValue(t, out ret)) @@ -91,59 +92,4 @@ internal T GetOrAdd(TType t, Func f) return ret; } } - - //internal sealed class ContextAwareTables<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]T> where T : class? - //{ - // private readonly Hashtable _defaultTable; - // private readonly ConditionalWeakTable _collectibleTable; - - // public ContextAwareTables() - // { - // _defaultTable = new Hashtable(); - // _collectibleTable = new ConditionalWeakTable(); - // } - - // internal T GetOrCreateValue(Type t, Func f) - // { - // // The fast and most common default case - // T? ret = (T?)_defaultTable[t]; - // if (ret != null) - // return ret; - - // // Common case for collectible contexts - // if (_collectibleTable.TryGetValue(t, out ret)) - // return ret; - - // // Not found. Do the slower work of creating the value in the correct collection. - // AssemblyLoadContext? alc = AssemblyLoadContext.GetLoadContext(t.Assembly); - - // // Null and non-collectible load contexts use the default table - // if (alc == null || !alc.IsCollectible) - // { - // lock (_defaultTable) - // { - // if ((ret = (T?)_defaultTable[t]) == null) - // { - // ret = f(t); - // _defaultTable[t] = ret; - // } - // } - // } - - // // Collectible load contexts should use the ConditionalWeakTable so they can be unloaded - // else - // { - // lock (_collectibleTable) - // { - // if (!_collectibleTable.TryGetValue(t, out ret)) - // { - // ret = f(t); - // _collectibleTable.AddOrUpdate(t, ret); - // } - // } - // } - - // return ret; - // } - //} } diff --git a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/DataContract.cs b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/DataContract.cs index e6b80eb4fbaf88..1028c02ff4c1a0 100644 --- a/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/DataContract.cs +++ b/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/DataContract.cs @@ -299,7 +299,7 @@ internal virtual bool IsValidContract() internal class DataContractCriticalHelper { private static readonly ConcurrentDictionary s_typeToIDCache = new(); - private static readonly ContextAwareIndex s_dataContractCache = new(32); + private static readonly ContextAwareDataContractIndex s_dataContractCache = new(32); private static int s_dataContractID; private static readonly ContextAwareDictionary s_typeToBuiltInContract = new(); private static Dictionary? s_nameToBuiltInContract; @@ -499,7 +499,7 @@ private static void AssignDataContractToId(DataContract dataContract, int id) { lock (s_cacheLock) { - s_dataContractCache.SetItem(id, dataContract, dataContract.UnderlyingType); + s_dataContractCache.SetItem(id, dataContract); } }