Improve Enum.ToString performance (dotnet/coreclr#6645)
authorBen Adams <thundercat@illyriad.co.uk>
Wed, 10 Aug 2016 15:17:37 +0000 (16:17 +0100)
committerJan Kotas <jkotas@microsoft.com>
Wed, 10 Aug 2016 15:17:37 +0000 (08:17 -0700)
Reduces allocations by 66%; increases performance by 600% (x7)

With these changes ToString no longer boxes the value and doesn't create an empty array for `CustomAttributeRecords` when the enum is not flagged; also caches whether the enum is flagged.

It still boxes the enum to Enum to make the ToString virtual call however.

Using the @jkotas enummark :wink:

```csharp
enum MyEnum { One, Two, Three, Four, Five, Six, Seven, Eight, Nine, Ten };
public static void Main(string[] args)
{
    (MyEnum.Seven).ToString();
    int start = Environment.TickCount;
    for (int i = 0; i < 10000000; i++)
    {
        (MyEnum.Seven).ToString();
    }
    int end = Environment.TickCount;
    Console.WriteLine((end - start).ToString());
}
```
Pre: 5828ms, 5828ms, 5829ms (1.7M/s)
Post: 828ms, 828ms, 828ms (12.1M/s)

Commit migrated from https://github.com/dotnet/coreclr/commit/571b963cd4f7af8674d1031c4adefb3d8bc47618

src/coreclr/src/mscorlib/src/System/Enum.cs
src/coreclr/src/mscorlib/src/System/Reflection/CustomAttribute.cs
src/coreclr/src/mscorlib/src/System/RtType.cs

index 598719e..35e70d3 100644 (file)
@@ -24,14 +24,15 @@ namespace System
 
         #region Private Static Methods
         [System.Security.SecuritySafeCritical]  // auto-generated
-        private static ValuesAndNames GetCachedValuesAndNames(RuntimeType enumType, bool getNames)
+        private static TypeValuesAndNames GetCachedValuesAndNames(RuntimeType enumType, bool getNames)
         {
-            ValuesAndNames entry = enumType.GenericCache as ValuesAndNames;
+            TypeValuesAndNames entry = enumType.GenericCache as TypeValuesAndNames;
 
             if (entry == null || (getNames && entry.Names == null))
             {
                 ulong[] values = null;
                 String[] names = null;
+                bool isFlags = enumType.IsDefined(typeof(System.FlagsAttribute), false);
 
                 GetEnumValuesAndNames(
                     enumType.GetTypeHandleInternal(),
@@ -39,125 +40,119 @@ namespace System
                     JitHelpers.GetObjectHandleOnStack(ref names),
                     getNames);
 
-                entry = new ValuesAndNames(values, names);
+                entry = new TypeValuesAndNames(isFlags, values, names);
                 enumType.GenericCache = entry;
             }
 
             return entry;
         }
 
-        private static String InternalFormattedHexString(Object value)
+        private unsafe String InternalFormattedHexString()
+        {
+            fixed (void* pValue = &JitHelpers.GetPinningHelper(this).m_data)
+            {
+                switch (InternalGetCorElementType())
+                {
+                    case CorElementType.I1:
+                    case CorElementType.U1:
+                        return (*(byte*)pValue).ToString("X2", null);
+                    case CorElementType.Boolean:
+                        // direct cast from bool to byte is not allowed
+                        return Convert.ToByte(*(bool*)pValue).ToString("X2", null);
+                    case CorElementType.I2:
+                    case CorElementType.U2:
+                    case CorElementType.Char:
+                        return (*(ushort*)pValue).ToString("X4", null);
+                    case CorElementType.I4:
+                    case CorElementType.U4:
+                        return (*(uint*)pValue).ToString("X8", null);
+                    case CorElementType.I8:
+                    case CorElementType.U8:
+                        return (*(ulong*)pValue).ToString("X16", null);
+                    default:
+                        Contract.Assert(false, "Invalid Object type in Format");
+                        throw new InvalidOperationException(Environment.GetResourceString("InvalidOperation_UnknownEnumType"));
+                }
+            }
+        }
+
+        private static String InternalFormattedHexString(object value)
         {
             TypeCode typeCode = Convert.GetTypeCode(value);
 
             switch (typeCode)
             {
-                case TypeCode.SByte :
-                    {
-                        Byte result = (byte)(sbyte)value;
-
-                        return result.ToString("X2", null);
-                    }
-
-                case TypeCode.Byte :
-                    {
-                        Byte result = (byte)value;
-
-                        return result.ToString("X2", null);
-                    }
-
+                case TypeCode.SByte:
+                    return ((byte)(sbyte)value).ToString("X2", null);
+                case TypeCode.Byte:
+                    return ((byte)value).ToString("X2", null);
                 case TypeCode.Boolean:
-                    {
-                        // direct cast from bool to byte is not allowed
-                        Byte result = Convert.ToByte((bool)value);
-
-                        return result.ToString("X2", null);
-                    }
-
+                    // direct cast from bool to byte is not allowed
+                    return Convert.ToByte((bool)value).ToString("X2", null);
                 case TypeCode.Int16:
-                    {
-                        UInt16 result = (UInt16)(Int16)value;
-
-                        return result.ToString("X4", null);
-                    }
-
-                case TypeCode.UInt16 :
-                    {
-                        UInt16 result = (UInt16)value;
-
-                        return result.ToString("X4", null);
-                    }
-
+                    return ((UInt16)(Int16)value).ToString("X4", null);
+                case TypeCode.UInt16:
+                    return ((UInt16)value).ToString("X4", null);
                 case TypeCode.Char:
-                    {
-                        UInt16 result = (UInt16)(Char)value;
-
-                        return result.ToString("X4", null);
-                    }
-
+                    return ((UInt16)(Char)value).ToString("X4", null);
                 case TypeCode.UInt32:
-                    {
-                        UInt32 result = (UInt32)value;
-
-                        return result.ToString("X8", null);
-                    }
-
-                case TypeCode.Int32 :
-                    {
-                        UInt32 result = (UInt32)(int)value;
-
-                        return result.ToString("X8", null);
-                    }
-
-                case TypeCode.UInt64 :
-                    {
-                        UInt64 result = (UInt64)value;
-
-                        return result.ToString("X16", null);
-                    }
-
-                case TypeCode.Int64 :
-                    {
-                        UInt64 result = (UInt64)(Int64)value;
-
-                        return result.ToString("X16", null);
-                    }
-
+                    return ((UInt32)value).ToString("X8", null);
+                case TypeCode.Int32:
+                    return ((UInt32)(Int32)value).ToString("X8", null);
+                case TypeCode.UInt64:
+                    return ((UInt64)value).ToString("X16", null);
+                case TypeCode.Int64:
+                    return ((UInt64)(Int64)value).ToString("X16", null);
                 // All unsigned types will be directly cast
-                default :
+                default:
                     Contract.Assert(false, "Invalid Object type in Format");
                     throw new InvalidOperationException(Environment.GetResourceString("InvalidOperation_UnknownEnumType"));
             }
         }
 
-        private static String InternalFormat(RuntimeType eT, Object value)
+        internal static String GetEnumName(RuntimeType eT, ulong ulValue)
         {
             Contract.Requires(eT != null);
-            Contract.Requires(value != null);
-            if (!eT.IsDefined(typeof(System.FlagsAttribute), false)) // Not marked with Flags attribute
+            ulong[] ulValues = Enum.InternalGetValues(eT);
+            int index = Array.BinarySearch(ulValues, ulValue);
+
+            if (index >= 0)
             {
-                // Try to see if its one of the enum values, then we return a String back else the value
-                String retval = GetName(eT, value);
-                if (retval == null)
-                    return value.ToString();
-                else
-                    return retval;
+                string[] names = Enum.InternalGetNames(eT);
+                return names[index];
             }
-            else // These are flags OR'ed together (We treat everything as unsigned types)
-            {
-                return InternalFlagsFormat(eT, value);
 
-            }
+            return null; // return null so the caller knows to .ToString() the input
         }
 
-        private static String InternalFlagsFormat(RuntimeType eT, Object value)
+        private static String InternalFormat(RuntimeType eT, ulong value)
         {
             Contract.Requires(eT != null);
-            Contract.Requires(value != null);
-            ulong result = ToUInt64(value);
 
             // These values are sorted by value. Don't change this
-            ValuesAndNames entry = GetCachedValuesAndNames(eT, true);
+            TypeValuesAndNames entry = GetCachedValuesAndNames(eT, true);
+
+            if (!entry.IsFlag) // Not marked with Flags attribute
+            {
+                return Enum.GetEnumName(eT, value);
+            }
+            else // These are flags OR'ed together (We treat everything as unsigned types)
+            {
+                return InternalFlagsFormat(eT, entry, value);
+            }
+        }
+
+        private static String InternalFlagsFormat(RuntimeType eT,ulong result)
+        {
+            // These values are sorted by value. Don't change this
+            TypeValuesAndNames entry = GetCachedValuesAndNames(eT, true);
+
+            return InternalFlagsFormat(eT, entry, result);
+        }
+
+        private static String InternalFlagsFormat(RuntimeType eT, TypeValuesAndNames entry, ulong result)
+        {
+            Contract.Requires(eT != null);
 
             String[] names = entry.Names;
             ulong[] values = entry.Values;
@@ -191,10 +186,10 @@ namespace System
 
             // We were unable to represent this number as a bitwise or of valid flags
             if (result != 0)
-                return value.ToString();
+                return null; // return null so the caller knows to .ToString() the input
 
             // For the case when we have zero
-            if (saveResult==0)
+            if (saveResult == 0)
             {
                 if (values.Length > 0 && values[0] == 0)
                     return names[0]; // Zero was one of the enum values.
@@ -202,7 +197,9 @@ namespace System
                     return "0";
             }
             else
-            return retval.ToString(); // Return the string representation
+            {
+                return retval.ToString(); // Return the string representation
+            }
         }
 
         internal static ulong ToUInt64(Object value)
@@ -210,31 +207,47 @@ namespace System
             // Helper function to silently convert the value to UInt64 from the other base types for enum without throwing an exception.
             // This is need since the Convert functions do overflow checks.
             TypeCode typeCode = Convert.GetTypeCode(value);
-            ulong result;
 
-            switch(typeCode)
+            ulong result;
+            switch (typeCode)
             {
                 case TypeCode.SByte:
-                case TypeCode.Int16:
-                case TypeCode.Int32:
-                case TypeCode.Int64:
-                    result = (UInt64)Convert.ToInt64(value, CultureInfo.InvariantCulture);
+                    result = (ulong)(sbyte)value;
                     break;
-
                 case TypeCode.Byte:
+                    result = (byte)value;
+                    break;
+                case TypeCode.Boolean:
+                    // direct cast from bool to byte is not allowed
+                    result = Convert.ToByte((bool)value);
+                    break;
+                case TypeCode.Int16:
+                    result = (ulong)(Int16)value;
+                    break;
                 case TypeCode.UInt16:
+                    result = (UInt16)value;
+                    break;
+                case TypeCode.Char:
+                    result = (UInt16)(Char)value;
+                    break;
                 case TypeCode.UInt32:
+                    result = (UInt32)value;
+                    break;
+                case TypeCode.Int32:
+                    result = (ulong)(int)value;
+                    break;
                 case TypeCode.UInt64:
-                case TypeCode.Boolean:
-                case TypeCode.Char:
-                    result = Convert.ToUInt64(value, CultureInfo.InvariantCulture);
+                    result = (ulong)value;
+                    break;
+                case TypeCode.Int64:
+                    result = (ulong)(Int64)value;
                     break;
-
-                default:
                 // All unsigned types will be directly cast
+                default:
                     Contract.Assert(false, "Invalid Object type in ToUInt64");
                     throw new InvalidOperationException(Environment.GetResourceString("InvalidOperation_UnknownEnumType"));
             }
+
             return result;
         }
 
@@ -420,7 +433,7 @@ namespace System
 
             // Find the field. Let's assume that these are always static classes 
             // because the class is an enum.
-            ValuesAndNames entry = GetCachedValuesAndNames(rtType, true);
+            TypeValuesAndNames entry = GetCachedValuesAndNames(rtType, true);
             String[] enumNames = entry.Names;
             ulong[] enumValues = entry.Values;
 
@@ -627,42 +640,39 @@ namespace System
 
             // If the value is an Enum then we need to extract the underlying value from it
             if (valueType.IsEnum) {
-                Type valueUnderlyingType = GetUnderlyingType(valueType);
 
                 if (!valueType.IsEquivalentTo(enumType))
                     throw new ArgumentException(Environment.GetResourceString("Arg_EnumAndObjectMustBeSameType", valueType.ToString(), enumType.ToString()));
 
-                valueType = valueUnderlyingType;
-                value = ((Enum)value).GetValue();
+                if (format.Length != 1)
+                {
+                    // all acceptable format string are of length 1
+                    throw new FormatException(Environment.GetResourceString("Format_InvalidEnumFormatSpecification"));
+                }
+                return ((Enum)value).ToString(format);
             }
             // The value must be of the same type as the Underlying type of the Enum
             else if (valueType != underlyingType) {
                 throw new ArgumentException(Environment.GetResourceString("Arg_EnumFormatUnderlyingTypeAndObjectMustBeSameType", valueType.ToString(), underlyingType.ToString()));
             }
-
-            if( format.Length != 1) {
+            if (format.Length != 1)
+            {
                 // all acceptable format string are of length 1
                 throw new FormatException(Environment.GetResourceString("Format_InvalidEnumFormatSpecification"));
             }
-            
+
             char formatCh = format[0];
+            if (formatCh == 'G' || formatCh == 'g')
+                return GetEnumName(rtType, ToUInt64(value));
 
-            if (formatCh == 'D' || formatCh == 'd') {
+            if (formatCh == 'D' || formatCh == 'd')
                 return value.ToString();
-            }
 
-            if (formatCh == 'X' || formatCh == 'x') {
-                // Retrieve the value from the field.
+            if (formatCh == 'X' || formatCh == 'x')
                 return InternalFormattedHexString(value);
-            }
 
-            if (formatCh == 'G' || formatCh == 'g') {
-                return InternalFormat(rtType, value);
-            }
-
-            if (formatCh == 'F' || formatCh == 'f') {
-                return InternalFlagsFormat(rtType, value);
-            }
+            if (formatCh == 'F' || formatCh == 'f')
+                return Enum.InternalFlagsFormat(rtType, ToUInt64(value)) ?? value.ToString();
 
             throw new FormatException(Environment.GetResourceString("Format_InvalidEnumFormatSpecification"));
         }
@@ -670,15 +680,17 @@ namespace System
         #endregion
 
         #region Definitions
-        private class ValuesAndNames
+        private class TypeValuesAndNames
         {
             // Each entry contains a list of sorted pair of enum field names and values, sorted by values
-            public ValuesAndNames(ulong[] values, String[] names)
+            public TypeValuesAndNames(bool isFlag, ulong[] values, String[] names)
             {
+                this.IsFlag = isFlag;
                 this.Values = values;
                 this.Names = names;
             }
 
+            public bool IsFlag;
             public ulong[] Values;
             public String[] Names;
         }
@@ -727,6 +739,59 @@ namespace System
             }
         }
 
+        [System.Security.SecuritySafeCritical]
+        private unsafe ulong ToUInt64()
+        {
+            fixed (void* pValue = &JitHelpers.GetPinningHelper(this).m_data)
+            {
+                switch (InternalGetCorElementType())
+                {
+                    case CorElementType.I1:
+                        return (ulong)*(sbyte*)pValue;
+                    case CorElementType.U1:
+                        return *(byte*)pValue;
+                    case CorElementType.Boolean:
+                        return Convert.ToUInt64(*(bool*)pValue, CultureInfo.InvariantCulture);
+                    case CorElementType.I2:
+                        return (ulong)*(short*)pValue;
+                    case CorElementType.U2:
+                    case CorElementType.Char:
+                        return *(ushort*)pValue;
+                    case CorElementType.I4:
+                        return (ulong)*(int*)pValue;
+                    case CorElementType.U4:
+                    case CorElementType.R4:
+                        return *(uint*)pValue;
+                    case CorElementType.I8:
+                        return (ulong)*(long*)pValue;
+                    case CorElementType.U8:
+                    case CorElementType.R8:
+                        return *(ulong*)pValue;
+                    case CorElementType.I:
+                        if (IntPtr.Size == 8)
+                        {
+                            return *(ulong*)pValue;
+                        }
+                        else
+                        {
+                            return (ulong)*(int*)pValue;
+                        }
+                    case CorElementType.U:
+                        if (IntPtr.Size == 8)
+                        {
+                            return *(ulong*)pValue;
+                        }
+                        else
+                        {
+                            return *(uint*)pValue;
+                        }
+                    default:
+                        Contract.Assert(false, "Invalid primitive type");
+                        return 0;
+                }
+            }
+        }
+
         [System.Security.SecurityCritical]  // auto-generated
         [MethodImplAttribute(MethodImplOptions.InternalCall)]
         private extern bool InternalHasFlag(Enum flags);
@@ -794,7 +859,10 @@ namespace System
             // For BitFlags (indicated by the Flags custom attribute): If for each bit that is set in the value there is a corresponding constant
             //(a pure power of 2), then the  OR string (ie "Red | Yellow") is returned. Otherwise, if the value is zero or if you can't create a string that consists of
             // pure powers of 2 OR-ed together, you return a hex value
-            return Enum.InternalFormat((RuntimeType)GetType(), GetValue());
+
+
+            // Try to see if its one of the enum values, then we return a String back else the value
+            return Enum.InternalFormat((RuntimeType)GetType(), ToUInt64()) ?? GetValue().ToString();
         }
         #endregion
 
@@ -844,20 +912,25 @@ namespace System
 
         #region Public Methods
         public String ToString(String format) {
+            char formatCh;
             if (format == null || format.Length == 0)
-                format = "G";
+                formatCh = 'G';
+            else if (format.Length != 1)
+                throw new FormatException(Environment.GetResourceString("Format_InvalidEnumFormatSpecification"));
+            else
+                formatCh = format[0];
 
-            if (String.Compare(format, "G", StringComparison.OrdinalIgnoreCase) == 0)
+            if (formatCh == 'G' || formatCh == 'g')
                 return ToString();
 
-            if (String.Compare(format, "D", StringComparison.OrdinalIgnoreCase) == 0)
+            if (formatCh == 'D' || formatCh == 'd')
                 return GetValue().ToString();
 
-            if (String.Compare(format, "X", StringComparison.OrdinalIgnoreCase) == 0)
-                return InternalFormattedHexString(GetValue());
+            if (formatCh == 'X' || formatCh == 'x')
+                return InternalFormattedHexString();
 
-            if (String.Compare(format, "F", StringComparison.OrdinalIgnoreCase) == 0)
-                return InternalFlagsFormat((RuntimeType)GetType(), GetValue());
+            if (formatCh == 'F' || formatCh == 'f')
+                return InternalFlagsFormat((RuntimeType)GetType(), ToUInt64()) ?? GetValue().ToString();
 
             throw new FormatException(Environment.GetResourceString("Format_InvalidEnumFormatSpecification"));
         }
index 76dd619..3655082 100644 (file)
@@ -324,6 +324,11 @@ namespace System.Reflection
             MetadataEnumResult tkCustomAttributeTokens;
             scope.EnumCustomAttributes(targetToken, out tkCustomAttributeTokens);
 
+            if (tkCustomAttributeTokens.Length == 0)
+            {
+                return Array.Empty<CustomAttributeRecord>();
+            }
+
             CustomAttributeRecord[] records = new CustomAttributeRecord[tkCustomAttributeTokens.Length];
 
             for (int i = 0; i < records.Length; i++)
index b6c52b2..2cf1c02 100644 (file)
@@ -3919,17 +3919,9 @@ namespace System
             if (!(valueType.IsEnum || IsIntegerType(valueType)))
                 throw new ArgumentException(Environment.GetResourceString("Arg_MustBeEnumBaseTypeOrEnum"), "value");
 
-            ulong[] ulValues = Enum.InternalGetValues(this);
             ulong ulValue = Enum.ToUInt64(value);
-            int index = Array.BinarySearch(ulValues, ulValue);
 
-            if (index >= 0)
-            {
-                string[] names = Enum.InternalGetNames(this);
-                return names[index];
-            }
-
-            return null;
+            return Enum.GetEnumName(this, ulValue);
         }
         #endregion