Performance improvements for Enum.ToString()

[jackfree]

From Exchange performance analysis, Enum.ToString() is allocating a non-trivial amount of memory every time you call it, and it does not store/cache its values.

Turns out that InternalFormat in Enum.ToString() can just call RuntimeType.GetEnumName which does cache names and doesn't have extra allocations.

For those callers of the public API Type.GetEnumName, we can at least skip sorting and allocating twice (and only do it once) by saving the return values of a single call to GetEnumData.

Issue #3565

[dnfclas]

Hi @jackfree, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

[dnfclas]

@jackfree, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

[jkotas]

Could you please include a small test and numbers before/after that demonstrate the improvement? (see #2825 for example)

[jackfree]

@jkotas thanks for taking a look. Can you point me to an example test to start with? I see the link below for Issue #2825 for String.StartsWith, which includes some data for the new code added, but the commit does not include the code that generated the performance numbers, nor does it include any associated tests checked in for the brand new code.

I am not adding any new code but rather calling existing methods - are these existing codepaths missing functional and performance test coverage? It seems like a non-trivial burden for such a small change to call existing methods.

[jkotas]

The results in other PR are from the xunit performance test harness used for perf tests in this repo. You do not have to use it if you do not want to. I am just looking the simplest possible test that demonstrates the improvement.

I have tried the following simple test. It does not show any improvement with your change (~5350ms on x64 Windows release build both before and after your change). Could you please modify this test to demonstrate the improvement?

using System;

enum MyEnum { One, Two, Three, Four, Five, Six, Seven, Eight, Nine, Ten };

class My {
    static void Main() {
        int start = Environment.TickCount;
        for (int i = 0; i < 10000000; i++)
        {
            (MyEnum.Seven).ToString();
        }
        int end = Environment.TickCount;
        Console.WriteLine((end - start).ToString());
    }
}

[jackfree]

Thanks @jkotas -- that is a perfectly reasonable ask, I will do some minimal perf testing locally and provide some numbers.

Please note, however, that the original issue from Exchange was not about elapsed time, but rather allocations. Under load in the data center certain processes can see on the order of 1% of all allocations coming from Enum.ToString() calls. So the repro isn't trivial, and the issue wasn't time-taken.

Regarding the code change itself, does it look valid to you? Is there any reason not to avoid calling GetEnumData twice?

[jkotas]

the original issue from Exchange was not about elapsed time, but rather allocations

Allocations tend to affect elapsed time significantly in micro-benchmarks like the one above.

If the fix is reducing allocations in way that does not show up in micro-benchmark elapsed time, I am fine with allocation rate numbers measurements (e.g. see graphs in #3157)

Regarding the code change itself, does it look valid to you

There are multiple codepaths through Enum.ToString(). As far as I can tell, your change is on a rare codepath. I am not sure whether you can even hit this codepath, in particular on CoreCLR.

[redknightlois]

@jackfree if you write the benchmark using PerfDotNet you will be able to show the allocations difference quite easily (their latest release allows you to capture that too).

[mattwarren]

@jackfree @redknightlois here's a sample output from a BenchmarkDotNet run, including memory allocations:

The full code, results and complete output are available on this gist

[jkotas]

Enum.GetName then calls Type.GetEnumName
Ideally we want to call RuntimeType.GetEnumName

Type.GetEnumName is virtual method overriden by RuntimeType.GetEnumName. So this should be happening already.

[mattwarren]

Type.GetEnumName is virtual method overriden by RuntimeType.GetEnumName. So this should be happening already.

@jkotas I wondered if that was the case, thanks for clarifying.

I guess there must be another reason that my ToStringOptimised() method is slower than alloc-free code (although it is faster than a regular ToString() call).

[chrisaut]

Enum.GetName takes an object as its second argument, thus the value has to be boxed, no?

This explains the allocations I believe.

Perhaps a generic overload would help?

The optimized version just indexes into an array it seems, I think this will be hard to match in the generic case because one cannot just take the enum's value as an index, eg. I could have

enum MyEnum : long { One, Max = long.MaxValue, Min = long.MinValue }

A dictionary could work instead of the array I guess. Although if I followed the call chains correctly the implementation seems to do end up doing a binary search over a cached values/object array, which, assuming that most enums have a small number of values defined, is probably better than something more complex like a dictionary.

[mattwarren]

Enum.GetName takes an object as its second argument, thus the value has to be boxed, no? This explains the allocations I believe.

You're right, I completely missed the boxing, I need to re-install the Clr Heap Allocation Analyzer, so I can see the hidden allocations in VS.

Although if I followed the call chains correctly the implementation seems to do end up doing a binary search over a cached values/object array, which, assuming that most enums have a small number of values defined, is probably better than something more complex like a dictionary.

Yeah I see what you mean, it's this code that does that.

My benchmark wasn't meant to show a complete before/after, that can only happen when the changes in this PR have been applied. I was just trying to show what BenchmarkDotNet can do (I'm one of the authors the tool).

[benaadams]

Don't think this PR changes any behaviour as it already calls the correct method.

I have added PR #6645 which does change the allocations.

[jkotas]

Superseded by #6645.