From 33807463cfe7f3e91ec5a93b93d35ae3141a7024 Mon Sep 17 00:00:00 2001 From: "mr.slow" Date: Wed, 22 Dec 2021 10:42:48 +0200 Subject: [PATCH 1/9] Compile function returns what was compiled --- Expressmapper.Shared/MappingServiceProvider.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Expressmapper.Shared/MappingServiceProvider.cs b/Expressmapper.Shared/MappingServiceProvider.cs index 21eaa89..bb2284a 100644 --- a/Expressmapper.Shared/MappingServiceProvider.cs +++ b/Expressmapper.Shared/MappingServiceProvider.cs @@ -512,7 +512,7 @@ private void CompileNonGenericCollectionMapping(Type srcType, Type dstType) action(); } - private void CompileNonGenericCustomTypeMapper(Type srcType, Type dstType, ICustomTypeMapper typeMapper, long cacheKey) + private Func CompileNonGenericCustomTypeMapper(Type srcType, Type dstType, ICustomTypeMapper typeMapper, long cacheKey) { var sourceExpression = Expression.Parameter(typeof(object), "src"); var destinationExpression = Expression.Parameter(typeof(object), "dst"); @@ -555,7 +555,11 @@ private void CompileNonGenericCustomTypeMapper(Type srcType, Type dstType, ICust srcAssigned, dstAssigned, assignExp, assignContextExp, sourceAssignedExp, destAssignedExp, /*destinationAssignedExp,*/ resultAssignExp, resultVarExp); var lambda = Expression.Lambda>(blockExpression, sourceExpression, destinationExpression); - _customTypeMapperCache[cacheKey] = lambda.Compile(); + var result = lambda.Compile(); + + _customTypeMapperCache[cacheKey] = result; + + return result; } internal static Type GetCollectionElementType(Type type) From 12110ca07fb04ad43df0ad0b572533fa02b75aca Mon Sep 17 00:00:00 2001 From: "mr.slow" Date: Wed, 22 Dec 2021 10:44:43 +0200 Subject: [PATCH 2/9] Turn method to pure --- Expressmapper.Shared/MappingServiceProvider.cs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/Expressmapper.Shared/MappingServiceProvider.cs b/Expressmapper.Shared/MappingServiceProvider.cs index bb2284a..762e729 100644 --- a/Expressmapper.Shared/MappingServiceProvider.cs +++ b/Expressmapper.Shared/MappingServiceProvider.cs @@ -1,6 +1,7 @@ using System; using System.Collections; using System.Collections.Generic; +using System.Diagnostics.Contracts; using System.Linq; using System.Linq.Expressions; using System.Reflection; @@ -392,11 +393,14 @@ private object MapNonGenericInternal(Type srcType, Type dstType, object src, obj { var customTypeMapper = CustomMappers[cacheKey]; var typeMapper = customTypeMapper(); - if (!_customTypeMapperCache.ContainsKey(cacheKey)) + var exists = _customTypeMapperCache.TryGetValue(cacheKey, out var materializer); + if (!exists) { - CompileNonGenericCustomTypeMapper(srcType, dstType, typeMapper, cacheKey); + materializer = CompileNonGenericCustomTypeMapper(srcType, dstType, typeMapper); + _customTypeMapperCache[cacheKey] = materializer; } - return _customTypeMapperCache[cacheKey](src, dest); + + return materializer(src, dest); } ITypeMapper mapper = null; @@ -512,7 +516,8 @@ private void CompileNonGenericCollectionMapping(Type srcType, Type dstType) action(); } - private Func CompileNonGenericCustomTypeMapper(Type srcType, Type dstType, ICustomTypeMapper typeMapper, long cacheKey) + [Pure] + private Func CompileNonGenericCustomTypeMapper(Type srcType, Type dstType, ICustomTypeMapper typeMapper) { var sourceExpression = Expression.Parameter(typeof(object), "src"); var destinationExpression = Expression.Parameter(typeof(object), "dst"); @@ -557,8 +562,6 @@ private Func CompileNonGenericCustomTypeMapper(Type srcT var lambda = Expression.Lambda>(blockExpression, sourceExpression, destinationExpression); var result = lambda.Compile(); - _customTypeMapperCache[cacheKey] = result; - return result; } From 23d2e834501803ec10ad53663e86074bb6f0de13 Mon Sep 17 00:00:00 2001 From: "mr.slow" Date: Wed, 22 Dec 2021 10:58:36 +0200 Subject: [PATCH 3/9] Use reference-level replacements to bare with concurrent modifications --- .../MappingServiceProvider.cs | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/Expressmapper.Shared/MappingServiceProvider.cs b/Expressmapper.Shared/MappingServiceProvider.cs index 762e729..5f1a11e 100644 --- a/Expressmapper.Shared/MappingServiceProvider.cs +++ b/Expressmapper.Shared/MappingServiceProvider.cs @@ -5,6 +5,7 @@ using System.Linq; using System.Linq.Expressions; using System.Reflection; +using System.Threading; namespace ExpressMapper { @@ -14,7 +15,7 @@ public sealed class MappingServiceProvider : IMappingServiceProvider public Dictionary> CustomMappers { get; set; } public Dictionary> CustomMappingsBySource { get; set; } - private readonly Dictionary> _customTypeMapperCache = new Dictionary>(); + private volatile Dictionary> _customTypeMapperCache = new Dictionary>(); private readonly List _nonGenericCollectionMappingCache = new List(); private static readonly Type GenericEnumerableType = typeof(IEnumerable<>); @@ -397,7 +398,24 @@ private object MapNonGenericInternal(Type srcType, Type dstType, object src, obj if (!exists) { materializer = CompileNonGenericCustomTypeMapper(srcType, dstType, typeMapper); - _customTypeMapperCache[cacheKey] = materializer; + bool updateSucceeded = false; + do + { + var snapshot = _customTypeMapperCache; + var candidateCopy = new Dictionary>(snapshot); + if (candidateCopy.TryGetValue(cacheKey, out var alreadySetByAnotherThread)) + { + materializer = alreadySetByAnotherThread; + updateSucceeded = true; + } + else + { + candidateCopy[cacheKey] = materializer; + var original = Interlocked.CompareExchange(ref _customTypeMapperCache, candidateCopy, comparand: snapshot); + updateSucceeded = original == snapshot; // fails if updated by another thread. + } + + } while (updateSucceeded == false); } return materializer(src, dest); From bd3df494b014db4bf8353d0b71eebbb16e0b2e16 Mon Sep 17 00:00:00 2001 From: "mr.slow" Date: Wed, 22 Dec 2021 11:34:00 +0200 Subject: [PATCH 4/9] Add explicit value for Property --- Expressmapper.Shared/MappingServiceProvider.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Expressmapper.Shared/MappingServiceProvider.cs b/Expressmapper.Shared/MappingServiceProvider.cs index 5f1a11e..8fe6e9a 100644 --- a/Expressmapper.Shared/MappingServiceProvider.cs +++ b/Expressmapper.Shared/MappingServiceProvider.cs @@ -13,7 +13,9 @@ public sealed class MappingServiceProvider : IMappingServiceProvider { private readonly object _lock = new object(); - public Dictionary> CustomMappers { get; set; } + private Dictionary> _customMappers; + public Dictionary> CustomMappers { get => _customMappers; set => _customMappers = value; } + public Dictionary> CustomMappingsBySource { get; set; } private volatile Dictionary> _customTypeMapperCache = new Dictionary>(); private readonly List _nonGenericCollectionMappingCache = new List(); @@ -39,7 +41,7 @@ public MappingServiceProvider() new SourceMappingService(this), new DestinationMappingService(this) }; - CustomMappers = new Dictionary>(); + _customMappers = new Dictionary>(); CustomMappingsBySource = new Dictionary>(); } From ffa7386bff010dd3baec9feca1f9c3a1319e9f1e Mon Sep 17 00:00:00 2001 From: "mr.slow" Date: Wed, 22 Dec 2021 11:34:27 +0200 Subject: [PATCH 5/9] Extract method to add to dictionary in thread-safe manner --- .../MappingServiceProvider.cs | 45 ++++++++++--------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/Expressmapper.Shared/MappingServiceProvider.cs b/Expressmapper.Shared/MappingServiceProvider.cs index 8fe6e9a..f44eaa0 100644 --- a/Expressmapper.Shared/MappingServiceProvider.cs +++ b/Expressmapper.Shared/MappingServiceProvider.cs @@ -252,8 +252,7 @@ public void RegisterCustom(Func mapFunc) delegateMapperType.GetInfo().GetConstructor(new Type[] { typeof(Func<,>).MakeGenericType(src, dest) }), Expression.Constant(mapFunc)); var newLambda = Expression.Lambda>>(newExpression); - var compile = newLambda.Compile(); - CustomMappers.Add(cacheKey, compile); + AddToDictionary(ref _customMappers, cacheKey, newLambda.Compile()); } } @@ -400,24 +399,7 @@ private object MapNonGenericInternal(Type srcType, Type dstType, object src, obj if (!exists) { materializer = CompileNonGenericCustomTypeMapper(srcType, dstType, typeMapper); - bool updateSucceeded = false; - do - { - var snapshot = _customTypeMapperCache; - var candidateCopy = new Dictionary>(snapshot); - if (candidateCopy.TryGetValue(cacheKey, out var alreadySetByAnotherThread)) - { - materializer = alreadySetByAnotherThread; - updateSucceeded = true; - } - else - { - candidateCopy[cacheKey] = materializer; - var original = Interlocked.CompareExchange(ref _customTypeMapperCache, candidateCopy, comparand: snapshot); - updateSucceeded = original == snapshot; // fails if updated by another thread. - } - - } while (updateSucceeded == false); + materializer = AddToDictionary(ref _customTypeMapperCache, cacheKey, materializer); } return materializer(src, dest); @@ -599,5 +581,28 @@ public long CalculateCacheKey(Type source, Type dest) } #endregion + + private static TValue AddToDictionary(ref Dictionary dictionary, long cacheKey, TValue value) + { + bool added = false; + do + { + var snapshot = dictionary; + var candidateCopy = new Dictionary(snapshot); + if (candidateCopy.TryGetValue(cacheKey, out var alreadySetByAnotherThread)) + { + return alreadySetByAnotherThread; + } + else + { + candidateCopy.Add(cacheKey, value); + var original = Interlocked.CompareExchange(ref dictionary, candidateCopy, comparand: snapshot); + added = original == snapshot; // fails if updated by another thread. + } + } + while (added == false); + + return value; + } } } \ No newline at end of file From e0515033e43769bb290b21198214dc3806da8d3b Mon Sep 17 00:00:00 2001 From: "mr.slow" Date: Wed, 22 Dec 2021 11:36:17 +0200 Subject: [PATCH 6/9] Safe insert via reference change --- Expressmapper.Shared/MappingServiceProvider.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Expressmapper.Shared/MappingServiceProvider.cs b/Expressmapper.Shared/MappingServiceProvider.cs index f44eaa0..6505d28 100644 --- a/Expressmapper.Shared/MappingServiceProvider.cs +++ b/Expressmapper.Shared/MappingServiceProvider.cs @@ -272,8 +272,7 @@ public void RegisterCustom() where TMapper : ICustomTypeMapper>>(newExpression); - var compile = newLambda.Compile(); - CustomMappers[cacheKey] = compile; + AddToDictionary(ref _customMappers, cacheKey, newLambda.Compile()); } } From 40544eb27fce390dc66e1790ff351aca021a95cc Mon Sep 17 00:00:00 2001 From: "mr.slow" Date: Wed, 22 Dec 2021 11:36:53 +0200 Subject: [PATCH 7/9] Volatile does not help much --- Expressmapper.Shared/MappingServiceProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Expressmapper.Shared/MappingServiceProvider.cs b/Expressmapper.Shared/MappingServiceProvider.cs index 6505d28..d61a8a5 100644 --- a/Expressmapper.Shared/MappingServiceProvider.cs +++ b/Expressmapper.Shared/MappingServiceProvider.cs @@ -17,7 +17,7 @@ public sealed class MappingServiceProvider : IMappingServiceProvider public Dictionary> CustomMappers { get => _customMappers; set => _customMappers = value; } public Dictionary> CustomMappingsBySource { get; set; } - private volatile Dictionary> _customTypeMapperCache = new Dictionary>(); + private Dictionary> _customTypeMapperCache = new Dictionary>(); private readonly List _nonGenericCollectionMappingCache = new List(); private static readonly Type GenericEnumerableType = typeof(IEnumerable<>); From 8244077d0877edc8660165674a584590a224bb54 Mon Sep 17 00:00:00 2001 From: "mr.slow" Date: Wed, 22 Dec 2021 11:41:31 +0200 Subject: [PATCH 8/9] Avoid double dictionary lookups --- Expressmapper.Shared/MappingServiceProvider.cs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/Expressmapper.Shared/MappingServiceProvider.cs b/Expressmapper.Shared/MappingServiceProvider.cs index d61a8a5..6589c6b 100644 --- a/Expressmapper.Shared/MappingServiceProvider.cs +++ b/Expressmapper.Shared/MappingServiceProvider.cs @@ -300,9 +300,8 @@ public TN Map(T src, TN dest) var destType = typeof(TN); var cacheKey = CalculateCacheKey(srcType, destType); - if (CustomMappers.ContainsKey(cacheKey)) + if (CustomMappers.TryGetValue(cacheKey, out var customTypeMapper)) { - var customTypeMapper = CustomMappers[cacheKey]; var typeMapper = customTypeMapper() as ICustomTypeMapper; var context = new DefaultMappingContext { Source = src, Destination = dest }; return typeMapper.Map(context); @@ -390,9 +389,8 @@ private object MapNonGenericInternal(Type srcType, Type dstType, object src, obj } var cacheKey = CalculateCacheKey(srcType, dstType); - if (CustomMappers.ContainsKey(cacheKey)) + if (CustomMappers.TryGetValue(cacheKey, out var customTypeMapper)) { - var customTypeMapper = CustomMappers[cacheKey]; var typeMapper = customTypeMapper(); var exists = _customTypeMapperCache.TryGetValue(cacheKey, out var materializer); if (!exists) @@ -429,9 +427,8 @@ private object MapNonGenericInternal(Type srcType, Type dstType, object src, obj } else { - if (CustomMappingsBySource.ContainsKey(srcHash)) + if (CustomMappingsBySource.TryGetValue(srcHash, out var mappings)) { - var mappings = CustomMappingsBySource[srcHash]; var typeMappers = mappings.Where(m => SourceService.TypeMappers.ContainsKey(m)) .Select(m => SourceService.TypeMappers[m]) @@ -460,9 +457,8 @@ private object MapNonGenericInternal(Type srcType, Type dstType, object src, obj return nonGenericMapFunc(src, dest); } - if (mappingService.TypeMappers.ContainsKey(cacheKey)) + if (mappingService.TypeMappers.TryGetValue(cacheKey, out mapper)) { - mapper = mappingService.TypeMappers[cacheKey]; var nonGenericMapFunc = mapper.GetNonGenericMapFunc(); return nonGenericMapFunc(src, dest); } From bdbcef20f2d29f3c79ff0be15a7bebbc499ebc95 Mon Sep 17 00:00:00 2001 From: "mr.slow" Date: Wed, 22 Dec 2021 11:56:25 +0200 Subject: [PATCH 9/9] Avoid double lookup as wekk --- Expressmapper.Shared/MappingServiceProvider.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Expressmapper.Shared/MappingServiceProvider.cs b/Expressmapper.Shared/MappingServiceProvider.cs index 6589c6b..6139727 100644 --- a/Expressmapper.Shared/MappingServiceProvider.cs +++ b/Expressmapper.Shared/MappingServiceProvider.cs @@ -415,10 +415,8 @@ private object MapNonGenericInternal(Type srcType, Type dstType, object src, obj if (dstType != actualDstType && actualDstType.GetInfo().IsAssignableFrom(dstType)) throw new InvalidCastException($"Your destination object instance type '{actualSrcType.FullName}' is not assignable from destination type you specified '{srcType}'."); - if (CustomMappingsBySource.ContainsKey(srcHash)) + if (CustomMappingsBySource.TryGetValue(srcHash, out var mappings)) { - var mappings = CustomMappingsBySource[srcHash]; - mapper = mappings.Where(m => DestinationService.TypeMappers.ContainsKey(m)) .Select(m => DestinationService.TypeMappers[m])