Proposal: call `partial` methods within method bindings

[jonpryor]

Context: dotnet/runtime#82121

Background: When should the GC run? If a "large" object is allocated, should that change the GC heuristics for when it runs?

If someone allocates a large Bitmap, should we GC more often on devices with low memory?

We've had questions around this for quite some time, but the only "reasonable" way to alter GC heuristics was by using GC.AddMemoryPressure() and GC.RemoveMemoryPressure(), but Mono didn't support these methods, so it was all moot: there was no way to inform the GC about "implicit" non-managed memory allocations, so we didn't.

That is now changing with dotnet/runtime#82121

Meaning there is now a way to tell our GC about implicit non-managed memory usage!

Background: Binding: Consider the current (abbreviated) binding for Android.Grpahics.Bitmap:

namespace Android.Graphics {
	public sealed partial class Bitmap {
		internal Bitmap (IntPtr javaReference, JniHandleOwnership transfer) : base (javaReference, transfer)
		{
		}

		public unsafe int AllocationByteCount {
			get {
				const string __id = "getAllocationByteCount.()I";
				try {
					var __rm = _members.InstanceMethods.InvokeAbstractInt32Method (__id, this, null);
					return __rm;
				} finally {
				}
			}
		}

		public static unsafe Android.Graphics.Bitmap? CreateBitmap (Android.Graphics.Bitmap src)
		{
			const string __id = "createBitmap.(Landroid/graphics/Bitmap;)Landroid/graphics/Bitmap;";
			try {
				JniArgumentValue* __args = stackalloc JniArgumentValue [1];
				__args [0] = new JniArgumentValue ((src == null) ? IntPtr.Zero : ((global::Java.Lang.Object) src).Handle);
				var __rm = _members.StaticMethods.InvokeObjectMethod (__id, __args);
				return global::Java.Lang.Object.GetObject<Android.Graphics.Bitmap> (__rm.Handle, JniHandleOwnership.TransferLocalRef);
			} finally {
				global::System.GC.KeepAlive (src);
			}
		}
	}
}

What would we like to do? We'd like to "alter" certain methods so that we can "reasonably" tell the GC "this instance has x bytes of associated memory".

How do we do that?

Proposal: Update generator to emit C# partial methods for each bound member and constructor, allowing partial classes to be written which allow "extra work" to be done when the bound method is executed.

Open question: How many partial methods should we have, and where should they be called from?

• One, before/after the JNI call? (Which one?)
• Two, on member entry & exit?
• What should the name convention be? "Obvious" is _On prefix + member name + some kind of suffix. What about overloads? Should we care about overloads, or have all overloads invoke the same set of partial methods?

How many such partial methods do we need, anyway? While trying to write up the following example, it occurs to me that, while having Bitmap.CreateBitmap() call GC.AddMemoryPresure() "makes sense", it doesn't really make sense becaus it calls Object.GetObject<Bitmap>(), which means the Bitmap(IntPtr, JniHandleOwnership) constructor will be invoked. This suggests we may only need one such partial method here, for constructors, and all other members aren't needed for this use case.

Example: Bitmap with partial methods!

namespace Android.Graphics {
	public sealed partial class Bitmap {
		partial void _OnConstructor ();

		internal Bitmap (IntPtr javaReference, JniHandleOwnership transfer) : base (javaReference, transfer)
		{
			_OnConstructor ();
		}

		partial void _OnAllocationByteCount_Entry ();
		partial void _OnAllocationByteCount_Exit (int ret);

		public unsafe int AllocationByteCount {
			get {
				const string __id = "getAllocationByteCount.()I";
				try {
					_OnAllocationByteCount_Entry ();
					var __rm = _members.InstanceMethods.InvokeAbstractInt32Method (__id, this, null);
					var __ret = __rm;
					_OnAllocationByteCount_Exit (__ret);
					return __ret;
				} finally {
				}
			}
		}

		partial void _OnCreateBitmap_Entry (Android.Graphics.Bitmap src);
		partial void _OnCreateBitmap_Exit (Android.Graphics.Bitmap ret);

		public static unsafe Android.Graphics.Bitmap? CreateBitmap (Android.Graphics.Bitmap src)
		{
			_OnCreateBitmap_Entry (src);
			const string __id = "createBitmap.(Landroid/graphics/Bitmap;)Landroid/graphics/Bitmap;";
			try {
				JniArgumentValue* __args = stackalloc JniArgumentValue [1];
				__args [0] = new JniArgumentValue ((src == null) ? IntPtr.Zero : ((global::Java.Lang.Object) src).Handle);
				var __rm = _members.StaticMethods.InvokeObjectMethod (__id, __args);
				var __ret = global::Java.Lang.Object.GetObject<Android.Graphics.Bitmap> (__rm.Handle, JniHandleOwnership.TransferLocalRef);
				_OnCreateBitmap_Exit (__ret);
				return __ret;
			} finally {
				global::System.GC.KeepAlive (src);
			}
		}
	}
}

Once such partial methods exist, we can "reasonably" update Mono.Android.dll to implement required partial methods to Do Something Useful:

partial class Bitmap {

	partial void _OnConstructor ()
	{
		GC.AddMemoryPressure (ByteCount);
	}

	protected override void Dispose (bool disposing)
	{
		if (Handle != IntPtr.Zero) {
			GC.RemoveMemoryPressure (ByteCount);
		}
	}
}

Meta-question: is overriding Dispose() "sufficient enough" in practice for calling GC.RemoveMemoryPressure()? Or could this result in constantly increasing memory pressure?

[jpobst]

Putting aside whether or not calling GC.AddMemoryPresure() for Bitmap is a good idea (:man_shrugging:), it feels like there may not be a big enough need to warrant a generator change for this.

Could the Bitmap case be covered with some handwritten binding code instead?

Are there a non-trivial amount of other classes that would want to call GC.AddMemoryPresure()?

What would we like to do? We'd like to "alter" certain methods so that we can "reasonably" tell the GC "this instance has x bytes of associated memory".

Note that GC.AddMemoryPresure() does not take an object, so it does not tie memory to an object instance. It simply updates a global "extra memory somewhere" number.

[jonpryor]

@jpobst asked:

Could the Bitmap case be covered with some handwritten binding code instead?

Depends on how things play out. If we wanted to use the Bitmap(IntPtr, JniHandleOwnership) constructor, as demo'd above, then no, as there is no way to prevent that constructor from being emitted.

If we instead use things like Bitmap.CreateBitmap(), then yes, we could use metadata to rename that method and "replace" the public method. This in turn suggests "just replace every method that returns Bitmap", which sounds plausible until we realize that there are 85 such methods:

% grep 'return="android.graphics.Bitmap"' bin/BuildDebug/api/api-33.xml.in | wc -l
      85

Eep?

I suspect that a partial method for the "wrapper" constructor may be "enough", and should be fairly straightforward to implement (Famous Last Words™), and that it isn't necessary to provide partial methods for every bound member.

[jpobst]

I think this is a very niche need, so if we implement it I think it should require opting in via metadata so we don't generate the partial for every class.

Something like:

<attr path="package[@name='android.graphics']/class[@name='Bitmap']" name="constructorPartial">true</attr>