-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve performance of Activator.CreateInstance #32520
Merged
GrabYourPitchforks
merged 75 commits into
dotnet:master
from
GrabYourPitchforks:fast_createinstance
Nov 26, 2020
Merged
Changes from 73 commits
Commits
Show all changes
75 commits
Select commit
Hold shift + click to select a range
d0bde83
Fast object instantiation, take two
GrabYourPitchforks 67539c2
Merge remote-tracking branch 'origin/master' into fast_createinstance…
GrabYourPitchforks edf3858
PR feedback
GrabYourPitchforks c5ae79f
Re-enable analyzers in project
GrabYourPitchforks e09e91b
Don't run COM tests on non-Windows
GrabYourPitchforks 17a3bde
PR feedback
GrabYourPitchforks 2467fdd
Fix mergeMerge remote-tracking branch 'origin/master' into fast_creat…
GrabYourPitchforks eb368eb
Cleanup + PR feedback
GrabYourPitchforks b85fb74
Merge remote-tracking branch 'origin/master' into fast_createinstance
GrabYourPitchforks faeef8c
Remove 'unwrapNullable' parameter
GrabYourPitchforks bdb403a
Quick GetDefaultCtor helper
GrabYourPitchforks c173fda
Remove unmanaged Allocate / CreateInstance
GrabYourPitchforks 5c52acd
Fix GetDefaultConstructor signature
GrabYourPitchforks a1c362c
Fix AV in allocator
GrabYourPitchforks 3ce9c41
Fix indentation
GrabYourPitchforks 904b25d
Fix COM instantiation
GrabYourPitchforks 1f6434a
Remove COM allocator special-case
GrabYourPitchforks c1ecb07
Fix mono build break; quick code cleanup
GrabYourPitchforks a071262
Rename GetNewobjHelperFnPtr -> GetAllocatorFtn
GrabYourPitchforks dc417fd
Cleanup in the allocator routines
GrabYourPitchforks 6904495
Fix linker failures, add asserts
GrabYourPitchforks 05c35a8
Remove unwanted CreateInstanceDefaultCtor overload
GrabYourPitchforks 3d8515a
Use WeakRef in ActivatorCache
GrabYourPitchforks 5acef41
Reintroduce COM specialization
GrabYourPitchforks adfae42
Allow runtime to create unboxing stub on my behalf
GrabYourPitchforks 49a8de1
Fix linker warning
GrabYourPitchforks 98c7941
Update error conditions
GrabYourPitchforks 0a443c2
Friendlier exception messages when activation fails
GrabYourPitchforks 3b34ae6
Cleanup usings
GrabYourPitchforks 6e1b7cd
Move friendly exception messages to common place
GrabYourPitchforks b588d5d
Fix TIE wrappers
GrabYourPitchforks 55aa128
Update unit tests
GrabYourPitchforks 0ef275c
Call CreateInstanceCheckThis for better exception handling
GrabYourPitchforks 4c1f5dd
PR feedback + fix failing coreclr unit tests
GrabYourPitchforks 97f5f60
Fix build breaks
GrabYourPitchforks 867bdc0
Merge remote-tracking branch 'levib/fast_createinstance' into fast_cr…
GrabYourPitchforks 2e44013
Fix strings.resx whitespace
GrabYourPitchforks 9c267fd
Fix build breaks on mono & non-Windows
GrabYourPitchforks bead4c6
PR feedback
GrabYourPitchforks 8f12dd8
Fix GC hole in AllocateComObject
GrabYourPitchforks 3a2021d
Refactor GetActivationInfo
GrabYourPitchforks 46ed0b8
Refactor RunClassConstructor
GrabYourPitchforks d3bc632
Update GetUninitializedObject to call the new APIs
GrabYourPitchforks 3f362f2
Hook up ActivatorCache to new system
GrabYourPitchforks 879937b
Fix typo causing build break
GrabYourPitchforks 93bcedb
Change GetActivationInfo to QCALL
GrabYourPitchforks e04401f
Simplify some call sites
GrabYourPitchforks 9887e9a
Merge remote-tracking branch 'origin/master' into fast_createinstance…
GrabYourPitchforks 186fd83
Fix failing tests
GrabYourPitchforks 4d3d9db
Fix bad assert
GrabYourPitchforks 036cbf0
Add managed _AllocateComObject member to make FCall checks happier
GrabYourPitchforks 882b63e
Avoid using GetEEFuncEntryPoint
GrabYourPitchforks 0b0c8db
Fix build error in ecalllist.h
GrabYourPitchforks c50d287
Update COM invocation unit tests
GrabYourPitchforks e2235cc
Merge remote-tracking branch 'origin/master' into fast_createinstance…
GrabYourPitchforks cebbf48
Allow propagation of PNSE in RuntimeHandles
GrabYourPitchforks d373fe7
Merge remote-tracking branch 'origin/master' into fast_createinstance…
GrabYourPitchforks 12b4578
Remove _AllocateComObject sentinel
GrabYourPitchforks a7fa617
PR feedback
GrabYourPitchforks 3e9438c
PR feedback - simplify GetActivationInfo out params
GrabYourPitchforks 9c20d84
Merge remote-tracking branch 'origin/master' into fast_createinstance…
GrabYourPitchforks 4b38b6c
Add missing ifdef around _AllocateComInterop
GrabYourPitchforks 9ed2b9b
Final inspection + code cleanup
GrabYourPitchforks 0ebebd6
Fix compilation error on non-Windows
GrabYourPitchforks 374cf34
Code cleanup & PR feedback
GrabYourPitchforks 6b23817
Merge remote-tracking branch 'origin/master' into fast_createinstance…
GrabYourPitchforks 11be754
More code cleanup
GrabYourPitchforks 5d43a91
Rename ActivatorCache source file
GrabYourPitchforks 0635399
Remove unnecessary asserts, underscores, indentation
GrabYourPitchforks 9ac3c3f
Fix [DynamicallyAccessedMembers] annotations
GrabYourPitchforks a06ef73
Fix bad assert
GrabYourPitchforks 1b1261f
Update src/coreclr/src/vm/reflectioninvocation.cpp
GrabYourPitchforks 94f8614
PR feedback
GrabYourPitchforks c73e9fa
Update src/coreclr/src/vm/reflectioninvocation.cpp
jkotas 359a588
Remove incorrect assert in ActivatorCache
GrabYourPitchforks File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
142 changes: 142 additions & 0 deletions
142
src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.ActivatorCache.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,142 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using System.Diagnostics; | ||
using System.Diagnostics.CodeAnalysis; | ||
using System.Runtime.CompilerServices; | ||
|
||
namespace System | ||
{ | ||
internal sealed partial class RuntimeType | ||
{ | ||
/// <summary> | ||
/// A cache which allows optimizing <see cref="Activator.CreateInstance"/>, | ||
/// <see cref="RuntimeType.CreateInstanceDefaultCtor"/>, and related APIs. | ||
/// </summary> | ||
private sealed unsafe class ActivatorCache | ||
{ | ||
// The managed calli to the newobj allocator, plus its first argument (MethodTable*). | ||
// In the case of the COM allocator, first arg is ComClassFactory*, not MethodTable*. | ||
private readonly delegate*<void*, object?> _pfnAllocator; | ||
private readonly void* _allocatorFirstArg; | ||
|
||
// The managed calli to the parameterless ctor, taking "this" (as object) as its first argument. | ||
private readonly delegate*<object?, void> _pfnCtor; | ||
private readonly bool _ctorIsPublic; | ||
|
||
#if DEBUG | ||
private readonly RuntimeType _originalRuntimeType; | ||
#endif | ||
|
||
internal ActivatorCache(RuntimeType rt) | ||
{ | ||
Debug.Assert(rt != null); | ||
|
||
#if DEBUG | ||
_originalRuntimeType = rt; | ||
#endif | ||
|
||
// The check below is redundant since these same checks are performed at the | ||
// unmanaged layer, but this call will throw slightly different exceptions | ||
// than the unmanaged layer, and callers might be dependent on this. | ||
|
||
rt.CreateInstanceCheckThis(); | ||
|
||
try | ||
{ | ||
RuntimeTypeHandle.GetActivationInfo(rt, | ||
out _pfnAllocator!, out _allocatorFirstArg, | ||
out _pfnCtor!, out _ctorIsPublic); | ||
} | ||
catch (Exception ex) | ||
{ | ||
// Exception messages coming from the runtime won't include | ||
// the type name. Let's include it here to improve the | ||
// debugging experience for our callers. | ||
|
||
string friendlyMessage = SR.Format(SR.Activator_CannotCreateInstance, rt, ex.Message); | ||
switch (ex) | ||
{ | ||
case ArgumentException: throw new ArgumentException(friendlyMessage); | ||
case PlatformNotSupportedException: throw new PlatformNotSupportedException(friendlyMessage); | ||
case NotSupportedException: throw new NotSupportedException(friendlyMessage); | ||
case MethodAccessException: throw new MethodAccessException(friendlyMessage); | ||
case MissingMethodException: throw new MissingMethodException(friendlyMessage); | ||
case MemberAccessException: throw new MemberAccessException(friendlyMessage); | ||
} | ||
|
||
throw; // can't make a friendlier message, rethrow original exception | ||
} | ||
|
||
// Activator.CreateInstance returns null given typeof(Nullable<T>). | ||
|
||
if (_pfnAllocator == null) | ||
{ | ||
Debug.Assert(Nullable.GetUnderlyingType(rt) != null, | ||
"Null allocator should only be returned for Nullable<T>."); | ||
|
||
static object? ReturnNull(void* _) => null; | ||
_pfnAllocator = &ReturnNull; | ||
} | ||
|
||
// If no ctor is provided, we have Nullable<T>, a ctorless value type T, | ||
// or a ctorless __ComObject. In any case, we should replace the | ||
// ctor call with our no-op stub. The unmanaged GetActivationInfo layer | ||
// would have thrown an exception if 'rt' were a normal reference type | ||
// without a ctor. | ||
|
||
if (_pfnCtor == null) | ||
{ | ||
static void CtorNoopStub(object? uninitializedObject) { } | ||
_pfnCtor = &CtorNoopStub; // we use null singleton pattern if no ctor call is necessary | ||
|
||
Debug.Assert(_ctorIsPublic); // implicit parameterless ctor is always considered public | ||
} | ||
|
||
// We don't need to worry about invoking cctors here. The runtime will figure it | ||
// out for us when the instance ctor is called. For value types, because we're | ||
// creating a boxed default(T), the static cctor is called when *any* instance | ||
// method is invoked. | ||
} | ||
|
||
internal bool CtorIsPublic => _ctorIsPublic; | ||
|
||
[MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
internal object? CreateUninitializedObject(RuntimeType rt) | ||
{ | ||
// We don't use RuntimeType, but we force the caller to pass it so | ||
// that we can keep it alive on their behalf. Once the object is | ||
// constructed, we no longer need the reference to the type instance, | ||
// as the object itself will keep the type alive. | ||
|
||
#if DEBUG | ||
if (_originalRuntimeType != rt) | ||
{ | ||
Debug.Fail("Caller passed the wrong RuntimeType to this routine." | ||
+ Environment.NewLineConst + "Expected: " + (_originalRuntimeType ?? (object)"<null>") | ||
+ Environment.NewLineConst + "Actual: " + (rt ?? (object)"<null>")); | ||
} | ||
#endif | ||
|
||
object? retVal = _pfnAllocator(_allocatorFirstArg); | ||
GC.KeepAlive(rt); | ||
return retVal; | ||
} | ||
|
||
[MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
internal void CallConstructor(object? uninitializedObject) | ||
{ | ||
#if DEBUG | ||
if (uninitializedObject != null && !uninitializedObject.GetType().IsEquivalentTo(_originalRuntimeType)) | ||
{ | ||
Debug.Fail("Caller passed an unexpected 'this' parameter to ctor - possible type safety violation." | ||
+ Environment.NewLineConst + "Expected type: " + (_originalRuntimeType ?? (object)"<null>") | ||
+ Environment.NewLineConst + "Actual type: " + uninitializedObject.GetType()); | ||
} | ||
#endif | ||
|
||
_pfnCtor(uninitializedObject); | ||
} | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,12 +17,6 @@ | |
|
||
namespace System | ||
{ | ||
// this is a work around to get the concept of a calli. It's not as fast but it would be interesting to | ||
// see how it compares to the current implementation. | ||
// This delegate will disappear at some point in favor of calli | ||
|
||
internal delegate void CtorDelegate(object instance); | ||
|
||
// Keep this in sync with FormatFlags defined in typestring.h | ||
internal enum TypeNameFormatFlags | ||
{ | ||
|
@@ -3968,113 +3962,45 @@ private void CreateInstanceCheckThis() | |
return instance; | ||
} | ||
|
||
// the cache entry | ||
private sealed class ActivatorCache | ||
{ | ||
// the delegate containing the call to the ctor | ||
internal readonly RuntimeMethodHandleInternal _hCtorMethodHandle; | ||
internal MethodAttributes _ctorAttributes; | ||
internal CtorDelegate? _ctor; | ||
|
||
// Lazy initialization was performed | ||
internal volatile bool _isFullyInitialized; | ||
|
||
private static ConstructorInfo? s_delegateCtorInfo; | ||
|
||
internal ActivatorCache(RuntimeMethodHandleInternal rmh) | ||
{ | ||
_hCtorMethodHandle = rmh; | ||
} | ||
|
||
private void Initialize() | ||
{ | ||
if (!_hCtorMethodHandle.IsNullHandle()) | ||
{ | ||
_ctorAttributes = RuntimeMethodHandle.GetAttributes(_hCtorMethodHandle); | ||
|
||
// The default ctor path is optimized for reference types only | ||
ConstructorInfo delegateCtorInfo = s_delegateCtorInfo ??= typeof(CtorDelegate).GetConstructor(new Type[] { typeof(object), typeof(IntPtr) })!; | ||
|
||
// No synchronization needed here. In the worst case we create extra garbage | ||
GrabYourPitchforks marked this conversation as resolved.
Show resolved
Hide resolved
|
||
_ctor = (CtorDelegate)delegateCtorInfo.Invoke(new object?[] { null, RuntimeMethodHandle.GetFunctionPointer(_hCtorMethodHandle) }); | ||
} | ||
_isFullyInitialized = true; | ||
} | ||
|
||
public void EnsureInitialized() | ||
{ | ||
if (!_isFullyInitialized) | ||
Initialize(); | ||
} | ||
} | ||
|
||
/// <summary> | ||
/// The slow path of CreateInstanceDefaultCtor | ||
/// Helper to invoke the default (parameterless) constructor. | ||
/// </summary> | ||
private object? CreateInstanceDefaultCtorSlow(bool publicOnly, bool wrapExceptions, bool fillCache) | ||
[DebuggerStepThrough] | ||
[DebuggerHidden] | ||
internal object? CreateInstanceDefaultCtor(bool publicOnly, bool skipCheckThis, bool fillCache, bool wrapExceptions) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cleaning up these useless args is left for future PR? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah. Wanted to avoid causing churn in the mono codebase at the moment. |
||
{ | ||
RuntimeMethodHandleInternal runtimeCtor = default; | ||
bool canBeCached = false; | ||
bool hasNoDefaultCtor = false; | ||
// Get or create the cached factory. Creating the cache will fail if one | ||
// of our invariant checks fails; e.g., no appropriate ctor found. | ||
// | ||
// n.b. In coreclr we ignore 'skipCheckThis' (assumed to be false) | ||
// and 'fillCache' (assumed to be true). | ||
|
||
object instance = RuntimeTypeHandle.CreateInstance(this, publicOnly, wrapExceptions, ref canBeCached, ref runtimeCtor, ref hasNoDefaultCtor); | ||
if (hasNoDefaultCtor) | ||
if (GenericCache is not ActivatorCache cache) | ||
{ | ||
throw new MissingMethodException(SR.Format(SR.Arg_NoDefCTor, this)); | ||
cache = new ActivatorCache(this); | ||
GenericCache = cache; | ||
} | ||
|
||
if (canBeCached && fillCache) | ||
if (!cache.CtorIsPublic && publicOnly) | ||
{ | ||
// cache the ctor | ||
GenericCache = new ActivatorCache(runtimeCtor); | ||
throw new MissingMethodException(SR.Format(SR.Arg_NoDefCTor, this)); | ||
} | ||
|
||
return instance; | ||
} | ||
// Compat: allocation always takes place outside the try block so that OOMs | ||
// bubble up to the caller; the ctor invocation is within the try block so | ||
// that it can be wrapped in TIE if needed. | ||
|
||
/// <summary> | ||
/// Helper to invoke the default (parameterless) constructor. | ||
/// </summary> | ||
[DebuggerStepThrough] | ||
[DebuggerHidden] | ||
internal object? CreateInstanceDefaultCtor(bool publicOnly, bool skipCheckThis, bool fillCache, bool wrapExceptions) | ||
{ | ||
// Call the cached | ||
if (GenericCache is ActivatorCache cacheEntry) | ||
object? obj = cache.CreateUninitializedObject(this); | ||
try | ||
{ | ||
cacheEntry.EnsureInitialized(); | ||
|
||
if (publicOnly) | ||
{ | ||
if (cacheEntry._ctor != null && | ||
(cacheEntry._ctorAttributes & MethodAttributes.MemberAccessMask) != MethodAttributes.Public) | ||
{ | ||
throw new MissingMethodException(SR.Format(SR.Arg_NoDefCTor, this)); | ||
} | ||
} | ||
|
||
// Allocate empty object and call the default constructor if present. | ||
object instance = RuntimeTypeHandle.Allocate(this); | ||
GrabYourPitchforks marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Debug.Assert(cacheEntry._ctor != null || IsValueType); | ||
if (cacheEntry._ctor != null) | ||
{ | ||
try | ||
{ | ||
cacheEntry._ctor(instance); | ||
} | ||
catch (Exception e) when (wrapExceptions) | ||
{ | ||
throw new TargetInvocationException(e); | ||
} | ||
} | ||
|
||
return instance; | ||
cache.CallConstructor(obj); | ||
} | ||
catch (Exception e) when (wrapExceptions) | ||
{ | ||
throw new TargetInvocationException(e); | ||
} | ||
|
||
if (!skipCheckThis) | ||
CreateInstanceCheckThis(); | ||
|
||
return CreateInstanceDefaultCtorSlow(publicOnly, wrapExceptions, fillCache); | ||
return obj; | ||
} | ||
|
||
internal void InvalidateCachedNestedType() => Cache.InvalidateCachedNestedType(); | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this better than, say:
? I realize the above is probably a bit more IL, but it also means you won't catch and rethrow if the exception didn't match one of the special-cased types. I don't know how common that will be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not ignoring this, but leaving this thread marked unresolved for now because I think we can address it as part of #36194.