Skip to content

Commit 7e0f0b7

Browse files
buyaa-njkotas
buyaa-n
and
jkotas
authored
Do not check object references for equality after hot reload (#73009)
* Compare type instead of module, check MetadataUpdater.IsSupported in HashCode * Perf tweaks for Equals and GetHashCode - No need to use GetHashCodeOfPtr when we are wrapping the hashcode using HashCode.Combine that takes care of the randomization - Use underlying type handle for type hashcodes. It is faster than the default object hashcode. - Delete special casing for MetadataUpdater.IsSupported from RuntimeMethodInfo.GetHashCode. It is actually a de-optimization for RuntimeMethodInfo with the two changes above, and about neutral for the rest. - Avoid virtual call for ReflectedType comparison - Restore comment for RuntimeMethodInfo Co-authored-by: Jan Kotas <jkotas@microsoft.com>
1 parent 9865cc7 commit 7e0f0b7

File tree

9 files changed

+172
-75
lines changed

9 files changed

+172
-75
lines changed

‎src/coreclr/System.Private.CoreLib/src/System/Reflection/MdFieldInfo.cs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System.Diagnostics;
55
using System.Globalization;
6+
using System.Reflection.Metadata;
67
using RuntimeTypeCache = System.RuntimeType.RuntimeTypeCache;
78

89
namespace System.Reflection
@@ -33,8 +34,7 @@ internal override bool CacheEquals(object? o)
3334
return
3435
o is MdFieldInfo m &&
3536
m.m_tkField == m_tkField &&
36-
m_declaringType.TypeHandle.GetModuleHandle().Equals(
37-
m.m_declaringType.TypeHandle.GetModuleHandle());
37+
ReferenceEquals(m_declaringType, m.m_declaringType);
3838
}
3939
#endregion
4040

@@ -43,6 +43,13 @@ o is MdFieldInfo m &&
4343

4444
public override int MetadataToken => m_tkField;
4545
internal override RuntimeModule GetRuntimeModule() { return m_declaringType.GetRuntimeModule(); }
46+
47+
public override bool Equals(object? obj) =>
48+
ReferenceEquals(this, obj) ||
49+
(MetadataUpdater.IsSupported && CacheEquals(obj));
50+
51+
public override int GetHashCode() =>
52+
HashCode.Combine(m_tkField.GetHashCode(), m_declaringType.GetUnderlyingNativeHandle().GetHashCode());
4653
#endregion
4754

4855
#region FieldInfo Overrides

‎src/coreclr/System.Private.CoreLib/src/System/Reflection/RtFieldInfo.cs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System.Diagnostics;
55
using System.Globalization;
6+
using System.Reflection.Metadata;
67
using System.Runtime.CompilerServices;
78
using RuntimeTypeCache = System.RuntimeType.RuntimeTypeCache;
89

@@ -183,6 +184,13 @@ internal override RuntimeModule GetRuntimeModule()
183184
return RuntimeTypeHandle.GetModule(RuntimeFieldHandle.GetApproxDeclaringType(this));
184185
}
185186

187+
public override bool Equals(object? obj) =>
188+
ReferenceEquals(this, obj) ||
189+
(MetadataUpdater.IsSupported && CacheEquals(obj));
190+
191+
public override int GetHashCode() =>
192+
HashCode.Combine(m_fieldHandle.GetHashCode(), m_declaringType.GetUnderlyingNativeHandle().GetHashCode());
193+
186194
#endregion
187195

188196
#region FieldInfo Overrides

‎src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.CoreCLR.cs‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Diagnostics;
66
using System.Diagnostics.CodeAnalysis;
77
using System.Globalization;
8+
using System.Reflection.Metadata;
89
using System.Runtime.CompilerServices;
910
using System.Text;
1011
using System.Threading;
@@ -72,7 +73,8 @@ internal RuntimeConstructorInfo(
7273
RuntimeMethodHandleInternal IRuntimeMethodInfo.Value => new RuntimeMethodHandleInternal(m_handle);
7374

7475
internal override bool CacheEquals(object? o) =>
75-
o is RuntimeConstructorInfo m && m.m_handle == m_handle;
76+
o is RuntimeConstructorInfo m && m.m_handle == m_handle &&
77+
ReferenceEquals(m_declaringType, m.m_declaringType);
7678

7779
internal Signature Signature
7880
{
@@ -118,6 +120,13 @@ public override string ToString()
118120

119121
return m_toString;
120122
}
123+
124+
public override bool Equals(object? obj) =>
125+
ReferenceEquals(this, obj) ||
126+
(MetadataUpdater.IsSupported && CacheEquals(obj));
127+
128+
public override int GetHashCode() =>
129+
HashCode.Combine(m_handle.GetHashCode(), m_declaringType.GetUnderlyingNativeHandle().GetHashCode());
121130
#endregion
122131

123132
#region ICustomAttributeProvider

‎src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeEventInfo.cs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System.Collections.Generic;
55
using System.Diagnostics;
6+
using System.Reflection.Metadata;
67
using RuntimeTypeCache = System.RuntimeType.RuntimeTypeCache;
78

89
namespace System.Reflection
@@ -53,8 +54,7 @@ internal override bool CacheEquals(object? o)
5354
return
5455
o is RuntimeEventInfo m &&
5556
m.m_token == m_token &&
56-
RuntimeTypeHandle.GetModule(m_declaringType).Equals(
57-
RuntimeTypeHandle.GetModule(m.m_declaringType));
57+
ReferenceEquals(m_declaringType, m.m_declaringType);
5858
}
5959

6060
internal BindingFlags BindingFlags => m_bindingFlags;
@@ -68,6 +68,13 @@ public override string ToString()
6868

6969
return m_addMethod.GetParametersNoCopy()[0].ParameterType.FormatTypeName() + " " + Name;
7070
}
71+
72+
public override bool Equals(object? obj) =>
73+
ReferenceEquals(this, obj) ||
74+
(MetadataUpdater.IsSupported && CacheEquals(obj));
75+
76+
public override int GetHashCode() =>
77+
HashCode.Combine(m_token.GetHashCode(), m_declaringType.GetUnderlyingNativeHandle().GetHashCode());
7178
#endregion
7279

7380
#region ICustomAttributeProvider

‎src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeFieldInfo.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ internal RuntimeType GetDeclaringTypeInternal()
4646

4747
public override Module Module => GetRuntimeModule();
4848
public override bool IsCollectible => m_declaringType.IsCollectible;
49+
4950
#endregion
5051

5152
#region Object Overrides

‎src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.CoreCLR.cs‎

Lines changed: 9 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -157,56 +157,18 @@ public override string ToString()
157157
return m_toString;
158158
}
159159

160-
public override int GetHashCode()
161-
{
162-
// See RuntimeMethodInfo.Equals() below.
163-
if (IsGenericMethod)
164-
return RuntimeHelpers.GetHashCodeOfPtr(m_handle);
165-
else
166-
return base.GetHashCode();
167-
}
168-
169-
public override bool Equals(object? obj)
170-
{
171-
if (!IsGenericMethod)
172-
return obj == (object)this;
173-
174-
// We cannot do simple object identity comparisons for generic methods.
175-
// Equals will be called in CerHashTable when RuntimeType+RuntimeTypeCache.GetGenericMethodInfo()
176-
// retrieve items from and insert items into s_methodInstantiations which is a CerHashtable.
177-
178-
RuntimeMethodInfo? mi = obj as RuntimeMethodInfo;
179-
180-
if (mi == null || !mi.IsGenericMethod)
181-
return false;
182-
183-
// now we know that both operands are generic methods
184-
185-
IRuntimeMethodInfo handle1 = RuntimeMethodHandle.StripMethodInstantiation(this);
186-
IRuntimeMethodInfo handle2 = RuntimeMethodHandle.StripMethodInstantiation(mi);
187-
if (handle1.Value.Value != handle2.Value.Value)
188-
return false;
189-
190-
Type[] lhs = GetGenericArguments();
191-
Type[] rhs = mi.GetGenericArguments();
160+
// We cannot do simple object identity comparisons due to generic methods.
161+
// Equals and GetHashCode will be called in CerHashTable when RuntimeType+RuntimeTypeCache.GetGenericMethodInfo()
162+
// retrieve items from and insert items into s_methodInstantiations.
192163

193-
if (lhs.Length != rhs.Length)
194-
return false;
164+
public override int GetHashCode() =>
165+
HashCode.Combine(m_handle.GetHashCode(), m_declaringType.GetUnderlyingNativeHandle().GetHashCode());
195166

196-
for (int i = 0; i < lhs.Length; i++)
197-
{
198-
if (lhs[i] != rhs[i])
199-
return false;
200-
}
167+
public override bool Equals(object? obj) =>
168+
obj is RuntimeMethodInfo m && m_handle == m.m_handle &&
169+
ReferenceEquals(m_declaringType, m.m_declaringType) &&
170+
ReferenceEquals(m_reflectedTypeCache.GetRuntimeType(), m.m_reflectedTypeCache.GetRuntimeType());
201171

202-
if (DeclaringType != mi.DeclaringType)
203-
return false;
204-
205-
if (ReflectedType != mi.ReflectedType)
206-
return false;
207-
208-
return true;
209-
}
210172
#endregion
211173

212174
#region ICustomAttributeProvider

‎src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System.Collections.Generic;
55
using System.Diagnostics;
66
using System.Globalization;
7+
using System.Reflection.Metadata;
78
using System.Text;
89
using RuntimeTypeCache = System.RuntimeType.RuntimeTypeCache;
910

@@ -55,8 +56,7 @@ internal override bool CacheEquals(object? o)
5556
return
5657
o is RuntimePropertyInfo m &&
5758
m.m_token == m_token &&
58-
RuntimeTypeHandle.GetModule(m_declaringType).Equals(
59-
RuntimeTypeHandle.GetModule(m.m_declaringType));
59+
ReferenceEquals(m_declaringType, m.m_declaringType);
6060
}
6161

6262
internal Signature Signature
@@ -179,6 +179,13 @@ public override IList<CustomAttributeData> GetCustomAttributesData()
179179
public override Module Module => GetRuntimeModule();
180180
internal RuntimeModule GetRuntimeModule() { return m_declaringType.GetRuntimeModule(); }
181181
public override bool IsCollectible => m_declaringType.IsCollectible;
182+
183+
public override bool Equals(object? obj) =>
184+
ReferenceEquals(this, obj) ||
185+
(MetadataUpdater.IsSupported && CacheEquals(obj));
186+
187+
public override int GetHashCode() =>
188+
HashCode.Combine(m_token.GetHashCode(), m_declaringType.GetUnderlyingNativeHandle().GetHashCode());
182189
#endregion
183190

184191
#region PropertyInfo Overrides

‎src/libraries/System.Runtime/tests/System/DelegateTests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,7 @@ public static void SameGenericMethodObtainedViaDelegateAndReflectionAreSameForCl
458458
var m1 = ((MethodCallExpression)((Expression<Action>)(() => new ClassG().M<string, object>())).Body).Method;
459459
var m2 = new Action(new ClassG().M<string, object>).Method;
460460
Assert.True(m1.Equals(m2));
461-
Assert.True(m1.GetHashCode().Equals(m2.GetHashCode()));
461+
Assert.Equal(m1.GetHashCode(), m2.GetHashCode());
462462
Assert.Equal(m1.MethodHandle.Value, m2.MethodHandle.Value);
463463
}
464464

@@ -468,7 +468,7 @@ public static void SameGenericMethodObtainedViaDelegateAndReflectionAreSameForSt
468468
var m1 = ((MethodCallExpression)((Expression<Action>)(() => new StructG().M<string, object>())).Body).Method;
469469
var m2 = new Action(new StructG().M<string, object>).Method;
470470
Assert.True(m1.Equals(m2));
471-
Assert.True(m1.GetHashCode().Equals(m2.GetHashCode()));
471+
Assert.Equal(m1.GetHashCode(), m2.GetHashCode());
472472
Assert.Equal(m1.MethodHandle.Value, m2.MethodHandle.Value);
473473
}
474474

0 commit comments

Comments
 (0)