From c6042da5a97a4299af6792d93e505fa1075bcd90 Mon Sep 17 00:00:00 2001 From: James Ko Date: Wed, 22 Feb 2017 13:01:40 -0500 Subject: [PATCH] Separate Comparer and EqualityComparer creation logic into a non-generic class (dotnet/coreclr#9640) Commit migrated from https://github.com/dotnet/coreclr/commit/84092471c678da00bb59d86fb5fcd0890a524472 --- .../src/mscorlib/System.Private.CoreLib.csproj | 1 + .../src/System/Collections/Generic/Comparer.cs | 79 +------- .../System/Collections/Generic/ComparerHelpers.cs | 210 +++++++++++++++++++++ .../System/Collections/Generic/EqualityComparer.cs | 73 +------ 4 files changed, 221 insertions(+), 142 deletions(-) create mode 100644 src/coreclr/src/mscorlib/src/System/Collections/Generic/ComparerHelpers.cs diff --git a/src/coreclr/src/mscorlib/System.Private.CoreLib.csproj b/src/coreclr/src/mscorlib/System.Private.CoreLib.csproj index af5bb8f..8162956 100644 --- a/src/coreclr/src/mscorlib/System.Private.CoreLib.csproj +++ b/src/coreclr/src/mscorlib/System.Private.CoreLib.csproj @@ -1085,6 +1085,7 @@ + diff --git a/src/coreclr/src/mscorlib/src/System/Collections/Generic/Comparer.cs b/src/coreclr/src/mscorlib/src/System/Collections/Generic/Comparer.cs index 056843a..714b386 100644 --- a/src/coreclr/src/mscorlib/src/System/Collections/Generic/Comparer.cs +++ b/src/coreclr/src/mscorlib/src/System/Collections/Generic/Comparer.cs @@ -20,14 +20,9 @@ namespace System.Collections.Generic [TypeDependencyAttribute("System.Collections.Generic.ObjectComparer`1")] public abstract class Comparer : IComparer, IComparer { - static readonly Comparer defaultComparer = CreateComparer(); - - public static Comparer Default { - get { - Contract.Ensures(Contract.Result>() != null); - return defaultComparer; - } - } + // To minimize generic instantiation overhead of creating the comparer per type, we keep the generic portion of the code as small + // as possible and define most of the creation logic in a non-generic class. + public static Comparer Default { get; } = (Comparer)ComparerHelpers.CreateDefaultComparer(typeof(T)); public static Comparer Create(Comparison comparison) { @@ -39,66 +34,6 @@ namespace System.Collections.Generic return new ComparisonComparer(comparison); } - // - // Note that logic in this method is replicated in vm\compile.cpp to ensure that NGen - // saves the right instantiations - // - private static Comparer CreateComparer() - { - object result = null; - RuntimeType t = (RuntimeType)typeof(T); - - // If T implements IComparable return a GenericComparer - if (typeof(IComparable).IsAssignableFrom(t)) - { - result = RuntimeTypeHandle.CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(GenericComparer), t); - } - else if (default(T) == null) - { - // If T is a Nullable where U implements IComparable return a NullableComparer - if (t.IsGenericType && t.GetGenericTypeDefinition() == typeof(Nullable<>)) { - RuntimeType u = (RuntimeType)t.GetGenericArguments()[0]; - if (typeof(IComparable<>).MakeGenericType(u).IsAssignableFrom(u)) { - result = RuntimeTypeHandle.CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(NullableComparer), u); - } - } - } - else if (t.IsEnum) - { - // Explicitly call Enum.GetUnderlyingType here. Although GetTypeCode - // ends up doing this anyway, we end up avoiding an unnecessary P/Invoke - // and virtual method call. - TypeCode underlyingTypeCode = Type.GetTypeCode(Enum.GetUnderlyingType(t)); - - // Depending on the enum type, we need to special case the comparers so that we avoid boxing - // Specialize differently for signed/unsigned types so we avoid problems with large numbers - switch (underlyingTypeCode) - { - case TypeCode.SByte: - case TypeCode.Int16: - case TypeCode.Int32: - result = RuntimeTypeHandle.CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(Int32EnumComparer), t); - break; - case TypeCode.Byte: - case TypeCode.UInt16: - case TypeCode.UInt32: - result = RuntimeTypeHandle.CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(UInt32EnumComparer), t); - break; - // 64-bit enums: use UnsafeEnumCastLong - case TypeCode.Int64: - result = RuntimeTypeHandle.CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(Int64EnumComparer), t); - break; - case TypeCode.UInt64: - result = RuntimeTypeHandle.CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(UInt64EnumComparer), t); - break; - } - } - - return result != null ? - (Comparer)result : - new ObjectComparer(); // Fallback to ObjectComparer, which uses boxing - } - public abstract int Compare(T x, T y); int IComparer.Compare(object x, object y) { @@ -196,7 +131,7 @@ namespace System.Collections.Generic { public Int32EnumComparer() { - Debug.Assert(typeof(T).IsEnum, "This type is only intended to be used to compare enums!"); + Debug.Assert(typeof(T).IsEnum); } // Used by the serialization engine. @@ -231,7 +166,7 @@ namespace System.Collections.Generic { public UInt32EnumComparer() { - Debug.Assert(typeof(T).IsEnum, "This type is only intended to be used to compare enums!"); + Debug.Assert(typeof(T).IsEnum); } // Used by the serialization engine. @@ -262,7 +197,7 @@ namespace System.Collections.Generic { public Int64EnumComparer() { - Debug.Assert(typeof(T).IsEnum, "This type is only intended to be used to compare enums!"); + Debug.Assert(typeof(T).IsEnum); } public override int Compare(T x, T y) @@ -290,7 +225,7 @@ namespace System.Collections.Generic { public UInt64EnumComparer() { - Debug.Assert(typeof(T).IsEnum, "This type is only intended to be used to compare enums!"); + Debug.Assert(typeof(T).IsEnum); } // Used by the serialization engine. diff --git a/src/coreclr/src/mscorlib/src/System/Collections/Generic/ComparerHelpers.cs b/src/coreclr/src/mscorlib/src/System/Collections/Generic/ComparerHelpers.cs new file mode 100644 index 0000000..3e42841 --- /dev/null +++ b/src/coreclr/src/mscorlib/src/System/Collections/Generic/ComparerHelpers.cs @@ -0,0 +1,210 @@ +// Licensed to the .NET Foundation under one or more agreements. +// 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.Diagnostics; +using static System.RuntimeTypeHandle; + +namespace System.Collections.Generic +{ + /// + /// Helper class for creating the default and . + /// + /// + /// This class is intentionally type-unsafe and non-generic to minimize the generic instantiation overhead of creating + /// the default comparer/equality comparer for a new type parameter. Efficiency of the methods in here does not matter too + /// much since they will only be run once per type parameter, but generic code involved in creating the comparers needs to be + /// kept to a minimum. + /// + internal static class ComparerHelpers + { + /// + /// Creates the default . + /// + /// The type to create the default comparer for. + /// + /// The logic in this method is replicated in vm/compile.cpp to ensure that NGen saves the right instantiations. + /// + internal static object CreateDefaultComparer(Type type) + { + Debug.Assert(type != null && type is RuntimeType); + + object result = null; + var runtimeType = (RuntimeType)type; + + // If T implements IComparable return a GenericComparer + if (typeof(IComparable<>).MakeGenericType(type).IsAssignableFrom(type)) + { + result = CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(GenericComparer), runtimeType); + } + // Nullable does not implement IComparable directly because that would add an extra interface call per comparison. + // Instead, it relies on Comparer.Default to specialize for nullables and do the lifted comparisons if T implements IComparable. + else if (type.IsGenericType) + { + if (type.GetGenericTypeDefinition() == typeof(Nullable<>)) + { + result = TryCreateNullableComparer(runtimeType); + } + } + // The comparer for enums is specialized to avoid boxing. + else if (type.IsEnum) + { + result = TryCreateEnumComparer(runtimeType); + } + + return result ?? CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(ObjectComparer), runtimeType); + } + + /// + /// Creates the default for a nullable type. + /// + /// The nullable type to create the default comparer for. + private static object TryCreateNullableComparer(RuntimeType nullableType) + { + Debug.Assert(nullableType != null); + Debug.Assert(nullableType.IsGenericType && nullableType.GetGenericTypeDefinition() == typeof(Nullable<>)); + + var embeddedType = (RuntimeType)nullableType.GetGenericArguments()[0]; + + if (typeof(IComparable<>).MakeGenericType(embeddedType).IsAssignableFrom(embeddedType)) + { + return RuntimeTypeHandle.CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(NullableComparer), embeddedType); + } + + return null; + } + + /// + /// Creates the default for an enum type. + /// + /// The enum type to create the default comparer for. + private static object TryCreateEnumComparer(RuntimeType enumType) + { + Debug.Assert(enumType != null); + Debug.Assert(enumType.IsEnum); + + // Explicitly call Enum.GetUnderlyingType here. Although GetTypeCode + // ends up doing this anyway, we end up avoiding an unnecessary P/Invoke + // and virtual method call. + TypeCode underlyingTypeCode = Type.GetTypeCode(Enum.GetUnderlyingType(enumType)); + + // Depending on the enum type, we need to special case the comparers so that we avoid boxing. + // Specialize differently for signed/unsigned types so we avoid problems with large numbers. + switch (underlyingTypeCode) + { + case TypeCode.SByte: + case TypeCode.Int16: + case TypeCode.Int32: + return RuntimeTypeHandle.CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(Int32EnumComparer), enumType); + case TypeCode.Byte: + case TypeCode.UInt16: + case TypeCode.UInt32: + return RuntimeTypeHandle.CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(UInt32EnumComparer), enumType); + // 64-bit enums: Use `UnsafeEnumCastLong` + case TypeCode.Int64: + return RuntimeTypeHandle.CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(Int64EnumComparer), enumType); + case TypeCode.UInt64: + return RuntimeTypeHandle.CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(UInt64EnumComparer), enumType); + } + + return null; + } + + /// + /// Creates the default . + /// + /// The type to create the default equality comparer for. + /// + /// The logic in this method is replicated in vm/compile.cpp to ensure that NGen saves the right instantiations. + /// + internal static object CreateDefaultEqualityComparer(Type type) + { + Debug.Assert(type != null && type is RuntimeType); + + object result = null; + var runtimeType = (RuntimeType)type; + + // Specialize for byte so Array.IndexOf is faster. + if (type == typeof(byte)) + { + result = new ByteEqualityComparer(); + } + // If T implements IEquatable return a GenericEqualityComparer + else if (typeof(IEquatable<>).MakeGenericType(type).IsAssignableFrom(type)) + { + result = CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(GenericEqualityComparer), runtimeType); + } + // Nullable does not implement IEquatable directly because that would add an extra interface call per comparison. + // Instead, it relies on EqualityComparer.Default to specialize for nullables and do the lifted comparisons if T implements IEquatable. + else if (type.IsGenericType) + { + if (type.GetGenericTypeDefinition() == typeof(Nullable<>)) + { + result = TryCreateNullableEqualityComparer(runtimeType); + } + } + // The equality comparer for enums is specialized to avoid boxing. + else if (type.IsEnum) + { + result = TryCreateEnumEqualityComparer(runtimeType); + } + + return result ?? CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(ObjectEqualityComparer), runtimeType); + } + + /// + /// Creates the default for a nullable type. + /// + /// The nullable type to create the default equality comparer for. + private static object TryCreateNullableEqualityComparer(RuntimeType nullableType) + { + Debug.Assert(nullableType != null); + Debug.Assert(nullableType.IsGenericType && nullableType.GetGenericTypeDefinition() == typeof(Nullable<>)); + + var embeddedType = (RuntimeType)nullableType.GetGenericArguments()[0]; + + if (typeof(IEquatable<>).MakeGenericType(embeddedType).IsAssignableFrom(embeddedType)) + { + return RuntimeTypeHandle.CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(NullableEqualityComparer), embeddedType); + } + + return null; + } + + /// + /// Creates the default for an enum type. + /// + /// The enum type to create the default equality comparer for. + private static object TryCreateEnumEqualityComparer(RuntimeType enumType) + { + Debug.Assert(enumType != null); + Debug.Assert(enumType.IsEnum); + + // See the METHOD__JIT_HELPERS__UNSAFE_ENUM_CAST and METHOD__JIT_HELPERS__UNSAFE_ENUM_CAST_LONG cases in getILIntrinsicImplementation + // for how we cast the enum types to integral values in the comparer without boxing. + + TypeCode underlyingTypeCode = Type.GetTypeCode(Enum.GetUnderlyingType(enumType)); + + // Depending on the enum type, we need to special case the comparers so that we avoid boxing. + // Note: We have different comparers for short and sbyte, since for those types GetHashCode does not simply return the value. + // We need to preserve what they would return. + switch (underlyingTypeCode) + { + case TypeCode.Int16: + return RuntimeTypeHandle.CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(ShortEnumEqualityComparer), enumType); + case TypeCode.SByte: + return RuntimeTypeHandle.CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(SByteEnumEqualityComparer), enumType); + case TypeCode.Int32: + case TypeCode.UInt32: + case TypeCode.Byte: + case TypeCode.UInt16: + return RuntimeTypeHandle.CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(EnumEqualityComparer), enumType); + case TypeCode.Int64: + case TypeCode.UInt64: + return RuntimeTypeHandle.CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(LongEnumEqualityComparer), enumType); + } + + return null; + } + } +} diff --git a/src/coreclr/src/mscorlib/src/System/Collections/Generic/EqualityComparer.cs b/src/coreclr/src/mscorlib/src/System/Collections/Generic/EqualityComparer.cs index 0f9259d..7178ddc 100644 --- a/src/coreclr/src/mscorlib/src/System/Collections/Generic/EqualityComparer.cs +++ b/src/coreclr/src/mscorlib/src/System/Collections/Generic/EqualityComparer.cs @@ -18,76 +18,9 @@ namespace System.Collections.Generic [TypeDependencyAttribute("System.Collections.Generic.ObjectEqualityComparer`1")] public abstract class EqualityComparer : IEqualityComparer, IEqualityComparer { - static readonly EqualityComparer defaultComparer = CreateComparer(); - - public static EqualityComparer Default { - get { - Contract.Ensures(Contract.Result>() != null); - return defaultComparer; - } - } - - // - // Note that logic in this method is replicated in vm\compile.cpp to ensure that NGen - // saves the right instantiations - // - private static EqualityComparer CreateComparer() - { - Contract.Ensures(Contract.Result>() != null); - - object result = null; - RuntimeType t = (RuntimeType)typeof(T); - - // Specialize type byte for performance reasons - if (t == typeof(byte)) { - result = new ByteEqualityComparer(); - } - // If T implements IEquatable return a GenericEqualityComparer - else if (typeof(IEquatable).IsAssignableFrom(t)) - { - result = RuntimeTypeHandle.CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(GenericEqualityComparer), t); - } - else if (default(T) == null) // Reference type/Nullable - { - // If T is a Nullable where U implements IEquatable return a NullableEqualityComparer - if (t.IsGenericType && t.GetGenericTypeDefinition() == typeof(Nullable<>)) { - RuntimeType u = (RuntimeType)t.GetGenericArguments()[0]; - if (typeof(IEquatable<>).MakeGenericType(u).IsAssignableFrom(u)) { - result = RuntimeTypeHandle.CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(NullableEqualityComparer), u); - } - } - } - // See the METHOD__JIT_HELPERS__UNSAFE_ENUM_CAST and METHOD__JIT_HELPERS__UNSAFE_ENUM_CAST_LONG cases in getILIntrinsicImplementation - else if (t.IsEnum) { - TypeCode underlyingTypeCode = Type.GetTypeCode(Enum.GetUnderlyingType(t)); - - // Depending on the enum type, we need to special case the comparers so that we avoid boxing - // Note: We have different comparers for Short and SByte because for those types we need to make sure we call GetHashCode on the actual underlying type as the - // implementation of GetHashCode is more complex than for the other types. - switch (underlyingTypeCode) { - case TypeCode.Int16: // short - result = RuntimeTypeHandle.CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(ShortEnumEqualityComparer), t); - break; - case TypeCode.SByte: - result = RuntimeTypeHandle.CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(SByteEnumEqualityComparer), t); - break; - case TypeCode.Int32: - case TypeCode.UInt32: - case TypeCode.Byte: - case TypeCode.UInt16: //ushort - result = RuntimeTypeHandle.CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(EnumEqualityComparer), t); - break; - case TypeCode.Int64: - case TypeCode.UInt64: - result = RuntimeTypeHandle.CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(LongEnumEqualityComparer), t); - break; - } - } - - return result != null ? - (EqualityComparer)result : - new ObjectEqualityComparer(); // Fallback to ObjectEqualityComparer, which uses boxing - } + // To minimize generic instantiation overhead of creating the comparer per type, we keep the generic portion of the code as small + // as possible and define most of the creation logic in a non-generic class. + public static EqualityComparer Default { get; } = (EqualityComparer)ComparerHelpers.CreateDefaultEqualityComparer(typeof(T)); [Pure] public abstract bool Equals(T x, T y); -- 2.7.4