Support faster null checks (#21765)
authorBen Adams <thundercat@illyriad.co.uk>
Fri, 4 Jan 2019 19:38:22 +0000 (20:38 +0100)
committerJan Kotas <jkotas@microsoft.com>
Fri, 4 Jan 2019 19:38:21 +0000 (11:38 -0800)
13 files changed:
src/System.Private.CoreLib/shared/System/Globalization/SortVersion.cs
src/System.Private.CoreLib/shared/System/Reflection/Assembly.cs
src/System.Private.CoreLib/shared/System/Reflection/ConstructorInfo.cs
src/System.Private.CoreLib/shared/System/Reflection/EventInfo.cs
src/System.Private.CoreLib/shared/System/Reflection/FieldInfo.cs
src/System.Private.CoreLib/shared/System/Reflection/MemberInfo.cs
src/System.Private.CoreLib/shared/System/Reflection/MethodBase.cs
src/System.Private.CoreLib/shared/System/Reflection/MethodInfo.cs
src/System.Private.CoreLib/shared/System/Reflection/Module.cs
src/System.Private.CoreLib/shared/System/Reflection/PropertyInfo.cs
src/System.Private.CoreLib/shared/System/Version.cs
src/System.Private.CoreLib/src/System/Delegate.cs
src/System.Private.CoreLib/src/System/MulticastDelegate.cs

index 46e9a83..cb0d4c4 100644 (file)
@@ -2,6 +2,8 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Runtime.CompilerServices;
+
 namespace System.Globalization
 {
     [Serializable]
@@ -75,20 +77,19 @@ namespace System.Globalization
             return m_NlsVersion * 7 | m_SortId.GetHashCode();
         }
 
+        // Force inline as the true/false ternary takes it above ALWAYS_INLINE size even though the asm ends up smaller
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static bool operator ==(SortVersion left, SortVersion right)
         {
-            if (((object)left) != null)
-            {
-                return left.Equals(right);
-            }
-
-            if (((object)right) != null)
+            // Test "right" first to allow branch elimination when inlined for null checks (== null)
+            // so it can become a simple test
+            if (right is null)
             {
-                return right.Equals(left);
+                // return true/false not the test result https://github.com/dotnet/coreclr/issues/914
+                return (left is null) ? true : false;
             }
 
-            // Both null.
-            return true;
+            return right.Equals(left);
         }
 
         public static bool operator !=(SortVersion left, SortVersion right)
index 516f81d..c7b39ff 100644 (file)
@@ -8,6 +8,7 @@ using System.Collections.Generic;
 using System.Configuration.Assemblies;
 using System.Runtime.Serialization;
 using System.Security;
+using System.Runtime.CompilerServices;
 
 namespace System.Reflection
 {
@@ -147,15 +148,20 @@ namespace System.Reflection
         public override bool Equals(object o) => base.Equals(o);
         public override int GetHashCode() => base.GetHashCode();
 
+        // Force inline as the true/false ternary takes it above ALWAYS_INLINE size even though the asm ends up smaller
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static bool operator ==(Assembly left, Assembly right)
         {
-            if (object.ReferenceEquals(left, right))
-                return true;
-
-            if ((object)left == null || (object)right == null)
-                return false;
+            // Test "right" first to allow branch elimination when inlined for null checks (== null)
+            // so it can become a simple test
+            if (right is null)
+            {
+                // return true/false not the test result https://github.com/dotnet/coreclr/issues/914
+                return (left is null) ? true : false;
+            }
 
-            return left.Equals(right);
+            // Quick reference equality test prior to calling the virtual Equality
+            return ReferenceEquals(right, left) ? true : right.Equals(left);
         }
 
         public static bool operator !=(Assembly left, Assembly right)
index 3ee1dbf..3d8750c 100644 (file)
@@ -4,6 +4,7 @@
 
 using System.Diagnostics;
 using System.Globalization;
+using System.Runtime.CompilerServices;
 
 namespace System.Reflection
 {
@@ -21,15 +22,20 @@ namespace System.Reflection
         public override bool Equals(object obj) => base.Equals(obj);
         public override int GetHashCode() => base.GetHashCode();
 
+        // Force inline as the true/false ternary takes it above ALWAYS_INLINE size even though the asm ends up smaller
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static bool operator ==(ConstructorInfo left, ConstructorInfo right)
         {
-            if (object.ReferenceEquals(left, right))
-                return true;
-
-            if ((object)left == null || (object)right == null)
-                return false;
-
-            return left.Equals(right);
+            // Test "right" first to allow branch elimination when inlined for null checks (== null)
+            // so it can become a simple test
+            if (right is null)
+            {
+                // return true/false not the test result https://github.com/dotnet/coreclr/issues/914
+                return (left is null) ? true : false;
+            }
+
+            // Quick reference equality test prior to calling the virtual Equality
+            return ReferenceEquals(right, left) ? true : right.Equals(left);
         }
 
         public static bool operator !=(ConstructorInfo left, ConstructorInfo right) => !(left == right);
index ccd9acf..e352613 100644 (file)
@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 using System.Diagnostics;
+using System.Runtime.CompilerServices;
 
 #if FEATURE_COMINTEROP
 using EventRegistrationToken = System.Runtime.InteropServices.WindowsRuntime.EventRegistrationToken;
@@ -99,15 +100,20 @@ namespace System.Reflection
         public override bool Equals(object obj) => base.Equals(obj);
         public override int GetHashCode() => base.GetHashCode();
 
+        // Force inline as the true/false ternary takes it above ALWAYS_INLINE size even though the asm ends up smaller
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static bool operator ==(EventInfo left, EventInfo right)
         {
-            if (object.ReferenceEquals(left, right))
-                return true;
-
-            if ((object)left == null || (object)right == null)
-                return false;
+            // Test "right" first to allow branch elimination when inlined for null checks (== null)
+            // so it can become a simple test
+            if (right is null)
+            {
+                // return true/false not the test result https://github.com/dotnet/coreclr/issues/914
+                return (left is null) ? true : false;
+            }
 
-            return left.Equals(right);
+            // Quick reference equality test prior to calling the virtual Equality
+            return ReferenceEquals(right, left) ? true : right.Equals(left);
         }
 
         public static bool operator !=(EventInfo left, EventInfo right) => !(left == right);
index 0863c2b..53bcf17 100644 (file)
@@ -4,6 +4,7 @@
 
 using System.Diagnostics;
 using System.Globalization;
+using System.Runtime.CompilerServices;
 
 namespace System.Reflection
 {
@@ -39,15 +40,20 @@ namespace System.Reflection
         public override bool Equals(object obj) => base.Equals(obj);
         public override int GetHashCode() => base.GetHashCode();
 
+        // Force inline as the true/false ternary takes it above ALWAYS_INLINE size even though the asm ends up smaller
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static bool operator ==(FieldInfo left, FieldInfo right)
         {
-            if (object.ReferenceEquals(left, right))
-                return true;
-
-            if ((object)left == null || (object)right == null)
-                return false;
-
-            return left.Equals(right);
+            // Test "right" first to allow branch elimination when inlined for null checks (== null)
+            // so it can become a simple test
+            if (right is null)
+            {
+                // return true/false not the test result https://github.com/dotnet/coreclr/issues/914
+                return (left is null) ? true : false;
+            }
+
+            // Quick reference equality test prior to calling the virtual Equality
+            return ReferenceEquals(right, left) ? true : right.Equals(left);
         }
 
         public static bool operator !=(FieldInfo left, FieldInfo right) => !(left == right);
index c61198d..2f439d8 100644 (file)
@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 using System.Collections.Generic;
+using System.Runtime.CompilerServices;
 
 namespace System.Reflection
 {
@@ -44,32 +45,20 @@ namespace System.Reflection
         public override bool Equals(object obj) => base.Equals(obj);
         public override int GetHashCode() => base.GetHashCode();
 
+        // Force inline as the true/false ternary takes it above ALWAYS_INLINE size even though the asm ends up smaller
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static bool operator ==(MemberInfo left, MemberInfo right)
         {
-            if (object.ReferenceEquals(left, right))
-                return true;
-
-            if ((object)left == null || (object)right == null)
-                return false;
-
-            Type type1, type2;
-            MethodBase method1, method2;
-            FieldInfo field1, field2;
-            EventInfo event1, event2;
-            PropertyInfo property1, property2;
-
-            if ((type1 = left as Type) != null && (type2 = right as Type) != null)
-                return type1 == type2;
-            else if ((method1 = left as MethodBase) != null && (method2 = right as MethodBase) != null)
-                return method1 == method2;
-            else if ((field1 = left as FieldInfo) != null && (field2 = right as FieldInfo) != null)
-                return field1 == field2;
-            else if ((event1 = left as EventInfo) != null && (event2 = right as EventInfo) != null)
-                return event1 == event2;
-            else if ((property1 = left as PropertyInfo) != null && (property2 = right as PropertyInfo) != null)
-                return property1 == property2;
+            // Test "right" first to allow branch elimination when inlined for null checks (== null)
+            // so it can become a simple test
+            if (right is null)
+            {
+                // return true/false not the test result https://github.com/dotnet/coreclr/issues/914
+                return (left is null) ? true : false;
+            }
 
-            return false;
+            // Quick reference equality test prior to calling the virtual Equality
+            return ReferenceEquals(right, left) ? true : right.Equals(left);
         }
 
         public static bool operator !=(MemberInfo left, MemberInfo right) => !(left == right);
index 0037c74..e5ca0d5 100644 (file)
@@ -4,6 +4,7 @@
 
 using System.Globalization;
 using System.Diagnostics;
+using System.Runtime.CompilerServices;
 
 namespace System.Reflection
 {
@@ -62,23 +63,20 @@ namespace System.Reflection
         public override bool Equals(object obj) => base.Equals(obj);
         public override int GetHashCode() => base.GetHashCode();
 
+        // Force inline as the true/false ternary takes it above ALWAYS_INLINE size even though the asm ends up smaller
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static bool operator ==(MethodBase left, MethodBase right)
         {
-            if (object.ReferenceEquals(left, right))
-                return true;
-
-            if ((object)left == null || (object)right == null)
-                return false;
-
-            MethodInfo method1, method2;
-            ConstructorInfo constructor1, constructor2;
-
-            if ((method1 = left as MethodInfo) != null && (method2 = right as MethodInfo) != null)
-                return method1 == method2;
-            else if ((constructor1 = left as ConstructorInfo) != null && (constructor2 = right as ConstructorInfo) != null)
-                return constructor1 == constructor2;
+            // Test "right" first to allow branch elimination when inlined for null checks (== null)
+            // so it can become a simple test
+            if (right is null)
+            {
+                // return true/false not the test result https://github.com/dotnet/coreclr/issues/914
+                return (left is null) ? true : false;
+            }
 
-            return false;
+            // Quick reference equality test prior to calling the virtual Equality
+            return ReferenceEquals(right, left) ? true : right.Equals(left);
         }
 
         public static bool operator !=(MethodBase left, MethodBase right) => !(left == right);
index 95c62ba..920a9a2 100644 (file)
@@ -2,6 +2,8 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Runtime.CompilerServices;
+
 namespace System.Reflection
 {
     public abstract partial class MethodInfo : MethodBase
@@ -27,15 +29,20 @@ namespace System.Reflection
         public override bool Equals(object obj) => base.Equals(obj);
         public override int GetHashCode() => base.GetHashCode();
 
+        // Force inline as the true/false ternary takes it above ALWAYS_INLINE size even though the asm ends up smaller
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static bool operator ==(MethodInfo left, MethodInfo right)
         {
-            if (object.ReferenceEquals(left, right))
-                return true;
-
-            if ((object)left == null || (object)right == null)
-                return false;
-
-            return left.Equals(right);
+            // Test "right" first to allow branch elimination when inlined for null checks (== null)
+            // so it can become a simple test
+            if (right is null)
+            {
+                // return true/false not the test result https://github.com/dotnet/coreclr/issues/914
+                return (left is null) ? true : false;
+            }
+
+            // Quick reference equality test prior to calling the virtual Equality
+            return ReferenceEquals(right, left) ? true : right.Equals(left);
         }
 
         public static bool operator !=(MethodInfo left, MethodInfo right) => !(left == right);
index d290f78..5d2e418 100644 (file)
@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 using System.Collections.Generic;
+using System.Runtime.CompilerServices;
 using System.Runtime.Serialization;
 
 namespace System.Reflection
@@ -114,16 +115,21 @@ namespace System.Reflection
 
         public override bool Equals(object o) => base.Equals(o);
         public override int GetHashCode() => base.GetHashCode();
-
+        
+        // Force inline as the true/false ternary takes it above ALWAYS_INLINE size even though the asm ends up smaller
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static bool operator ==(Module left, Module right)
         {
-            if (object.ReferenceEquals(left, right))
-                return true;
-
-            if ((object)left == null || (object)right == null)
-                return false;
+            // Test "right" first to allow branch elimination when inlined for null checks (== null)
+            // so it can become a simple test
+            if (right is null)
+            {
+                // return true/false not the test result https://github.com/dotnet/coreclr/issues/914
+                return (left is null) ? true : false;
+            }
 
-            return left.Equals(right);
+            // Quick reference equality test prior to calling the virtual Equality
+            return ReferenceEquals(right, left) ? true : right.Equals(left);
         }
 
         public static bool operator !=(Module left, Module right) => !(left == right);
index ff8a02e..562907f 100644 (file)
@@ -4,6 +4,7 @@
 
 using System.Diagnostics;
 using System.Globalization;
+using System.Runtime.CompilerServices;
 
 namespace System.Reflection
 {
@@ -58,15 +59,20 @@ namespace System.Reflection
         public override bool Equals(object obj) => base.Equals(obj);
         public override int GetHashCode() => base.GetHashCode();
 
+        // Force inline as the true/false ternary takes it above ALWAYS_INLINE size even though the asm ends up smaller
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static bool operator ==(PropertyInfo left, PropertyInfo right)
         {
-            if (object.ReferenceEquals(left, right))
-                return true;
-
-            if ((object)left == null || (object)right == null)
-                return false;
-
-            return left.Equals(right);
+            // Test "right" first to allow branch elimination when inlined for null checks (== null)
+            // so it can become a simple test
+            if (right is null)
+            {
+                // return true/false not the test result https://github.com/dotnet/coreclr/issues/914
+                return (left is null) ? true : false;
+            }
+
+            // Quick reference equality test prior to calling the virtual Equality
+            return ReferenceEquals(right, left) ? true : right.Equals(left);
         }
 
         public static bool operator !=(PropertyInfo left, PropertyInfo right) => !(left == right);
index 444ee48..359837f 100644 (file)
@@ -5,6 +5,7 @@
 using System.Globalization;
 using System.Diagnostics;
 using System.Text;
+using System.Runtime.CompilerServices;
 
 namespace System
 {
@@ -405,14 +406,20 @@ namespace System
             return int.TryParse(component, NumberStyles.Integer, CultureInfo.InvariantCulture, out parsedComponent) && parsedComponent >= 0;
         }
 
+        // Force inline as the true/false ternary takes it above ALWAYS_INLINE size even though the asm ends up smaller
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static bool operator ==(Version v1, Version v2)
         {
-            if (v1 is null)
+            // Test "right" first to allow branch elimination when inlined for null checks (== null)
+            // so it can become a simple test
+            if (v2 is null)
             {
-                return v2 is null;
+                // return true/false not the test result https://github.com/dotnet/coreclr/issues/914
+                return (v1 is null) ? true : false;
             }
 
-            return v1.Equals(v2);
+            // Quick reference equality test prior to calling the virtual Equality
+            return ReferenceEquals(v2, v1) ? true : v2.Equals(v1);
         }
 
         public static bool operator !=(Version v1, Version v2)
index 677f7a1..270211a 100644 (file)
@@ -505,20 +505,32 @@ namespace System
             return d;
         }
 
+        // Force inline as the true/false ternary takes it above ALWAYS_INLINE size even though the asm ends up smaller
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static bool operator ==(Delegate d1, Delegate d2)
         {
-            if ((object)d1 == null)
-                return (object)d2 == null;
+            // Test d2 first to allow branch elimination when inlined for null checks (== null)
+            // so it can become a simple test
+            if (d2 is null)
+            {
+                // return true/false not the test result https://github.com/dotnet/coreclr/issues/914
+                return (d1 is null) ? true : false;
+            }
 
-            return d1.Equals(d2);
+            return ReferenceEquals(d2, d1) ? true : d2.Equals((object)d1);
         }
 
         public static bool operator !=(Delegate d1, Delegate d2)
         {
-            if ((object)d1 == null)
-                return (object)d2 != null;
+            // Test d2 first to allow branch elimination when inlined for not null checks (!= null)
+            // so it can become a simple test
+            if (d2 is null)
+            {
+                // return true/false not the test result https://github.com/dotnet/coreclr/issues/914
+                return (d1 is null) ? false : true;
+            }
 
-            return !d1.Equals(d2);
+            return ReferenceEquals(d2, d1) ? false : !d2.Equals(d1);
         }
 
         //
index e0ffd0f..fe4283b 100644 (file)
@@ -4,6 +4,7 @@
 
 using System.Diagnostics;
 using System.Reflection;
+using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
 using System.Runtime.Serialization;
 
@@ -428,24 +429,36 @@ namespace System
             return del;
         }
 
+        // Force inline as the true/false ternary takes it above ALWAYS_INLINE size even though the asm ends up smaller
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static bool operator ==(MulticastDelegate d1, MulticastDelegate d2)
         {
-            if (ReferenceEquals(d1, d2))
+            // Test d2 first to allow branch elimination when inlined for null checks (== null)
+            // so it can become a simple test
+            if (d2 is null)
             {
-                return true;
+                // return true/false not the test result https://github.com/dotnet/coreclr/issues/914
+                return (d1 is null) ? true : false;
             }
 
-            return d1 is null ? false : d1.Equals(d2);
+            return ReferenceEquals(d2, d1) ? true : d2.Equals((object)d1);
         }
 
+        // Force inline as the true/false ternary takes it above ALWAYS_INLINE size even though the asm ends up smaller
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static bool operator !=(MulticastDelegate d1, MulticastDelegate d2)
         {
-            if (ReferenceEquals(d1, d2))
+            // Can't call the == operator as it will call object==
+
+            // Test d2 first to allow branch elimination when inlined for not null checks (!= null)
+            // so it can become a simple test
+            if (d2 is null)
             {
-                return false;
+                // return true/false not the test result https://github.com/dotnet/coreclr/issues/914
+                return (d1 is null) ? false : true;
             }
 
-            return d1 is null ? true : !d1.Equals(d2);
+            return ReferenceEquals(d2, d1) ? false : !d2.Equals(d1);
         }
 
         public override sealed int GetHashCode()