-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Add ability to filter interpreted methods by assembly #116569
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
This change adds support for assembly name to the methodset stuff so that it can be used to select interpreted methods or methods to generate interpreter compilation dump for. To preserve parity, I've made the same change to the JIT's version of the same functionality. This change is a prerequisity for an upcoming change that enables running coreclr tests interpreted.
cc: @dotnet/jit-contrib |
Tagging subscribers to this area: @BrzVlad, @janvorli, @kg |
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 extends the method filtering and printing logic in both the JIT and interpreter to recognize and optionally include assembly names (using the !
delimiter) before class and method names. Key changes include:
- Introducing an
m_containsAssemblyName
flag and parsing!
in methodset initializers. - Extending
eePrintMethod
,AppendMethodName
, andPrintMethodName
APIs to takeincludeAssembly
andincludeClass
parameters. - Updating call sites in JIT config, EE interface, and interpreter to propagate the new flags.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/coreclr/jit/jitconfig.h | Added m_containsAssemblyName to record presence of an assembly delimiter |
src/coreclr/jit/jitconfig.cpp | Parsed ! and set m_containsAssemblyName ; passed flag into eePrintMethod |
src/coreclr/jit/eeinterface.cpp | Extended eePrintMethod signature and logic to conditionally print assemblies |
src/coreclr/jit/compiler.h | Added new parameters includeAssembly and includeClass to eePrintMethod |
src/coreclr/interpreter/naming.cpp | Updated AppendString and AppendMethodName to support assembly and class flags |
src/coreclr/interpreter/methodset.cpp | Parsed ! , set m_containsAssemblyName , passed flags into PrintMethodName |
src/coreclr/interpreter/interpretershared.h | Added m_containsAssemblyName to interpreter MethodName struct |
src/coreclr/interpreter/compiler.h | Added includeAssembly /includeClass to interpreter PrintMethodName signature |
src/coreclr/interpreter/compiler.cpp | Passed includeAssembly /includeClass through interpreter compiler flows |
Comments suppressed due to low confidence (5)
src/coreclr/jit/jitconfig.h:25
- [nitpick] Consider adding a comment to explain the purpose and expected usage of
m_containsAssemblyName
, similar to the other boolean flags in this struct.
bool m_containsAssemblyName;
src/coreclr/interpreter/interpretershared.h:85
- [nitpick] It would be helpful to document
m_containsAssemblyName
here, describing how it controls assembly-based filtering in the interpreter.
bool m_containsAssemblyName;
src/coreclr/jit/jitconfig.cpp:53
- Add unit tests for JIT method filtering that include and exclude methods based on assembly names to verify the new flag behaves correctly.
name->m_containsAssemblyName = (exclamation != nullptr);
src/coreclr/interpreter/methodset.cpp:53
- Introduce tests for the interpreter
MethodSet
to ensure methods are filtered properly when an assembly name prefix is present.
name->m_containsAssemblyName = (exclamation != nullptr);
src/coreclr/jit/eeinterface.cpp:251
- [nitpick] Update this comment to mention that
clsHnd
may beNO_CLASS_HANDLE
whenincludeClass
is false, for clarity.
// clsHnd - Handle for the owning class.
So in order to specifiy assembly the format would be |
These docs should be updated:
It would be |
No, only the assembly name would be there, so e.g. to specify all functions in one assembly, it would be |
@jakobbotsch good point, I'll update the docs. |
This change adds support for assembly name to the methodset stuff so that it can be used to select interpreted methods or methods to generate interpreter compilation dump for.
To preserve parity, I've made the same change to the JIT's version of the same functionality.
This change is a prerequisity for an upcoming change that enables running coreclr tests interpreted.