From: Ben Adams Date: Fri, 4 Jan 2019 19:38:22 +0000 (+0100) Subject: Support faster null checks (#21765) X-Git-Tag: accepted/tizen/unified/20190422.045933~196 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=4e82a5801bd804bff0bf6294d7135bbb63df92f6;p=platform%2Fupstream%2Fcoreclr.git Support faster null checks (#21765) --- diff --git a/src/System.Private.CoreLib/shared/System/Globalization/SortVersion.cs b/src/System.Private.CoreLib/shared/System/Globalization/SortVersion.cs index 46e9a83..cb0d4c4 100644 --- a/src/System.Private.CoreLib/shared/System/Globalization/SortVersion.cs +++ b/src/System.Private.CoreLib/shared/System/Globalization/SortVersion.cs @@ -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) diff --git a/src/System.Private.CoreLib/shared/System/Reflection/Assembly.cs b/src/System.Private.CoreLib/shared/System/Reflection/Assembly.cs index 516f81d..c7b39ff 100644 --- a/src/System.Private.CoreLib/shared/System/Reflection/Assembly.cs +++ b/src/System.Private.CoreLib/shared/System/Reflection/Assembly.cs @@ -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) diff --git a/src/System.Private.CoreLib/shared/System/Reflection/ConstructorInfo.cs b/src/System.Private.CoreLib/shared/System/Reflection/ConstructorInfo.cs index 3ee1dbf..3d8750c 100644 --- a/src/System.Private.CoreLib/shared/System/Reflection/ConstructorInfo.cs +++ b/src/System.Private.CoreLib/shared/System/Reflection/ConstructorInfo.cs @@ -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); diff --git a/src/System.Private.CoreLib/shared/System/Reflection/EventInfo.cs b/src/System.Private.CoreLib/shared/System/Reflection/EventInfo.cs index ccd9acf..e352613 100644 --- a/src/System.Private.CoreLib/shared/System/Reflection/EventInfo.cs +++ b/src/System.Private.CoreLib/shared/System/Reflection/EventInfo.cs @@ -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); diff --git a/src/System.Private.CoreLib/shared/System/Reflection/FieldInfo.cs b/src/System.Private.CoreLib/shared/System/Reflection/FieldInfo.cs index 0863c2b..53bcf17 100644 --- a/src/System.Private.CoreLib/shared/System/Reflection/FieldInfo.cs +++ b/src/System.Private.CoreLib/shared/System/Reflection/FieldInfo.cs @@ -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); diff --git a/src/System.Private.CoreLib/shared/System/Reflection/MemberInfo.cs b/src/System.Private.CoreLib/shared/System/Reflection/MemberInfo.cs index c61198d..2f439d8 100644 --- a/src/System.Private.CoreLib/shared/System/Reflection/MemberInfo.cs +++ b/src/System.Private.CoreLib/shared/System/Reflection/MemberInfo.cs @@ -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); diff --git a/src/System.Private.CoreLib/shared/System/Reflection/MethodBase.cs b/src/System.Private.CoreLib/shared/System/Reflection/MethodBase.cs index 0037c74..e5ca0d5 100644 --- a/src/System.Private.CoreLib/shared/System/Reflection/MethodBase.cs +++ b/src/System.Private.CoreLib/shared/System/Reflection/MethodBase.cs @@ -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); diff --git a/src/System.Private.CoreLib/shared/System/Reflection/MethodInfo.cs b/src/System.Private.CoreLib/shared/System/Reflection/MethodInfo.cs index 95c62ba..920a9a2 100644 --- a/src/System.Private.CoreLib/shared/System/Reflection/MethodInfo.cs +++ b/src/System.Private.CoreLib/shared/System/Reflection/MethodInfo.cs @@ -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); diff --git a/src/System.Private.CoreLib/shared/System/Reflection/Module.cs b/src/System.Private.CoreLib/shared/System/Reflection/Module.cs index d290f78..5d2e418 100644 --- a/src/System.Private.CoreLib/shared/System/Reflection/Module.cs +++ b/src/System.Private.CoreLib/shared/System/Reflection/Module.cs @@ -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); diff --git a/src/System.Private.CoreLib/shared/System/Reflection/PropertyInfo.cs b/src/System.Private.CoreLib/shared/System/Reflection/PropertyInfo.cs index ff8a02e..562907f 100644 --- a/src/System.Private.CoreLib/shared/System/Reflection/PropertyInfo.cs +++ b/src/System.Private.CoreLib/shared/System/Reflection/PropertyInfo.cs @@ -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); diff --git a/src/System.Private.CoreLib/shared/System/Version.cs b/src/System.Private.CoreLib/shared/System/Version.cs index 444ee48..359837f 100644 --- a/src/System.Private.CoreLib/shared/System/Version.cs +++ b/src/System.Private.CoreLib/shared/System/Version.cs @@ -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) diff --git a/src/System.Private.CoreLib/src/System/Delegate.cs b/src/System.Private.CoreLib/src/System/Delegate.cs index 677f7a1..270211a 100644 --- a/src/System.Private.CoreLib/src/System/Delegate.cs +++ b/src/System.Private.CoreLib/src/System/Delegate.cs @@ -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); } // diff --git a/src/System.Private.CoreLib/src/System/MulticastDelegate.cs b/src/System.Private.CoreLib/src/System/MulticastDelegate.cs index e0ffd0f..fe4283b 100644 --- a/src/System.Private.CoreLib/src/System/MulticastDelegate.cs +++ b/src/System.Private.CoreLib/src/System/MulticastDelegate.cs @@ -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()