-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Report folded method bodies in the MSTAT file #117459
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
Conversation
So that we have this for statistics or visualizations when needed.
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.
Pull Request Overview
This PR adds support for reporting folded method bodies in the MSTAT output to improve downstream statistics and visualizations.
- Introduces a
GetName
API onIDependencyNode
and implements it in core framework types. - Adds a
ReportFoldedNode
hook to the object-dumping infrastructure and invokes it in the object writer. - Enhances
MstatObjectDumper
to collect, encode, and emit deduplicated method-body mappings.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/IDependencyNode.cs | Added GetName method to the dependency-node interface |
src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyNodeCore.cs | Explicit interface implementation for the new GetName method |
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ReachabilityInstrumentationProvider.cs | Provided a GetName implementation on UntrackedSymbol |
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/ObjectWriter.cs | Reports folded symbol nodes via ReportFoldedNode |
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDumper.cs | Added default ReportFoldedNode in the dumper base class |
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MstatObjectDumper.cs | Tracks and emits deduplicated method-body data |
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/IObjectDumper.cs | Declared new ReportFoldedNode in dumper interface |
Comments suppressed due to low confidence (4)
src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/IDependencyNode.cs:44
- [nitpick] Consider adding an XML doc comment for the new
GetName
method to explain its purpose and expected return value.
string GetName(DependencyContextType context);
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDumper.cs:19
- [nitpick] Add XML documentation to explain when and why
ReportFoldedNode
is called and how implementations should handle it.
public virtual void ReportFoldedNode(NodeFactory factory, ObjectNode originalNode, ISymbolNode targetNode) { }
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/IObjectDumper.cs:11
- [nitpick] Consider adding an XML doc comment to the interface method to describe its contract and potential side effects.
void ReportFoldedNode(NodeFactory factory, ObjectNode originalNode, ISymbolNode targetNode);
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MstatObjectDumper.cs:183
- New logic for collecting and encoding deduplicated method bodies should be covered by a unit or integration test to verify the MSTAT output includes the expected
DeduplicatedMethods
section.
var deduplicatedMethods = new InstructionEncoder(new BlobBuilder());
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ReachabilityInstrumentationProvider.cs
Outdated
Show resolved
Hide resolved
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas |
…yInstrumentationProvider.cs Co-authored-by: Copilot <[email protected]>
@sbomer is this something you'd be comfortable reviewing? |
/ba-g wasm deadletters |
So that we have this for statistics or visualizations when needed.
Cc @dotnet/ilc-contrib