Runtime: Can the generic Activator.CreateInstance<T> overload be optimized?

Created on 12 Oct 2016  路  16Comments  路  Source: dotnet/runtime

Maybe instead of doing the same thing as the Type-based overload, we could simply forward to a generic method in JitHelpers whose IL gets substituted with initobj/newobj like here. Would seem to be worth it since this is an intrinsic method emitted by the C# compiler.

area-System.Reflection enhancement tenet-performance up-for-grabs

Most helpful comment

Resolved via dotnet/coreclr#25145 by @jkotas.

All 16 comments

The hard part of Activator.CreateInstance<T> is to make it call the right constructor. I do not think you can do it well without changing the JIT (especially in shared generic code).

https://github.com/dotnet/corert/issues/368 is related.

This got me thinking about optimising Activator.CreateInstance for generic classes with the "new()" type constraint.

It would be possible to optimise the usage of Activator.CreateInstance when calling the paramtereless constructor using dynamic code compilation and static fields in generic classes (similar to how Enumerable.Empty works).

I've hackup up a prototype along and a benchmark and it seems that that would produce a speed-up by a factor of 2 (and ~3 for structs) on my machine, although when talking about nanoseconds it's hardly much.

I envisage that the activator class would hold a version of my CachedGenericConstructor<T> class (below) as a private nested class and use that for parameterless construction.

IMPORTANT NOTE: I am using .NET 4.6.1 which might be different to .NET Core. I don't have access to .NET Core at this moment.

// * Summary *

Host Process Environment Information:
BenchmarkDotNet=v0.9.8.0
OS=Microsoft Windows NT 6.2.9200.0
Processor=Intel(R) Core(TM) i7-4710HQ CPU 2.50GHz, ProcessorCount=8
Frequency=2439993 ticks, Resolution=409.8372 ns, Timer=TSC
CLR=MS.NET 4.0.30319.42000, Arch=64-bit RELEASE [RyuJIT]
GC=Concurrent Server
JitModules=clrjit-v4.6.1586.0

Type=GenericTypeConstructor Mode=Throughput GarbageCollection=Concurrent Workstation

| Method | Median | StdDev |
| --- | --- | --- |
| UsingExisting_DefaultConstructor_Class | 83.6808 ns | 1.8544 ns |
| UsingExisting_DefaultConstructor_Struct | 76.5256 ns | 1.8457 ns |
| UsingExisting_CustomConstructor_Class | 86.0214 ns | 2.9979 ns |
| UsingDynamicCompilation_DefaultConstructor_Class | 43.1595 ns | 1.6310 ns |
| UsingDynamicCompilation_CustomConstructor_Class | 44.4120 ns | 0.9563 ns |
| UsingDynamicCompilation_DefaultConstructor_Struct | 27.8055 ns | 0.2270 ns |

// ** BenchmarkRunner: End **

Method names:

  • UsingExisting means "Activator.CreateInstance"
  • UsingDynamicCompilation means the proposed code
  • DefaultConstructor_Class is a class with a default constructor.
  • CustomConstructor_Class is a class with a non-default parameterless constructor.
  • DefaultConstructor_Struct is a struct.

The main logic is as follows

```c#
static class CachedGenericConstructor where T : new()
{
private static readonly Func _factory;

static CachedGenericConstructor()
{
    var type = typeof (T);
    var constructorInfo = type.GetConstructor(new Type[0]);
    if (constructorInfo != null)
    {
        Expression newExpression = Expression.New(constructorInfo);
        var lambdaExpression = Expression.Lambda<Func<T>>(newExpression);
        var function = lambdaExpression.Compile();
        _factory = function;
    }
    else if (type.IsValueType)
    {
        Expression defaultExpression = Expression.Default(type);
        var lambdaExpression = Expression.Lambda<Func<T>>(defaultExpression);
        var function = lambdaExpression.Compile();
        _factory = function;
    }
    else
    {
        throw new InvalidOperationException("Not sure that this is possible...");
    }
}

public static T Construct() => _factory();

}
```

We would not want to (and it is not even possible today) to take dependency on Linq expressions from CoreLib.

This optimization should be done in AOT-friendly way - without dynamic code generation.

I'll be brave and say that this is not worth the effort. And close it. Feel free to push back if you think otherwise.

@karelz have just bumped into this, and would like to suggest that this gets another look, specifically because Activator.CreateInstance<T>() gets emitted by the compiler when instantiation occurs on a generic type (which seems to be a relatively common factory pattern).

The benchmark below compares a simple non-generic instantiation (as a baseline) with a generic new T() and a compiled delegate approach as proposed above by @andrewjsaid, for both struct and class (thanks to @YohDeadfall who drew my attention to this in https://github.com/npgsql/npgsql/pull/2402).

The point of the compiled delegate isn't to suggest an implementation for the JIT (wrt to @jkotas's comment above), but rather to show what perf-conscious users must currently do to avoid the perf hit introduced by Activator.CreateInstance<T>(). An intrinsics approach such as https://github.com/dotnet/corert/issues/368 would also remove the needless allocation for value types, obviating https://github.com/dotnet/coreclr/issues/16621. However, I'm not aware of the complexity or difficulties in implementing something like this within the JIT, nor whether they would be worth the potential ten-fold perf improvement.

One last note:

Benchmark results

BenchmarkDotNet=v0.11.3, OS=ubuntu 18.10
Intel Xeon W-2133 CPU 3.60GHz, 1 CPU, 12 logical and 6 physical cores
.NET Core SDK=3.0.100-preview3-010431
  [Host]     : .NET Core 3.0.0-preview3-27503-5 (CoreCLR 4.6.27422.72, CoreFX 4.7.19.12807), 64bit RyuJIT
  DefaultJob : .NET Core 3.0.0-preview3-27503-5 (CoreCLR 4.6.27422.72, CoreFX 4.7.19.12807), 64bit RyuJIT


| Method | Mean | Error | StdDev | Median | Gen 0/1k Op | Gen 1/1k Op | Gen 2/1k Op | Allocated Memory/Op |
|--------------------------- |----------:|----------:|----------:|----------:|------------:|------------:|------------:|--------------------:|
| StructWithNew | 3.427 ns | 0.0463 ns | 0.0387 ns | 3.419 ns | - | - | - | - |
| StructWithGenericNew | 37.589 ns | 0.8184 ns | 1.7441 ns | 36.673 ns | 0.0055 | - | - | 24 B |
| StructWithCompiledDelegate | 4.035 ns | 0.0053 ns | 0.0047 ns | 4.035 ns | - | - | - | - |
| ClassWithNew | 5.041 ns | 0.0400 ns | 0.0374 ns | 5.028 ns | 0.0056 | - | - | 24 B |
| ClassWithGenericNew | 48.467 ns | 0.3867 ns | 0.3229 ns | 48.381 ns | 0.0055 | - | - | 24 B |
| ClassWithCompiledDelegate | 10.476 ns | 0.0339 ns | 0.0283 ns | 10.481 ns | 0.0055 | - | - | 24 B |

Benchmark code (BDN)

```c#
public class Instantiation
{
static readonly Instantiator StructInstantiator = new Instantiator();
static readonly Instantiator ClassInstantiator = new Instantiator();

[Benchmark]
public SomeStruct StructWithNew() => StructInstantiator.WithNewStruct();

[Benchmark]
public SomeStruct StructWithGenericNew() => StructInstantiator.WithGenericNew();

[Benchmark]
public SomeStruct StructWithCompiledDelegate() => StructInstantiator.WithCompiledDelegate();

[Benchmark]
public SomeClass ClassWithNew() => ClassInstantiator.WithNewClass();

[Benchmark]
public SomeClass ClassWithGenericNew() => ClassInstantiator.WithGenericNew();

[Benchmark]
public SomeClass ClassWithCompiledDelegate() => ClassInstantiator.WithCompiledDelegate();

}

public class Instantiator where T : new()
{
static readonly Func CompiledDelegate = Expression
.Lambda>(Expression.New(typeof(T)))
.Compile();

public SomeStruct WithNewStruct() => new SomeStruct();
public SomeClass WithNewClass() => new SomeClass();
public T WithGenericNew() => new T();
public T WithCompiledDelegate() => CompiledDelegate();

}

public struct SomeStruct
{
public int A;
public int B;
}

public class SomeClass
{
public int A;
public int B;
}
```

I let @jkotas to comment if you changed his mind. This is way out of my area of expertise ...
cc @adamsitnik @steveharter @GrabYourPitchforks

I had hoped that https://github.com/dotnet/corefx/issues/24390 would be a way to address this. Essentially, you could ask the runtime for a delegate which acts as a fact factory for some type _T_, then you could cache that delegate and keep calling it as needed.

It would be certainly better to do this without runtime codegen (i.e. without using Reflection.Emit or equivalent) using the technique used by .NET Native. This requires change in the JIT that only a very few folks are able to do.

We have given up to large degree on making CoreCLR to be low footprint and AOT friendly runtime, so dependency on Reflection.Emit from Activator.CreateInstance would not prohibitive. Anybody interested in submitting a PR? The implementation should basically emit the delegate that @GrabYourPitchforks described using Reflection.Emit and cache in a static.

TBH I don't think we'd need ref emit. Eventually a reflection-based instantiation turns into a call to AllocateObject. If we can perform all of the Type checks up front and cache the pointer that needs to be passed to this method, we could make a very fast implementation of GetUninitializedObject. A fast factory would then wrap a call to this followed by a calli call to the object ctor.

Right, this is pretty close what the ultimate implementation with the help of the JIT would do. It would still need some new intrinsics and such. If we took that route, I think it is better to do the proper AOT-friendly fix that does what you have described as part of JITing the method.

I wrote a scratch yesterday, but haven't tested it or even compiled. It's near the same @GrabYourPitchforks said, so I can take the issue and will open a PR later.

As @jkotas wrote in https://github.com/dotnet/coreclr/issues/6843#issuecomment-241566379, this code doesn't support default constructors for value types, but will fix it.

public static class Activator
{
    private static readonly MethodInfo s_createInstanceBoxedMethod = new Func<int>(CreateInstanceBoxed).Method.GetGenericMethodDefinition();
    private static object CreateInstanceBoxed<T>(bool nonPublic, bool wrapExceptions) =>
        CreateInstance<T>(nonPublic, wrapExceptions);

    public static object CreateInstance(Type type) => CreateInstance(type, nonPublic);

    public static object CreateInstance(Type type, bool nonPublic)
    {
        if (type == null)
            throw new ArgumentNullException(nameof(type));

        // This methods should be cached like it's going now
        return ((Func<object>)s_createInstanceBoxedMethod.MakeGenericMethod(type).CreateDelegate(typeof(Func<object>)));
    }

    public static T CreateInstance<T>() => CreateInstance<T>(false, true);

    private static T CreateInstance<T>(bool nonPublic, bool wrapExceptions)
    {
        if (ActivatorCahce<T>.HasElementType)
            throw new MissingMethodException(SR.Arg_NoDefCTor);

        if (ActivatorCahce<T>.IsValueType)
            return default;

        if (ActivatorCache<T>.HasPublicConstructor || nonPublic)
        {
            object instance = RuntimeTypeHandle.Allocate(ActivatorCache<T>.Type);

            try
            {
                ActivatorCache<T>.Constructor(instance);
            }
            catch (Exception exception) when (wrapExceptions)
            {
                throw new TargetInvocationException(exception);
            }

            return instance;
        }

        throw new MissingMethodException(SR.Arg_NoDefCTor);
    }

    private static class ActivatorCache
    {
        private static readonly ConstructorInfo s_delegateCtorInfo = typeof(CtorDelegate).GetConstructor(new [] { typeof(object), typeof(IntPtr) });

        public static CtorDelegate CreateConstructor(ConstructorInfo ctorInfo)
        {
            RuntimeMethodHandle ctorHandle = ctorInfo.RuntimeMethodHandle;
            return s_delegateCtorInfo.Invoke(null, RuntimeMethodHandle.GetFunctionPointer(ctorHandle));
        }
    }

    private static class ActivatorCache<T>
    {
        public static readonly RuntimeType Type;
        public static readonly bool IsValueType;
        public static readonly bool HasElementType;
        public static readonly bool HasPublicConstructor;
        public static readonly CtorDelegate Constructor;

        static ActivatorCache()
        {
            Type = (RuntimeType)typeof(T);
            IsValueType =Type.IsValueType;
            HasElementType =Type.HasElementType;

            if (IsValueType || HasElementType)
            {
                return;
            }

            ConstructorInfo runtimeCtorInfo = Type.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, new [] { Type }, null);

            if (runtimeCtorInfo != null)
            {
                HasPublicConstructor = runtimeCtorInfo.IsPublic;
                Constructor = ActivatorCache.CreateConstructor(runtimeCtorInfo);
            }
        }
    }
}

We have given up to large degree on making CoreCLR to be low footprint and AOT friendly runtime

Does it means CoreRT dead? Are there any roadmaps or plans for releasing CoreRT?

The code shared between runtimes should stay to be AOT-friendly. My note applies to non-shared code that is specific to CoreCLR.

Are there any roadmaps or plans for releasing CoreRT?

https://github.com/dotnet/corert/issues/7200

Thank you

Resolved via dotnet/coreclr#25145 by @jkotas.

Fixed by dotnet/coreclr#25145

Was this page helpful?
0 / 5 - 0 ratings

Related issues

noahfalk picture noahfalk  路  3Comments

omariom picture omariom  路  3Comments

yahorsi picture yahorsi  路  3Comments

matty-hall picture matty-hall  路  3Comments

bencz picture bencz  路  3Comments