Skip to content

Commit

Permalink
Streamline default ctor path in reflection on custom attributes (dotn…
Browse files Browse the repository at this point in the history
…et/coreclr#27451)

Saves some allocations and removes a few FCalls/QCalls

Commit migrated from dotnet/coreclr@0a62070
  • Loading branch information
jkotas authored Oct 28, 2019
1 parent cc42a80 commit e1dbc54
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 142 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1124,7 +1124,7 @@ private static bool IsCustomAttributeDefined(
{
if (FilterCustomAttributeRecord(car[i].tkCtor, in scope,
decoratedModule, decoratedMetadataToken, attributeFilterType, mustBeInheritable, ref derivedAttributes,
out _, out _, out _, out _))
out _, out _, out _))
return true;
}
}
Expand Down Expand Up @@ -1186,7 +1186,7 @@ private static void AddCustomAttributes(
if (!FilterCustomAttributeRecord(caRecord.tkCtor, in scope,
decoratedModule, decoratedMetadataToken, attributeFilterType!, mustBeInheritable,
ref derivedAttributes,
out RuntimeType attributeType, out IRuntimeMethodInfo? ctor, out bool ctorHasParameters, out bool isVarArg))
out RuntimeType attributeType, out IRuntimeMethodInfo? ctorWithParameters, out bool isVarArg))
{
continue;
}
Expand All @@ -1197,13 +1197,13 @@ private static void AddCustomAttributes(
// Create custom attribute object
int cNamedArgs;
object attribute;
if (ctorHasParameters)
if (ctorWithParameters != null)
{
attribute = CreateCaObject(decoratedModule, attributeType, ctor!, ref blobStart, blobEnd, out cNamedArgs);
attribute = CreateCaObject(decoratedModule, attributeType, ctorWithParameters, ref blobStart, blobEnd, out cNamedArgs);
}
else
{
attribute = RuntimeTypeHandle.CreateCaInstance(attributeType, ctor);
attribute = attributeType.CreateInstanceDefaultCtor(publicOnly: false, skipCheckThis: true, fillCache: true, wrapExceptions: false);

// It is allowed by the ECMA spec to have an empty signature blob
int blobLen = (int)((byte*)blobEnd - (byte*)blobStart);
Expand Down Expand Up @@ -1304,12 +1304,10 @@ private static bool FilterCustomAttributeRecord(
bool mustBeInheritable,
ref RuntimeType.ListBuilder<object> derivedAttributes,
out RuntimeType attributeType,
out IRuntimeMethodInfo? ctor,
out bool ctorHasParameters,
out IRuntimeMethodInfo? ctorWithParameters,
out bool isVarArg)
{
ctor = null;
ctorHasParameters = false;
ctorWithParameters = null;
isVarArg = false;

// Resolve attribute type from ctor parent token found in decorated decoratedModule scope
Expand All @@ -1334,29 +1332,21 @@ private static bool FilterCustomAttributeRecord(
// Resolve the attribute ctor
ConstArray ctorSig = scope.GetMethodSignature(caCtorToken);
isVarArg = (ctorSig[0] & 0x05) != 0;
ctorHasParameters = ctorSig[1] != 0;
bool ctorHasParameters = ctorSig[1] != 0;

if (ctorHasParameters)
{
// Resolve method ctor token found in decorated decoratedModule scope
// See https://github.com/dotnet/coreclr/issues/21456 for why we fast-path non-generics here (fewer allocations)
if (attributeType.IsGenericType)
{
ctor = decoratedModule.ResolveMethod(caCtorToken, attributeType.GenericTypeArguments, null)!.MethodHandle.GetMethodInfo();
ctorWithParameters = decoratedModule.ResolveMethod(caCtorToken, attributeType.GenericTypeArguments, null)!.MethodHandle.GetMethodInfo();
}
else
{
ctor = ModuleHandle.ResolveMethodHandleInternal(decoratedModule.GetNativeHandle(), caCtorToken);
ctorWithParameters = ModuleHandle.ResolveMethodHandleInternal(decoratedModule.GetNativeHandle(), caCtorToken);
}
}
else
{
// Resolve method ctor token from decorated decoratedModule scope
ctor = attributeType.GetTypeHandleInternal().GetDefaultConstructor();

if (ctor == null && !attributeType.IsValueType)
throw new MissingMethodException(".ctor");
}

// Visibility checks
MetadataToken tkParent = new MetadataToken();
Expand Down Expand Up @@ -1398,11 +1388,11 @@ private static bool FilterCustomAttributeRecord(
RuntimeTypeHandle attributeTypeHandle = attributeType.TypeHandle;

bool result = RuntimeMethodHandle.IsCAVisibleFromDecoratedType(JitHelpers.GetQCallTypeHandleOnStack(ref attributeTypeHandle),
ctor != null ? ctor.Value : RuntimeMethodHandleInternal.EmptyHandle,
ctorWithParameters != null ? ctorWithParameters.Value : RuntimeMethodHandleInternal.EmptyHandle,
JitHelpers.GetQCallTypeHandleOnStack(ref parentTypeHandle),
JitHelpers.GetQCallModuleOnStack(ref decoratedModule)) != Interop.BOOL.FALSE;

GC.KeepAlive(ctor);
GC.KeepAlive(ctorWithParameters);
return result;
}
#endregion
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,6 @@ internal static bool HasElementType(RuntimeType type)
[MethodImpl(MethodImplOptions.InternalCall)]
internal static extern object CreateInstance(RuntimeType type, bool publicOnly, bool wrapExceptions, ref bool canBeCached, ref RuntimeMethodHandleInternal ctor, ref bool hasNoDefaultCtor);

[MethodImpl(MethodImplOptions.InternalCall)]
internal static extern object CreateCaInstance(RuntimeType type, IRuntimeMethodInfo? ctor);

[MethodImpl(MethodImplOptions.InternalCall)]
internal static extern object Allocate(RuntimeType type);

Expand Down Expand Up @@ -395,17 +392,6 @@ internal static MdUtf8String GetUtf8Name(RuntimeType type)
[MethodImpl(MethodImplOptions.InternalCall)]
internal static extern IRuntimeMethodInfo GetDeclaringMethod(RuntimeType type);

[DllImport(JitHelpers.QCall, CharSet = CharSet.Unicode)]
private static extern void GetDefaultConstructor(QCallTypeHandle handle, ObjectHandleOnStack method);

internal IRuntimeMethodInfo? GetDefaultConstructor()
{
IRuntimeMethodInfo? ctor = null;
RuntimeTypeHandle nativeHandle = GetNativeHandle();
GetDefaultConstructor(JitHelpers.GetQCallTypeHandleOnStack(ref nativeHandle), JitHelpers.GetObjectHandleOnStack(ref ctor));
return ctor;
}

[DllImport(JitHelpers.QCall, CharSet = CharSet.Unicode)]
private static extern void GetTypeByName(string name, bool throwOnError, bool ignoreCase, StackCrawlMarkHandle stackMark,
ObjectHandleOnStack assemblyLoadContext,
Expand Down
62 changes: 0 additions & 62 deletions src/coreclr/src/vm/customattribute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -665,68 +665,6 @@ FCIMPL5(VOID, Attribute::ParseAttributeArguments, void* pCa, INT32 cCa,
}
FCIMPLEND


FCIMPL2(Object*, RuntimeTypeHandle::CreateCaInstance, ReflectClassBaseObject* pCaTypeUNSAFE, ReflectMethodObject* pCtorUNSAFE) {
CONTRACTL {
FCALL_CHECK;
PRECONDITION(CheckPointer(pCaTypeUNSAFE));
PRECONDITION(!pCaTypeUNSAFE->GetType().IsGenericVariable());
PRECONDITION(pCaTypeUNSAFE->GetType().IsValueType() || CheckPointer(pCtorUNSAFE));
}
CONTRACTL_END;

struct _gc
{
REFLECTCLASSBASEREF refCaType;
OBJECTREF o;
REFLECTMETHODREF refCtor;
} gc;

gc.refCaType = (REFLECTCLASSBASEREF)ObjectToOBJECTREF(pCaTypeUNSAFE);
MethodTable* pCaMT = gc.refCaType->GetType().GetMethodTable();

gc.o = NULL;
gc.refCtor = (REFLECTMETHODREF)ObjectToOBJECTREF(pCtorUNSAFE);
MethodDesc *pCtor = gc.refCtor != NULL ? gc.refCtor->GetMethod() : NULL;

HELPER_METHOD_FRAME_BEGIN_RET_PROTECT(gc);
{
PRECONDITION(
(!pCtor && gc.refCaType->GetType().IsValueType() && !gc.refCaType->GetType().GetMethodTable()->HasDefaultConstructor()) ||
(pCtor == gc.refCaType->GetType().GetMethodTable()->GetDefaultConstructor()));

gc.o = pCaMT->Allocate();

if (pCtor)
{

ARG_SLOT args;

if (pCaMT->IsValueType())
{
MethodDescCallSite ctor(pCtor, &gc.o);
args = PtrToArgSlot(gc.o->UnBox());
ctor.CallWithValueTypes(&args);
}
else
{

PREPARE_NONVIRTUAL_CALLSITE_USING_METHODDESC(pCtor);
DECLARE_ARGHOLDER_ARRAY(CtorArgs, 1);
CtorArgs[ARGNUM_0] = OBJECTREF_TO_ARGHOLDER(gc.o);

// Call the ctor...
CALL_MANAGED_METHOD_NORET(CtorArgs);
}

}
}
HELPER_METHOD_FRAME_END();

return OBJECTREFToObject(gc.o);
}
FCIMPLEND

FCIMPL6(LPVOID, COMCustomAttribute::CreateCaObject, ReflectModuleBaseObject* pAttributedModuleUNSAFE, ReflectClassBaseObject* pCaTypeUNSAFE, ReflectMethodObject *pMethodUNSAFE, BYTE** ppBlob, BYTE* pEndBlob, INT32* pcNamedArgs)
{
FCALL_CONTRACT;
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/src/vm/customattribute.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,6 @@ class COMCustomAttribute
static FCDECL5(VOID, ParseAttributeUsageAttribute, PVOID pData, ULONG cData, ULONG* pTargets, CLR_BOOL* pInherited, CLR_BOOL* pAllowMultiple);
static FCDECL6(LPVOID, CreateCaObject, ReflectModuleBaseObject* pAttributedModuleUNSAFE, ReflectClassBaseObject* pCaTypeUNSAFE, ReflectMethodObject *pMethodUNSAFE, BYTE** ppBlob, BYTE* pEndBlob, INT32* pcNamedArgs);
static FCDECL7(void, GetPropertyOrFieldData, ReflectModuleBaseObject *pModuleUNSAFE, BYTE** ppBlobStart, BYTE* pBlobEnd, STRINGREF* pName, CLR_BOOL* pbIsProperty, OBJECTREF* pType, OBJECTREF* value);
static FCDECL4(VOID, GetSecurityAttributes, ReflectModuleBaseObject *pModuleUNSAFE, DWORD tkToken, CLR_BOOL fAssembly, PTRARRAYREF* ppArray);

private:

Expand Down
2 changes: 0 additions & 2 deletions src/coreclr/src/vm/ecalllist.h
Original file line number Diff line number Diff line change
Expand Up @@ -220,14 +220,12 @@ FCFuncEnd()

FCFuncStart(gCOMTypeHandleFuncs)
FCFuncElement("CreateInstance", RuntimeTypeHandle::CreateInstance)
FCFuncElement("CreateCaInstance", RuntimeTypeHandle::CreateCaInstance)
FCFuncElement("CreateInstanceForAnotherGenericParameter", RuntimeTypeHandle::CreateInstanceForGenericType)
QCFuncElement("GetGCHandle", RuntimeTypeHandle::GetGCHandle)

FCFuncElement("IsInstanceOfType", RuntimeTypeHandle::IsInstanceOfType)
FCFuncElement("GetDeclaringMethod", RuntimeTypeHandle::GetDeclaringMethod)
FCFuncElement("GetDeclaringType", RuntimeTypeHandle::GetDeclaringType)
QCFuncElement("GetDefaultConstructor", RuntimeTypeHandle::GetDefaultConstructor)
QCFuncElement("MakePointer", RuntimeTypeHandle::MakePointer)
QCFuncElement("MakeByRef", RuntimeTypeHandle::MakeByRef)
QCFuncElement("MakeSZArray", RuntimeTypeHandle::MakeSZArray)
Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/src/vm/ilmarshalers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3026,8 +3026,7 @@ ILSafeHandleMarshaler::ReturnOverride(

if (!pMT->HasDefaultConstructor())
{
MAKE_WIDEPTR_FROMUTF8(wzMethodName, COR_CTOR_METHOD_NAME);
COMPlusThrowNonLocalized(kMissingMethodException, wzMethodName);
COMPlusThrowNonLocalized(kMissingMethodException, COR_CTOR_METHOD_NAME_W);
}

// 2) prealloc a safehandle
Expand Down
56 changes: 22 additions & 34 deletions src/coreclr/src/vm/runtimehandles.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,21 @@ BOOL QCALLTYPE RuntimeMethodHandle::IsCAVisibleFromDecoratedType(
targetHandle.IsTypeDesc())
COMPlusThrowArgumentNull(NULL, W("Arg_InvalidHandle"));

if (pTargetCtor == NULL)
{
MethodTable* pTargetMT = targetHandle.AsMethodTable();

if (pTargetMT->HasDefaultConstructor())
{
pTargetCtor = pTargetMT->GetDefaultConstructor();
}
else
{
if (!pTargetMT->IsValueType())
COMPlusThrowNonLocalized(kMissingMethodException, COR_CTOR_METHOD_NAME_W);
}
}

bResult = CheckCAVisibilityFromDecoratedType(targetHandle.AsMethodTable(), pTargetCtor, sourceHandle.AsMethodTable(), sourceModuleHandle);
END_QCALL;

Expand Down Expand Up @@ -1158,45 +1173,18 @@ MethodDesc* QCALLTYPE RuntimeTypeHandle::GetInterfaceMethodImplementation(QCall:
TypeHandle typeHandle = pTypeHandle.AsTypeHandle();
TypeHandle thOwnerOfMD = pOwner.AsTypeHandle();

// Ok to have INVALID_SLOT in the case where abstract class does not implement an interface method.
// This case can not be reproed using C# "implements" all interface methods
// with at least an abstract method. b19897_GetInterfaceMap_Abstract.exe tests this case.
//@TODO:STUBDISPATCH: Don't need to track down the implementation, just the declaration, and this can
//@TODO: be done faster - just need to make a function FindDispatchDecl.
DispatchSlot slot(typeHandle.GetMethodTable()->FindDispatchSlotForInterfaceMD(thOwnerOfMD, pMD, FALSE /* throwOnConflict */));
// Ok to have INVALID_SLOT in the case where abstract class does not implement an interface method.
// This case can not be reproed using C# "implements" all interface methods
// with at least an abstract method. b19897_GetInterfaceMap_Abstract.exe tests this case.
//@TODO:STUBDISPATCH: Don't need to track down the implementation, just the declaration, and this can
//@TODO: be done faster - just need to make a function FindDispatchDecl.
DispatchSlot slot(typeHandle.GetMethodTable()->FindDispatchSlotForInterfaceMD(thOwnerOfMD, pMD, FALSE /* throwOnConflict */));
if (!slot.IsNull())
pResult = slot.GetMethodDesc();
pResult = slot.GetMethodDesc();

END_QCALL;

return pResult;
}

void QCALLTYPE RuntimeTypeHandle::GetDefaultConstructor(QCall::TypeHandle pTypeHandle, QCall::ObjectHandleOnStack retMethod)
{
QCALL_CONTRACT;

BEGIN_QCALL;

MethodDesc* pCtor = NULL;

TypeHandle typeHandle = pTypeHandle.AsTypeHandle();

if (!typeHandle.IsTypeDesc())
{
MethodTable* pMethodTable = typeHandle.AsMethodTable();
if (pMethodTable->HasDefaultConstructor())
pCtor = pMethodTable->GetDefaultConstructor();
}

if (pCtor != NULL)
{
GCX_COOP();
retMethod.Set(pCtor->GetStubMethodInfo());
}
END_QCALL;

return;
}

FCIMPL1(ReflectMethodObject*, RuntimeTypeHandle::GetDeclaringMethod, ReflectClassBaseObject *pTypeUNSAFE) {
Expand Down
5 changes: 0 additions & 5 deletions src/coreclr/src/vm/runtimehandles.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,6 @@ class RuntimeTypeHandle {
MethodDesc** pConstructor,
CLR_BOOL *pbHasNoDefaultCtor);

static FCDECL2(Object*, CreateCaInstance, ReflectClassBaseObject* refCaType, ReflectMethodObject* pCtorUNSAFE);

static
void QCALLTYPE MakeByRef(QCall::TypeHandle pTypeHandle, QCall::ObjectHandleOnStack retType);

Expand Down Expand Up @@ -191,9 +189,6 @@ class RuntimeTypeHandle {

static FCDECL1(ReflectMethodObject*, GetDeclaringMethod, ReflectClassBaseObject *pType);

static
void QCALLTYPE GetDefaultConstructor(QCall::TypeHandle pTypeHandle, QCall::ObjectHandleOnStack retMethod);

static FCDECL1(ReflectClassBaseObject*, GetDeclaringType, ReflectClassBaseObject* pType);
static FCDECL1(FC_BOOL_RET, IsValueType, ReflectClassBaseObject* pType);
static FCDECL1(FC_BOOL_RET, IsInterface, ReflectClassBaseObject* pType);
Expand Down

0 comments on commit e1dbc54

Please sign in to comment.