-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Conversation
…erformance. No public API additions were made.
Renamed EnumCache to EnumMembers. Renamed EnumBridge to EnumCache.
I've reduced the use of generics with an Enum type argument internally to just the following methods on public abstract Array GetValuesNonGeneric();
public abstract TEnum ToObject(ulong value);
public abstract object ToObjectNonGeneric(ulong value);
public abstract bool IsEnum(object value); Now, I'd like to merge the changes upstream into this branch which include the migration of |
The "Activator.CreateInstance(typeof(EnumBridge<,,>).MakeGenericType(enumType, underlyingType, typeof(UnderlyingOperations)))" call is still problematic. This call will generate a pile of code per enum type. This is bad for footprint in CoreCLR, and it does not work at all for Mono and CoreRT full AOT environments that this implementation is shared with now. Instead, there should be only 10 or so optimized implementations per the underlying type and there should be some unsafe code used to convert the enum to the underlying type. The implementation should not use |
@jkotas Thanks, here I thought using |
@jkotas I've removed the use of |
Yes, |
See the comment here: coreclr/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/TraceLogging/PropertyValue.cs Lines 204 to 206 in e6c49f7
The .NET Native reference is specifically in respect to how The other reference is part of reflection implementation. Reflection will have references to this API by definition. |
@MichalStrehovsky Thanks for explaining that. I'd love to get this implementation into .NET Core but I'm definitely out of my depth currently with the restrictions now imposed with sharing the implementation across AOT runtimes. Is there anyone that could help with this? Is the way this is implemented not feasible now that it's shared with AOT runtimes? Thanks. |
I do not think that the current implementation would work well for .NET Core either. The problem surfaces in different way in .NET Core as extra footprint: A lot more code gets JITed for each enum. I think the idea behind this optimization is good, it just needs to be implemented differently: #22161 (comment) . |
While it could be implemented without IsEnumUsed By
Non-Generic ImplementationType valueType = value.GetType();
if (valueType.IsEnum && valueType.IsEquivalentTo(enumType)) Generic Implementationvalue is TEnum; At least 1.7x performance improvement but probably much more as I don't have tests for just this method. ToObjectUsed By
Non-Generic ImplementationInternalBoxEnum(rt, uint64Value); Generic ImplementationTUnderlying underlying = default(TUnderlyingOperations).ToObject(uint64Value);
return Unsafe.As<TUnderlying, TEnum>(ref underlying); Approximately 3.0x performance improvement. GetValuesNon-Generic Implementationulong[] values = Enum.InternalGetValues(this);
Array ret = Array.CreateInstance(this, values.Length);
for (int i = 0; i < values.Length; i++)
{
object val = Enum.ToObject(this, values[i]);
ret.SetValue(val, i);
}
return ret; Generic ImplementationEnumCache<TUnderlying, TUnderlyingOperations> cache = EnumCache<TEnum, TUnderlying, TUnderlyingOperations>.Instance;
TUnderlying[] values = cache._values;
int nonNegativeStart = cache._nonNegativeStart;
int length = values.Length;
TEnum[] array = new TEnum[length];
for (int i = nonNegativeStart; i < length; ++i)
{
array[i - nonNegativeStart] = Unsafe.As<TUnderlying, TEnum>(ref values[i]);
}
int start = length - nonNegativeStart;
for (int i = 0; i < nonNegativeStart; ++i)
{
array[start + i] = Unsafe.As<TUnderlying, TEnum>(ref values[i]);
}
return array; Approximately 16-58x performance improvement depending on enum size. |
You are using the generic code clones as a cache. Producing this code (JITing, etc.) is expensive. The cost to populate this cache is large. Instead, you should be able to get similar performance improvements by caching data. The allocation of this data is cheap compared to producing the code. For example, the implementation of
There is a separate question on whether the performance of repeated calls to |
@jkotas Thanks for taking the time to explain that. I will adjust the implementation to use the non-generic implementations and re-benchmark and see where that puts us. |
Just re-benchmarked with the non-generic implementation and here are the main things.
|
…rically over the enum type.
I'm going to close this and re-open in the new dotnet/platform repo when that's done. |
From my experience with Enums.NET here is a generic implementation for enums.
No new API's have been added but this new implementation will easily allow adding new generic API's when approved which will provide even better performance.
The only known breaking changes involve the parameter name included in thrown
ArgumentException
s and methods that used to throw forfloat
,double
,IntPtr
, andUIntPtr
enums now work as would be expected.There may also be breaking changes between the order of thrown
Exception
s when there are multiple argument errors in a method call.Here are the benchmark results of this generic enum implementation.
Notes:
master
so these gains are already on top of the gains from Improve performance of Enum.{Try}Parse #21214.Compare
,Equals
, andHasFlag
are not included as their implementation was not changed for performance reasons.Summary:
GetValues
has a huge performance gain shown here from ~16-58x depending on enum size.IsDefined
has ~5.7x performance gain for contiguous enums and ~3.2-5x performance gain for non-contiguous enums depending on enum size.ToString
with a defined value has ~1.8-2.9x performance gain depending on enum size.Parse
of names has ~1.7x performance gain.Parse
of names has ~1.9x performance gain.Parse
of values has ~1.6x performance gain.Parse
of values has ~1.8x performance gain.ToObject(Object)
has ~2.3x performance gain.ToObject(Int32)
has ~3x performance gain.IConvertible.To*
methods have ~3.2x performance gain.Provides implementation for https://github.com/dotnet/corefx/issues/15453.