[release/7.0] JIT: Simplify JitDisasm matching behavior and support namespaces/generics in release #75260
Conversation
This changes how the JIT matches method names and signatures for method
sets (e.g. JitDisasm). It also starts printing method instantiations for full method names
and makes references to types consistent in generic instantiations and the signature.
In addition it starts supporting generic instantiations in release too.
To do this, most of the type printing is moved to the JIT, which also aligns the output
between crossgen2 and the VM (there were subtle differences here, like spaces between generic type arguments).
More importantly, we (for the most part) stop relying on JIT-EE methods that are documented to only be for debug purposes.
The new behavior of the matching is the following:
* The matching behavior is always string based.
* The JitDisasm string is a space-separated list of patterns. Patterns can arbitrarily
contain both '*' (match any characters) and '?' (match any 1 character).
* The string matched against depends on characters in the pattern:
+ If the pattern contains a ':' character, the string matched against is prefixed by the class name and a colon
+ If the pattern contains a '(' character, the string matched against is suffixed by the signature
+ If the class name (part before colon) contains a '[', the class contains its generic instantiation
+ If the method name (part between colon and '(') contains a '[', the method contains its generic instantiation
For example, consider
```
namespace MyNamespace
{
public class C<T1, T2>
{
[MethodImpl(MethodImplOptions.NoInlining)]
public void M<T3, T4>(T1 arg1, T2 arg2, T3 arg3, T4 arg4)
{
}
}
}
new C<sbyte, string>().M<int, object>(default, default, default, default); // compilation 1
new C<int, int>().M<int, int>(default, default, default, default); // compilation 2
```
The full strings are:
Before the change:
```
MyNamespace.C`2[SByte,__Canon][System.SByte,System.__Canon]:M(byte,System.__Canon,int,System.__Canon)
MyNamespace.C`2[Int32,Int32][System.Int32,System.Int32]:M(int,int,int,int)
```
Notice no method instantiation and the double class instantiation, which seems like an EE bug. Also two different names are used for sbyte: System.SByte and byte.
After the change the strings are:
```
MyNamespace.C`2[byte,System.__Canon]:M[int,System.__Canon](byte,System.__Canon,int,System.__Canon)
MyNamespace.C`2[int,int]:M[int,int](int,int,int,int)
```
The following strings will match both compilations:
```
M
*C`2:M
*C`2[*]:M[*](*)
MyNamespace.C`2:M
```
The following will match only the first one:
```
M[int,*Canon]
MyNamespace.C`2[byte,*]:M
M(*Canon)
```
There is one significant change in behavior here, which is that I have removed the special case that allows matching class names without namespaces. In particular, today Console:WriteLine would match all overloads of System.Console.WriteLine, while after this change it will not match. However, with generalized wild cards the replacement is simple in *Console:WriteLine.
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsBackport of #74430 to release/7.0 Customer ImpactWithout this PR specifying methods via the
This PR fixes all of these items. The PR results in a change in behavior of how the matching for TestingTesting was done manually on crossgen2/runtime/SPMI scenarios by specifying various combinations for these environment variables. RiskWhile the change looks big there are minimal changes to main product scenarios. All of this code, except for the crossgen2 change, is only enabled when
|
|
cc @dotnet/jit-contrib @stephentoub PTAL @EgorBo |
Avoid using the same JIT-EE GUID as the main branch.
|
@jakobbotsch I presume this is a new, unique JIT-EE GUID in the release/7.0 branch? If so, we won't be able to use any existing SuperPMI collections on release/7.0 testing. We should probably manually kick off a SuperPMI collection on this branch after this change merges (and propagates to the internal mirror) to ensure we have a usable collection. |
Yep.
I'll make sure to do that. |
|
Signed off, approved.
@jakobbotsch I'm about to merge, so this is a friendly reminder to do the action requested above. |
Backport of #74430 to release/7.0
Customer Impact
Without this PR specifying methods via the
DOTNET_JitDisasmthat we enabled in release builds in .NET 7 is limited in the following ways:Additionally, the way that methods can be specified is inconsistent between debug/checked/release builds and between CG2/NAOT/runtime scenarios.
This PR fixes all of these items and aligns the behavior of debug/checked/release builds and CG2/NAOT/runtime scenarios to be the same.
The PR results in a change in behavior of how the matching for
JitDisasmworks, and how names are printed byJitDisasmSummary. Since these are new features we'd like to get the behavior change in from the beginning instead of taking a potential break in .NET 8, however small it might be.Testing
Testing was done manually on crossgen2/runtime/SPMI scenarios by specifying various combinations for these environment variables.
Risk
While the change looks big there are minimal changes to main product scenarios. All of this code, except for the crossgen2 change, is only enabled when
DOTNET_JitDisasm/DOTNET_JitDisasmSummaryare set. I have verified that these functions are otherwise not called in release builds. Therefore I view the risk as low.