[Java.Interop.Tools.JavaCallableWrappers] fix more places to use TypeDefinitionCache

[jonathanpeppers]

Reviewing code in JavaCallableWrapperGenerator, I found places we weren't using the supplied TypeDefinitionCache or IMetadataResolver. Thus we appeared to be calling TypeReference.Resolve() potentially on the same types.

For example, dotnet trace output of an incremental build of a dotnet new maui project:

41.65ms xamarin.android.cecil!Mono.Cecil.TypeReference.Resolve()

I created a new TypeDefinitionRocks.ResolveCached() method to be used everywhere instead. Resulting with:

23.89ms xamarin.android.cecil!Mono.Cecil.TypeReference.Resolve()

Additionally, I updated to places to use plain foreach loops instead of System.Linq.

Before:
1.03s xamarin.android.build.tasks!Xamarin.Android.Tasks.GenerateJavaStubs.RunTask()
After:
944.48ms xamarin.android.build.tasks!Xamarin.Android.Tasks.GenerateJavaStubs.RunTask()

I think this likely saves about ~50ms off incremental builds of a dotnet new maui project.

[jonpryor]

At what point do we decide that IMetadataResolver is required and not optional? I feel like we're playing "wack-a-mole" with callsites which should be providing a cache, but aren't, because we overlook them.

I think we should thus require IMetadataResolver instances, i.e. IMetadataResolver resolver instead of IMetadataResolver? resolver, and fix the fallout.

[jonpryor]

@jonpryor wrote:

I think we should thus require IMetadataResolver instances, i.e. IMetadataResolver resolver instead of IMetadataResolver? resolver, and fix the fallout.

…and yes, this would be a partial "API break", in that currently non-warning code may start producing warnings, and /warnaserror would produce errors. That's the point. We should now require the caches, because we know it results in significant performance improvements.

This raises followup questions, such as what to do with our existing "compatibility overloads" such as https://github.com/xamarin/java.interop/blob/f8d77faf55347a58030a694332ba97f0dee88246/src/Java.Interop.Tools.Cecil/Java.Interop.Tools.Cecil/TypeDefinitionRocks.cs#L10-L12

We can't remove them; that would be an ABI break.

We should instead update them to be be an error to use:

		[Obsolete ("Use the TypeDefinitionCache overload for better performance.", error:true)]
		public static TypeDefinition? GetBaseType (this TypeDefinition type) =>
			throw new NotSupportedException();

[jonpryor]

Draft commit message:

Context: b81cfbb9ab8efa647a3bfcc2b2b97a5f1b1fa71e

Reviewing code in `JavaCallableWrapperGenerator`, I found codepaths
where we weren't caching `TypeReference.Resolve()` calls *at all*,
e.g. `JavaNativeTypeManager.GetPrimitiveClass(TypeDefinition)`.
We thus appear to be calling `TypeReference.Resolve()` potentially on
the same types.

For example, `dotnet trace` output of an incremental build of a `dotnet new maui` project:

	41.65ms xamarin.android.cecil!Mono.Cecil.TypeReference.Resolve()

It appears that, in trying to make our maintenance lives easier by
preserving backward API compatibility with callers that couldn't
provide a `TypeDefinitionCache` or `IMetadataResolver` instance -- by
providing method overloads which took e.g.
`IMetadataResolver? resolver` parameters which could be `null` -- we
made our optimization and performance lives *harder*, because not
all codepaths which needed to use caching *were* using caching, as
they were overlooked.

As the `Java.Interop.Tools.*` assemblies are internal API, introduce
an *API break* while preserving *ABI*:

`IMetadataResolver` is now required.

Thus, previous/current "compatibility methods" which allow *not*
providing an `IMetadataResolver` instance such as:

	// old and busted
	partial class TypeDefinitionRocks {
	    [Obsolete ("Use the TypeDefinitionCache overload for better performance.")] 
	    public static TypeDefinition? GetBaseType (this TypeDefinition type) => GetBaseType (type, resolver: null); 
	}

will now become *errors* to use:

	// new hawtness
	partial class TypeDefinitionRocks {
	    [Obsolete ("Use the TypeDefinitionCache overload for better performance.", error:true)] 
	    public static TypeDefinition? GetBaseType (this TypeDefinition type) => throw new NotSupportedException ();
	}

This allows the C# compiler to help us audit our codebase, ensuring
that *all* codepaths which call `TypeReference.Resolve()` instead use
`IMetadataResolver.Resolve()`, including:

  * `JavaCallableWrapperGenerator.GetAnnotationsString()`
  * `JavaCallableWrapperGenerator.WriteAnnotations()`
  * `JavaNativeTypeManager.GetPrimitiveClass()`

Requiring caching results in:

	23.89ms xamarin.android.cecil!Mono.Cecil.TypeReference.Resolve()

Additionally, I updated two places to use plain `foreach` loops
instead of using System.Linq.

  * Before:

        1.03s xamarin.android.build.tasks!Xamarin.Android.Tasks.GenerateJavaStubs.RunTask()

  * After:

        944.48ms xamarin.android.build.tasks!Xamarin.Android.Tasks.GenerateJavaStubs.RunTask()

I think this likely saves about ~50ms off incremental builds of a
`dotnet new maui` project.