Skip to content
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

[mono] Save trimmed methods list to a file #97705

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

fanyang-mono
Copy link
Member

This should give the user a way to know which exact methods got trimmed.

@ghost ghost assigned fanyang-mono Jan 30, 2024
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jan 30, 2024
Copy link
Member

@lambdageek lambdageek left a comment

Choose a reason for hiding this comment

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

mostly looks ok, but I'm not sure the short method names are really what we want to put in the file.

Also, how will users discover these files?

src/tasks/MonoTargetsTasks/ILStrip/ILStrip.cs Show resolved Hide resolved
@@ -247,6 +250,7 @@ private static string ComputeGuid(MetadataReader mr)
int methodToken = MetadataTokens.GetToken(mr, mdefh);
MethodDefinition mdef = mr.GetMethodDefinition(mdefh);
int rva = mdef.RelativeVirtualAddress;
string methodName = mr.GetString(mdef.Name);
Copy link
Member

Choose a reason for hiding this comment

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

this isn't the full name of the method (ie: NameSpace.ClassName+NestedClassName<T,U>.SomeMethod<V,W>(Arg1 arg1, Arg2 arg2, ..., ArgN argN)). It's just SomeMethod. Is that what we want? The file might have 100 methods all named ToString or something.

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right. Do you know how to get the method full signature?

Copy link
Member

Choose a reason for hiding this comment

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

There's not an easy way :-(

you need to get the mdef.GetDeclaringType() which will be a TypeDefinitionHandle. then you need to get the TypeDefinition using mr.GetTypeDefiniton() which will have a Namespace and a Name property. And also you have to get its GetDeclaringType if it's a nested type, and so on until there's no more enclosing types.

That will at least get you enough so you can print SomeNameSpace.SomeClass.SomeEnclosingType.SomeMethodName which is better. (it woudl be good to also print the method signature in case we need to care about overloads, but that's harder)

@fanyang-mono
Copy link
Member Author

but I'm not sure the short method names are really what we want to put in the file.

Is there a way to get the full method signature?

how will users discover these files?

This functionality is not useful for the user as they don't have control over what to trim and what not to trim. This is added to help mono developers to debug potential future issues.

@fanyang-mono fanyang-mono added area-Codegen-AOT-mono and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants