Fix DynamicInvokeMethodThunk hash code collisions

[filipnavara]

Fixes #103070

Let's see the CI if it breaks anything.

[MichalStrehovsky]

Alternative fix would be to go back to how we used to compute the name of these methods (it wasn't always a constant string), but I don't have a strong opinion. We don't otherwise override ComputeHashCode anywhere else. It's only virtual because of instantiated methods:

runtime/src/coreclr/tools/Common/TypeSystem/Common/MethodDesc.cs

Lines 493 to 495 in eb8f54d

	/// <summary>
	/// Compute HashCode. Should only be overridden by a MethodDesc that represents an instantiated method.
	/// </summary>

[jkotas]

Should only be overridden by a MethodDesc that represents an instantiated method.

Delete this comment since it is not valid anymore?

[filipnavara]

Delete this comment since it is not valid anymore?

I would like to keep some comment there.

There are runtime dependencies on the hash code both in NativeAOT and R2R. For NativeAOT it should mostly link to the same code in TypeHashingAlgorithms so the code is in sync with runtime implementation. For R2R there are places in unmanaged CoreCLR that do the XOR hash combining and that are not in any way linked to the Crossgen2 implementation even if they need to produce the same value.

Any suggestion for wording?

[MichalStrehovsky]

It might be easier to just add some suffix to the Name/DiagnosticName (e.g. number of parameters) so that we avert at least some of the collisions, drop the override, and call it good enough.

[jkotas]

Any suggestion for wording?

This hashcode is persisted into the image. The algorithm to compute it must be in sync with the one used at runtime.

It might be easier to just add some suffix to the Name/DiagnosticName (e.g. number of parameters)

Number of parameters is going to reduce the collisions like 3x (a lot of methods take one or two arguments). It would be still leaving a lot on the table.

[jkotas]

Thank you!