TypeDesc.GetMethods and TypeDesc.GetFields allocate too much

[MichalStrehovsky]

I'm seeing 260,000 allocations totaling 14.5 MB for a simple app. We really need to make these struct enumerators.

What we could probably do is:

• Make GetMethods and GetFields non-virtual.
• Add virtual methods internal int GetLimitMethodCookie(), internal int GetLimitFieldCookie(), internal MethodDesc GetNextMethod(int currentMethodCookie, out int nextMethodCookie), internal FieldDesc GetNextField(int currentFieldCookie, out int nextFieldCookie) to TypeDesc. These can be used to implement MethodCollection and FieldCollection struct and a struct enumerator for them
• Implement struct enumerator and collection.

The reason why the scheme is a bit more elaborate than e.g. a int MethodCount property with a MethodDesc GetMethodAtIndex(int index) is because in the native format metadata, indexing into a handle collection is a bit more involved due to the use of compressed integers. The API we need to add to the native format reader to enable this won't be pretty.

[MichalStrehovsky]

Related: dotnet/corefx#13624.

[jkotas]

I'm seeing 260,000 allocations totaling 14.5 MB for a simple app

This does not sound that bad to me. The GC is really good at getting rid of the short-lived garbage. I doubt that getting rid of these allocations will make anything materially faster.

What are the top call sites of the GetMethods/GetFields methods? I would expect most of them to come from resolving method and fields references - it may be interesting to special case it for the common cases. E,g. if the reference is ECMA metadata and the destination is ECMA metadata, we can avoid number of virtual calls and other overhead.

[MichalStrehovsky]

This does not sound that bad to me

The number of allocations is in the same bucket as what NameMangler produces with all the strings and ImmutableDictionaries - it basically overshadows everything else. Those two alone account for about a third of all allocations. Even if GC can do this efficiently, the best way to speed it up is to not give it any work. I still think it will be worth going after this.

A lot of the calls comes from the virtual method resolution logic.

[jkotas]

I agree that we should not be ignoring it, but introducing a custom IEnumerable equivalent would be choice of the last resort for me.

A lot of the calls comes from the virtual method resolution logic.

MethodDesc.GetMethod does not do what virtual resolution needs, and so it needs to enumerate all methods instead. If this is the top one, the fix should be to adjust MethodDesc.GetMethod so that it works for virtual method resolution.

[MichalStrehovsky]

the fix should be to adjust MethodDesc.GetMethod so that it works for virtual method resolution.

That will expose the problem with virtual methods injected by the type system and the philosophical conflict of GetMethod/GetMethods only ever operating on methods defined in the metadata.

[jkotas]

I think FindMatchingVirtualMethodOnTypeByNameAndSig can go through the same indirection as GetAllMethods so that there is a place to inject any extra ones if necessary.

(I assume that the bottleneck is coming from https://github.com/dotnet/corert/blob/488486065d532cf8d6b3e5bb8fcf787f7f0da483/src/Common/src/TypeSystem/Common/MetadataVirtualMethodAlgorithm.cs#L280.)