Skip to content

Commit 970e9fa

Browse files
hazzikhazzik
authored
Fix ProxyFactory cache (#1454)
* Fix ProxyFactory cache - use a correct implementation of cache - reduce the number of allocations of ProxyCacheEntry - reduce memory flow
1 parent 196a1c4 commit 970e9fa

File tree

9 files changed

+93
-141
lines changed

9 files changed

+93
-141
lines changed

‎src/NHibernate.Test/NHSpecificTest/NH3954/EqualsFixture.cs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System;
22
using System.Reflection;
3+
using System.Runtime.Serialization;
34
using NHibernate.Proxy;
45
using NHibernate.Proxy.DynamicProxy;
56
using NHibernate.Type;
@@ -48,9 +49,9 @@ public void TypeInequality()
4849
[Test]
4950
public void InterfaceEquality()
5051
{
51-
var entry1 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(INHibernateProxy), typeof(IProxy) });
52+
var entry1 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(INHibernateProxy), typeof(ISerializable) });
5253
// Interfaces order inverted on purpose: must be supported.
53-
var entry2 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(IProxy), typeof(INHibernateProxy) });
54+
var entry2 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(ISerializable), typeof(INHibernateProxy) });
5455
Assert.IsTrue(entry1.Equals(entry2));
5556
Assert.IsTrue(entry2.Equals(entry1));
5657
}
@@ -59,21 +60,21 @@ public void InterfaceEquality()
5960
[Test]
6061
public void InterfaceEqualityWithLotOfUnordererdAndDupInterfaces()
6162
{
62-
var entry1 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(INHibernateProxy), typeof(IProxy), typeof(IType), typeof(IDisposable), typeof(IFilter) });
63+
var entry1 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(INHibernateProxy), typeof(ISerializable), typeof(IType), typeof(IDisposable), typeof(IFilter) });
6364
// Interfaces order inverted and duplicated on purpose: must be supported.
64-
var entry2 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(IType), typeof(IProxy), typeof(IFilter), typeof(IDisposable), typeof(IType), typeof(IFilter), typeof(INHibernateProxy) });
65+
var entry2 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(IType), typeof(ISerializable), typeof(IFilter), typeof(IDisposable), typeof(IType), typeof(IFilter), typeof(INHibernateProxy) });
6566
Assert.IsTrue(entry1.Equals(entry2));
6667
Assert.IsTrue(entry2.Equals(entry1));
6768
}
6869

6970
[Test]
7071
public void InterfaceInequality()
7172
{
72-
var entry1 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(INHibernateProxy), typeof(IProxy) });
73-
var entry2 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(IProxy) });
73+
var entry1 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(INHibernateProxy), typeof(ISerializable) });
74+
var entry2 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(ISerializable) });
7475
TweakEntry(entry2, entry1.GetHashCode());
7576
Assert.IsFalse(entry1.Equals(entry2));
7677
Assert.IsFalse(entry2.Equals(entry1));
7778
}
7879
}
79-
}
80+
}

‎src/NHibernate.Test/NHSpecificTest/NH3954/ProxyCacheFixture.cs

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,36 @@
1-
using System.Collections.Concurrent;
1+
using System;
2+
using System.Collections.Concurrent;
23
using System.Reflection;
4+
using System.Runtime.Serialization;
35
using NHibernate.Proxy;
46
using NHibernate.Proxy.DynamicProxy;
57
using NUnit.Framework;
68

79
namespace NHibernate.Test.NHSpecificTest.NH3954
810
{
9-
[TestFixture, Explicit("Demonstrates bug impact on cache, but which tests will fail is a bit unpredictable")]
11+
[TestFixture, Explicit("Demonstrates bug impact on cache, but which tests will fail is a bit unpredictable"), Obsolete]
1012
public class ProxyCacheFixture
1113
{
1214
private ProxyCache _cache;
13-
private ConcurrentDictionary<ProxyCacheEntry, System.Type> _internalCache;
15+
private ConcurrentDictionary<ProxyCacheEntry, TypeInfo> _internalCache;
1416
private int _hashCode1;
1517
private int _hashCode2;
1618

1719
private static readonly FieldInfo InternalCacheField =
18-
typeof(ProxyCache).GetField("cache", BindingFlags.Static | BindingFlags.NonPublic);
20+
typeof(ProxyFactory).GetField("_cache", BindingFlags.Static | BindingFlags.NonPublic);
1921

2022
[SetUp]
2123
public void SetUp()
2224
{
2325
_cache = new ProxyCache();
2426

25-
_internalCache = (ConcurrentDictionary<ProxyCacheEntry, System.Type>)InternalCacheField.GetValue(null);
27+
_internalCache = (ConcurrentDictionary<ProxyCacheEntry, TypeInfo>)InternalCacheField.GetValue(null);
2628

2729
_cache.StoreProxyType(typeof(Entity1FakeProxy).GetTypeInfo(), typeof(Entity1));
2830
_cache.StoreProxyType(typeof(Entity2FakeProxy).GetTypeInfo(), typeof(Entity2), typeof(INHibernateProxy));
2931
_cache.StoreProxyType(typeof(Entity3FakeProxy).GetTypeInfo(), typeof(Entity3));
30-
_cache.StoreProxyType(typeof(Entity4FakeProxy).GetTypeInfo(), typeof(Entity4), typeof(IProxy));
31-
_cache.StoreProxyType(typeof(Entity5FakeProxy).GetTypeInfo(), typeof(Entity5), typeof(INHibernateProxy), typeof(IProxy));
32+
_cache.StoreProxyType(typeof(Entity4FakeProxy).GetTypeInfo(), typeof(Entity4), typeof(ISerializable));
33+
_cache.StoreProxyType(typeof(Entity5FakeProxy).GetTypeInfo(), typeof(Entity5), typeof(INHibernateProxy), typeof(ISerializable));
3234

3335
// Artificially inject other entries with same hashcodes
3436
_hashCode1 = new ProxyCacheEntry(typeof(Entity1), null).GetHashCode();
@@ -37,14 +39,14 @@ public void SetUp()
3739

3840
_hashCode2 = new ProxyCacheEntry(typeof(Entity2), new[] { typeof(INHibernateProxy) }).GetHashCode();
3941
Inject(new ProxyCacheEntry(typeof(Entity2), null), _hashCode2, typeof(Entity2FakeProxy2));
40-
Inject(new ProxyCacheEntry(typeof(Entity4), new[] { typeof(IProxy) }), _hashCode2, typeof(Entity4FakeProxy2));
41-
Inject(new ProxyCacheEntry(typeof(Entity5), new[] { typeof(INHibernateProxy), typeof(IProxy) }), _hashCode2, typeof(Entity5FakeProxy2));
42+
Inject(new ProxyCacheEntry(typeof(Entity4), new[] { typeof(ISerializable) }), _hashCode2, typeof(Entity4FakeProxy2));
43+
Inject(new ProxyCacheEntry(typeof(Entity5), new[] { typeof(INHibernateProxy), typeof(ISerializable) }), _hashCode2, typeof(Entity5FakeProxy2));
4244
}
4345

4446
private void Inject(ProxyCacheEntry entryToTweak, int hashcode, System.Type result)
4547
{
4648
TweakEntry(entryToTweak, hashcode);
47-
_internalCache[entryToTweak] = result;
49+
_internalCache[entryToTweak] = result.GetTypeInfo();
4850
}
4951

5052
private static readonly FieldInfo HashCodeField =
@@ -112,14 +114,14 @@ public void ProxyCacheEntity3FakeProxy2()
112114
[Test]
113115
public void ProxyCacheEntity4FakeProxy()
114116
{
115-
var result = _cache.GetProxyType(typeof(Entity4), typeof(IProxy));
117+
var result = _cache.GetProxyType(typeof(Entity4), typeof(ISerializable));
116118
Assert.AreEqual(typeof(Entity4FakeProxy), result);
117119
}
118120

119121
[Test]
120122
public void ProxyCacheEntity4FakeProxy2()
121123
{
122-
var entry = new ProxyCacheEntry(typeof(Entity4), new[] { typeof(IProxy) });
124+
var entry = new ProxyCacheEntry(typeof(Entity4), new[] { typeof(ISerializable) });
123125
TweakEntry(entry, _hashCode2);
124126
var result = _internalCache[entry];
125127
Assert.AreEqual(typeof(Entity4FakeProxy2), result);
@@ -128,15 +130,15 @@ public void ProxyCacheEntity4FakeProxy2()
128130
[Test]
129131
public void ProxyCacheEntity5FakeProxy()
130132
{
131-
var result = _cache.GetProxyType(typeof(Entity5), typeof(IProxy), typeof(INHibernateProxy));
133+
var result = _cache.GetProxyType(typeof(Entity5), typeof(ISerializable), typeof(INHibernateProxy));
132134
Assert.AreEqual(typeof(Entity5FakeProxy), result);
133135
}
134136

135137
[Test]
136138
public void ProxyCacheEntity5FakeProxy2()
137139
{
138140
// Interfaces order inverted on purpose: must be supported.
139-
var entry = new ProxyCacheEntry(typeof(Entity5), new[] { typeof(IProxy), typeof(INHibernateProxy) });
141+
var entry = new ProxyCacheEntry(typeof(Entity5), new[] { typeof(ISerializable), typeof(INHibernateProxy) });
140142
TweakEntry(entry, _hashCode2);
141143
var result = _internalCache[entry];
142144
Assert.AreEqual(typeof(Entity5FakeProxy2), result);
@@ -149,7 +151,7 @@ public void ProxyCacheNone()
149151
// (Otherwise the test may starts failing unexpectedly sometimes, as the original bug ...)
150152
// This one was not added in anyway.
151153
TypeInfo result;
152-
Assert.IsFalse(_cache.TryGetProxyType(typeof(Entity2), new[] { typeof(IProxy) }, out result));
154+
Assert.IsFalse(_cache.TryGetProxyType(typeof(Entity2), new[] { typeof(ISerializable) }, out result));
153155
}
154156
}
155-
}
157+
}

‎src/NHibernate/Async/Engine/Cascade.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010

1111
using System.Collections;
12-
12+
using System.Collections.Generic;
1313
using NHibernate.Collection;
1414
using NHibernate.Event;
1515
using NHibernate.Persister.Collection;

‎src/NHibernate/Proxy/DynamicProxy/DefaultMethodEmitter.cs

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,12 @@
1010
using System.Diagnostics;
1111
using System.Reflection;
1212
using System.Reflection.Emit;
13-
using NHibernate.Linq;
1413
using NHibernate.Util;
1514

1615
namespace NHibernate.Proxy.DynamicProxy
1716
{
1817
internal class DefaultMethodEmitter : IMethodBodyEmitter
1918
{
20-
private static readonly MethodInfo getInterceptor;
21-
2219
private static readonly MethodInfo handlerMethod = ReflectHelper.GetMethod<IInterceptor>(
2320
i => i.Intercept(null));
2421
private static readonly MethodInfo getArguments = typeof(InvocationInfo).GetMethod("get_Arguments");
@@ -33,17 +30,8 @@ internal class DefaultMethodEmitter : IMethodBodyEmitter
3330
typeof (object[])
3431
});
3532

36-
private static readonly PropertyInfo interceptorProperty = typeof (IProxy).GetProperty("Interceptor");
37-
38-
private static readonly ConstructorInfo notImplementedConstructor = typeof(NotImplementedException).GetConstructor(new System.Type[0]);
39-
4033
private readonly IArgumentHandler _argumentHandler;
4134

42-
static DefaultMethodEmitter()
43-
{
44-
getInterceptor = interceptorProperty.GetGetMethod();
45-
}
46-
4735
public DefaultMethodEmitter() : this(new DefaultArgumentHandler()) {}
4836

4937
public DefaultMethodEmitter(IArgumentHandler argumentHandler)
@@ -60,12 +48,12 @@ public void EmitMethodBody(MethodBuilder proxyMethod, MethodBuilder callbackMeth
6048
ParameterInfo[] parameters = method.GetParameters();
6149
IL.DeclareLocal(typeof (object[]));
6250
IL.DeclareLocal(typeof (InvocationInfo));
63-
IL.DeclareLocal(typeof(System.Type[]));
51+
IL.DeclareLocal(typeof (System.Type[]));
6452

6553
IL.Emit(OpCodes.Ldarg_0);
66-
IL.Emit(OpCodes.Callvirt, getInterceptor);
54+
IL.Emit(OpCodes.Ldfld, field);
6755

68-
// if (interceptor == null)
56+
// if (this.__interceptor == null)
6957
// return base.method(...);
7058

7159
Label skipBaseCall = IL.DefineLabel();
@@ -90,9 +78,9 @@ public void EmitMethodBody(MethodBuilder proxyMethod, MethodBuilder callbackMeth
9078
IL.Emit(OpCodes.Newobj, infoConstructor);
9179
IL.Emit(OpCodes.Stloc_1);
9280

93-
// this.Interceptor.Intercept(info);
81+
// this.__interceptor.Intercept(info);
9482
IL.Emit(OpCodes.Ldarg_0);
95-
IL.Emit(OpCodes.Callvirt, getInterceptor);
83+
IL.Emit(OpCodes.Ldfld, field);
9684
IL.Emit(OpCodes.Ldloc_1);
9785
IL.Emit(OpCodes.Callvirt, handlerMethod);
9886

‎src/NHibernate/Proxy/DynamicProxy/HashSetExtensions.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
1+
using System;
12
using System.Collections.Generic;
23

34
namespace NHibernate.Proxy.DynamicProxy
45
{
6+
[Obsolete("This class is not used anymore and will be removed in a next major version")]
57
public static class HashSetExtensions
68
{
9+
[Obsolete("This method is not used anymore and will be removed in a next major version")]
710
public static HashSet<T> Merge<T>(this HashSet<T> source, IEnumerable<T> toMerge)
811
{
912
foreach (T item in toMerge)
@@ -13,4 +16,4 @@ public static HashSet<T> Merge<T>(this HashSet<T> source, IEnumerable<T> toMerge
1316
return source;
1417
}
1518
}
16-
}
19+
}

‎src/NHibernate/Proxy/DynamicProxy/IProxyCache.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,16 @@
1111

1212
namespace NHibernate.Proxy.DynamicProxy
1313
{
14+
//Since v5.1
15+
[Obsolete("This interface is not used anymore and will be removed in a next major version")]
1416
public interface IProxyCache
1517
{
1618
bool Contains(System.Type baseType, params System.Type[] baseInterfaces);
19+
1720
TypeInfo GetProxyType(System.Type baseType, params System.Type[] baseInterfaces);
1821

1922
bool TryGetProxyType(System.Type baseType, System.Type[] baseInterfaces, out TypeInfo proxyType);
2023

2124
void StoreProxyType(TypeInfo result, System.Type baseType, params System.Type[] baseInterfaces);
2225
}
23-
}
26+
}

‎src/NHibernate/Proxy/DynamicProxy/ProxyCache.cs

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,32 +6,28 @@
66

77
#endregion
88

9-
using System.Collections.Concurrent;
9+
using System;
1010
using System.Reflection;
1111

1212
namespace NHibernate.Proxy.DynamicProxy
1313
{
14+
//Since v5.1
15+
[Obsolete("This class is not used anymore and will be removed in a next major version")]
1416
public class ProxyCache : IProxyCache
1517
{
16-
private static readonly ConcurrentDictionary<ProxyCacheEntry, TypeInfo> cache = new ConcurrentDictionary<ProxyCacheEntry, TypeInfo>();
17-
18-
#region IProxyCache Members
19-
2018
public bool Contains(System.Type baseType, params System.Type[] baseInterfaces)
2119
{
2220
if (baseType == null)
23-
{
2421
return false;
25-
}
2622

2723
var entry = new ProxyCacheEntry(baseType, baseInterfaces);
28-
return cache.ContainsKey(entry);
24+
return ProxyFactory._cache.ContainsKey(entry);
2925
}
3026

3127
public TypeInfo GetProxyType(System.Type baseType, params System.Type[] baseInterfaces)
3228
{
3329
var entry = new ProxyCacheEntry(baseType, baseInterfaces);
34-
return cache[entry];
30+
return ProxyFactory._cache[entry];
3531
}
3632

3733
public bool TryGetProxyType(System.Type baseType, System.Type[] baseInterfaces, out TypeInfo proxyType)
@@ -42,15 +38,13 @@ public bool TryGetProxyType(System.Type baseType, System.Type[] baseInterfaces,
4238
return false;
4339

4440
var entry = new ProxyCacheEntry(baseType, baseInterfaces);
45-
return cache.TryGetValue(entry, out proxyType);
41+
return ProxyFactory._cache.TryGetValue(entry, out proxyType);
4642
}
4743

4844
public void StoreProxyType(TypeInfo result, System.Type baseType, params System.Type[] baseInterfaces)
4945
{
5046
var entry = new ProxyCacheEntry(baseType, baseInterfaces);
51-
cache[entry] = result;
47+
ProxyFactory._cache[entry] = result;
5248
}
53-
54-
#endregion
5549
}
5650
}

‎src/NHibernate/Proxy/DynamicProxy/ProxyCacheEntry.cs

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,29 +8,32 @@
88

99
using System;
1010
using System.Collections.Generic;
11+
using System.Linq;
1112

1213
namespace NHibernate.Proxy.DynamicProxy
1314
{
1415
public class ProxyCacheEntry : IEquatable<ProxyCacheEntry>
1516
{
1617
private readonly int _hashCode;
18+
private readonly HashSet<System.Type> _uniqueInterfaces;
1719

1820
public ProxyCacheEntry(System.Type baseType, System.Type[] interfaces)
1921
{
20-
if (baseType == null)
21-
throw new ArgumentNullException(nameof(baseType));
22-
BaseType = baseType;
23-
_uniqueInterfaces = new HashSet<System.Type>(interfaces ?? new System.Type[0]);
22+
BaseType = baseType ?? throw new ArgumentNullException(nameof(baseType));
2423

25-
if (_uniqueInterfaces.Count == 0)
24+
var uniqueInterfaces = new HashSet<System.Type>();
25+
if (interfaces != null && interfaces.Length > 0)
2626
{
27-
_hashCode = baseType.GetHashCode();
28-
return;
27+
uniqueInterfaces.UnionWith(interfaces.Where(i => i != null));
2928
}
30-
31-
var allTypes = new List<System.Type>(_uniqueInterfaces) { baseType };
32-
_hashCode = 59;
33-
foreach (System.Type type in allTypes)
29+
_uniqueInterfaces = uniqueInterfaces;
30+
31+
_hashCode = 59 ^ baseType.GetHashCode();
32+
33+
if (_uniqueInterfaces.Count == 0)
34+
return;
35+
36+
foreach (var type in _uniqueInterfaces)
3437
{
3538
// This simple implementation is nonsensitive to list order. If changing it for a sensitive one,
3639
// take care of ordering the list.
@@ -39,14 +42,12 @@ public ProxyCacheEntry(System.Type baseType, System.Type[] interfaces)
3942
}
4043

4144
public System.Type BaseType { get; }
42-
public IReadOnlyCollection<System.Type> Interfaces { get { return _uniqueInterfaces; } }
43-
44-
private HashSet<System.Type> _uniqueInterfaces;
45+
46+
public IReadOnlyCollection<System.Type> Interfaces => _uniqueInterfaces;
4547

4648
public override bool Equals(object obj)
4749
{
48-
var that = obj as ProxyCacheEntry;
49-
return Equals(that);
50+
return Equals(obj as ProxyCacheEntry);
5051
}
5152

5253
public bool Equals(ProxyCacheEntry other)
@@ -64,4 +65,4 @@ public bool Equals(ProxyCacheEntry other)
6465

6566
public override int GetHashCode() => _hashCode;
6667
}
67-
}
68+
}

0 commit comments

Comments
 (0)