Skip to content

Limit impact of reflectable Invoke methods#83066

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.

Sign up for GitHub

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

Closed
MichalStrehovsky wants to merge 1 commit into from
Closed

Limit impact of reflectable Invoke methods #83066

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -271,8 +271,20 @@ protected override void GetMetadataDependenciesDueToReflectability(ref Dependenc
MethodDesc invokeMethod = type.GetMethod("Invoke", null);
if (!IsReflectionBlocked(invokeMethod))
{
// Size optimization: try to downgrade to metadata-only if possible
bool isActuallyInvokable = !type.IsGenericDefinition;
if (isActuallyInvokable)
{
isActuallyInvokable &= !invokeMethod.Signature.ReturnType.IsByRefLike;

@jkotasjkotasMar 7, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work once #45152 is fixed. POR is to fix that in .NET 8 together with other reflection invoke related functionality and perf issues. cc @steveharter

@MichalStrehovskyMichalStrehovskyMar 7, 2023

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The good news is that the delegate I was after with this (something with a SpanAction) seems to have disappeared with the other PRs I did around #83069 so this is no longer needed. It would still be a size savings, not an interesting one.

This was only interesting because the SpanAction delegate happened to also have ValueTuple in the signature that calls default equality comparer and brings a ton of stuff into hello world.

foreach (TypeDesc paramType in invokeMethod.Signature)
isActuallyInvokable &= !paramType.IsByRefLike;
}

dependencies ??= new DependencyList();
dependencies.Add(factory.ReflectedMethod(invokeMethod), "Delegate invoke method is always reflectable");
if (isActuallyInvokable)
dependencies.Add(factory.ReflectedMethod(invokeMethod), "Delegate invoke method is always reflectable");
else
dependencies.Add(factory.MethodMetadata(invokeMethod.GetTypicalMethodDefinition()), "Delegate invoke method is always reflectable");
}
}

Expand Down