Devirtualise Enum.ToString() / prevent boxing

[benaadams]

enum MyEnum { One, Two, Three, Four, Five, Six, Seven, Eight, Nine, Ten };
// ...
(MyEnum.Seven).ToString();

Will box on the .ToString()

Could the following method be added to Enum

protected unsafe static string ToString<TEnum>(TEnum e, int size) where TEnum : struct
{
    ulong ul = 0;
    switch (size)
    {
        case 1:
        case 2:
        case 4:
            var i = JitHelpers.UnsafeEnumCast(e);
            if (Enum.GetUnderlyingType(typeof(TEnum)) == typeof(uint)
                || Enum.GetUnderlyingType(typeof(TEnum)) == typeof(ushort)
                || Enum.GetUnderlyingType(typeof(TEnum)) == typeof(byte))
                ul = (ulong)(uint)i;
            else
                ul = (ulong)i;
            break;
        case 8:
            ul = (ulong)JitHelpers.UnsafeEnumCastLong(e);
            break;
    }
    return Enum.InternalFormat((RuntimeType)typeof(TEnum), ul) ?? e.ToString();
}

Then when the enum types are compiled they can have a .ToString method as follows

public override string ToString() => Enum.ToString(this, sizeof(MyEnum));

Just an idea... (or jit something better)

[jamesqo]

Then when the enum types are compiled they can have a .ToString method as follows

This will add additional size to the assembly for every declaration of an enum (including internal ones that are never called ToString on), just to avoid boxing on one method that is mainly used for debugging purposes.

Maybe it would be a better idea to provide a new static method instead to avoid the assembly bloat.

public class Enum
{
    public static string ToString<TEnum>(TEnum e) where TEnum : struct
    {
        // ...
    }
}

or better yet

public static class EnumExtensions
{
    public static string ToString<TEnum>(this TEnum e) where TEnum : struct
    {
        // ...
    }
}

Not sure if the latter will get picked in overload resolution though.

[benaadams]

Can't apply the generic constraint to Enum so that would get applied to every struct; then you'd need to do a type check and throw for regular structs :(

[jamesqo]

@benaadams Not really an issue. Doing something similar as was done here (it adds a new method JitHelpers.ContainsReferences<T>) we can easily add a new method JitHelpers.IsEnum<T> which will be substituted with a constant expr for each generic instantation (ldc.i4.1/true or ldc.i4.0/false) and inlined. Then we can use it like this:

public static string ToString<TEnum>(this TEnum e) where TEnum : struct
{
    if (!JitHelpers.IsEnum<TEnum>())
         throw new ArgumentException(nameof(e));
}

This would avoid any overhead of typeof(TEnum).IsEnum which is currently not optimized.

[RussKeldorph]

@jkotas Could you weigh in on this? Is this a problem we would solve, and would we solve it in any of the proposed ways?

[jkotas]

This is same theme as https://github.com/dotnet/corefx/issues/1514.

The proposals to add APIs for performance reasons should include data about the benefits.

Enum.ToString() is relatively slow reflection based API. It is not as slow as before thanks to @benaadams improvements. Avoiding boxing may shave some cost from it, but it is unlikely to do a miracle because of short term boxing is relatively cheap. The result is likely still going to be pretty far from the ideal performance that you would get from switch (value) case MyEnum.One: return "One"; case MyEnum.Two: return "Two"; .... ; }.

I think it may be more interesting to look at optimizing this via IL-to-IL optimizer (or Roslyn code generating extension dotnet/roslyn#5561 once/if it becomes reality) that you can instruct to replace calls to Enum.ToString() with hand-crafted switch statement that avoids reflection if you are using Enum formatting on hot paths.

[RussKeldorph]

Unclear this is a codegen issue at this point.

[TylerBrinkley]

What I'm proposing in corefx#15453 would address this as a generic static ToString method would be added and after it's implemented roslyn could be updated to call this generic version instead.