[Mono.Android] treat AllocObjectSupported as always true

[jonathanpeppers]

This removes an API < 10 code path, which should speed up all JNIEnv.StartCreateInstance / FinishCreateInstance calls by a very small amount. They no longer check a AllocObjectSupported field, which is now always true.

This also removes the need to pass in androidSdkVersion / android_api_level from the native side at startup.

We can also remove the _monodroid_get_android_api_level() p/invoke, which is unused.

[jonpryor]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jonpryor]

I'm not sure how much this change will actually help, as this code should effectively be "dead" with the death of generator --codegen-target=XamarinAndroid: dotnet/java-interop@d41793a

(i.e. only very very old bindings should be hitting these methods.)

Anything newer will be using JniPeerMembers, e.g.

https://github.com/dotnet/java-interop/blob/d30d55487ae11da10587e60a054bfb46f1e3510e/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs#L167-L169

See also e.g. https://github.com/dotnet/java-interop/blob/d30d55487ae11da10587e60a054bfb46f1e3510e/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteClass.txt#L24

or from our build tree:

// from src/Mono.Android/obj/Debug/net9.0/android-35/mcw/Java.Lang.Number.cs
namespace Java.Lang {

	public abstract partial class Number {
		public unsafe Number () : base (IntPtr.Zero, JniHandleOwnership.DoNotTransfer)
		{
			const string __id = "()V";

			if (((global::Java.Lang.Object) this).Handle != IntPtr.Zero)
				return;

			try {
				var __r = _members.InstanceMethods.StartCreateInstance (__id, ((object) this).GetType (), null);
				SetHandle (__r.Handle, JniHandleOwnership.TransferLocalRef);
				_members.InstanceMethods.FinishCreateInstance (__id, this, null);
			} finally {
			}
		}
	}
}

That said, we could probably remove the JniEnvironment.Runtime.NewObjectRequired check within StartCreateInstance().

[jonathanpeppers]

I was thinking of this as a "part 2" to:

• [monodroid] treat LocalRefsAreIndirect as always true #9138

After looking at this, it only would help a very small amount at startup (and maybe cleans up code). We'd avoid passing in the API level from Java.

Maybe it's worth removing JniEnvironment.Runtime.NewObjectRequired checks and make NewObjectRequired [Obsolete] as well?

[jonpryor]

@jonathanpeppers wrote:

Maybe it's worth removing JniEnvironment.Runtime.NewObjectRequired checks and make NewObjectRequired [Obsolete] as well?

Like this? dotnet/java-interop#1247

[jonpryor]

As per your comment on Discord:

Ok, I found maybe the size of JnienvInitializeArgs in C++ / C# didn't match
I removed from one and not the other

this means we should also add a public ulong struct_size field to the beginning of the struct, and assert that the size matches:

internal partial struct JnienvInitializeArgs {
    public static along ExpectedStructSize = IntPtr.Size == 8 ? XXX : YYY;
    public ulong struct_size;
    // …
}

Update the C++ side to be consistent:

In C++ land, set the new struct_size field in init_android_runtime():

struct JnienvInitializeArgs init = {};
init.struct_size = sizeof(JnienvInitializeArgs);

And finally in Initialize(), assert that things match:

if (args->struct_size != JnienvInitializeArgs.ExpectedStructSize) {
    FailFast(…);
}

While doing this, we may also want to reorder the struct members so that pointers are stored together, to minimize internal alignment padding.

[grendello]

sizeof() on the native size won't work as expected in this case. It will include padding of the structure which may differ depending on the target platform (even if bitness is the same). We'd have to calculate structure size as a sum of individual field sizes (a compile-time calculation), but then the managed size would have to compute it at run time, a bit of time loss.

However, perhaps we can just add a "serial number" field to the structure and bump it every time the structure changes, with comments to update both the managed and native sides at the same time?

[jonathanpeppers]

I added a comment to both structs. I think if I had seen that, I would have caught it earlier.