Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support C# Function Pointers? #666

Open
jonpryor opened this issue Jun 19, 2020 · 1 comment
Open

Support C# Function Pointers? #666

jonpryor opened this issue Jun 19, 2020 · 1 comment
Labels
proposal Issue raised for discussion, we do not even know if the change would be desirable yet

Comments

@jonpryor
Copy link
Member

Context: https://github.com/dotnet/csharplang/blob/master/proposals/function-pointers.md
Context: dotnet/runtime#32963

There is a proposal to add function pointers to the C# language. This would allow obtaining a pointer to a C# static method and hand it to native code without going through Delegates, and would be more efficient.

By @jonpryor's reading of the spec, In order to create a function pointer which can be handed off to native code, the method needs to have a [NativeCallableAttribute] declaration, but:

Methods marked by [[NativeCallable]] attribute are only callable from native code, not managed (can’t call methods, create a delegate, etc …).

This restriction means that we can't use the existing JNINativeWrapper.CreateDelegate() approach, which is used as part of exception marshaling, e.g.

https://github.com/xamarin/java.interop/blob/ef1d37b00ca7f84f9d07e057857af938816205a3/tests/generator-Tests/Tests-Core/expected.ji/Android.Text.ISpannable.cs#L70

The "easiest" way to support C# function pointers, if/when they ever exist, will be to integrate support into jnimarshalmethod-gen, such that instead of emitting

partial class __<$>_jni_marshal_methods {
    public static IntPtr FuncIJavaObject (IntPtr __jnienv, IntPtr __this) {}

    public static void __RegisterNativeMembers (JniNativeMethodRegistrationArguments args)
    {
        args.AddRegistrations (new JniNativeMethodRegistration[]{
                new JniNativeMethodRegistration ("funcIJavaObject",
                    "()Ljava/lang/Object;",
                    new Func<IntPtr, IntPtr, IntPtr> (FuncIJavaObject)),});
    }
}

We would emit "equivalent" code which uses [NativeCallable] and function pointers:

partial class __<$>_jni_marshal_methods {
    [NativeCallable]
    public static IntPtr FuncIJavaObject (IntPtr __jnienv, IntPtr __this) {}

    public static void __RegisterNativeMembers (JniNativeMethodRegistrationArguments args)
    {
        delegate*<IntPtr, IntPtr> fp = &FuncIJavaObject;
        args.AddRegistrations (new JniNativeMethodRegistration[]{
                new JniNativeMethodRegistration ("funcIJavaObject",
                    "()Ljava/lang/Object;",
                    (IntPtr) fp),});
    }
}

Note that for the above to work, we'd need to add a JniNativeMethodRegistration(string, string, IntPtr) constructor, which may be difficult to do without an ABI break, as that type only exposes fields, not properties, and we can't change the size of that struct:

https://github.com/xamarin/java.interop/blob/master/src/Java.Interop/Java.Interop/JniNativeMethodRegistration.cs

I doubt anybody is currently using this struct, so it might not be a problem to break ABI, but if it is, we'll need to think of a workaround.

@jonpryor jonpryor added the enhancement Proposed change to current functionality label Jun 19, 2020
@radekdoulik
Copy link
Member

Alternatively we might try to modify our current code which uses the System.Linq.Expression and generate the IL with ldftn instructions ourselves. It would need a bit of investigation whether it is possible and how to do it.

jonpryor added a commit to jonpryor/java.interop that referenced this issue Jan 4, 2022
Context: 926e4bc
Context: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-9.0/function-pointers
Context? dotnet#666

Commit 926e4bc allowed `jnienv-gen` to emit multiple different
JNIEnv invocation strategies at the same time, allowing
`tests/invocation-overhead` to try them "all at once" for side-by-
side comparisons.

Add support for JNIEnv invocation strategy which relies on
C#9 Function Pointers, a'la:

	partial struct JNIEnv {
	    public  delegate* unmanaged <IntPtr /* env */, jobject> ExceptionOccurred;
	}
	partial class JniEnvironment {
	    partial class Exceptions {
	        public static unsafe JniObjectReference ExceptionOccurred ()
	        {
	            IntPtr __env = JniEnvironment.EnvironmentPointer;
	            var tmp = (*((JNIEnv**)__env))->ExceptionOccurred (__env);
	            return new JniObjectReference (tmp, JniObjectReferenceType.Local);
	        }
	    }
	}

This *should* allow for performance better than "JIPinvokeTiming",
as it avoids P/Invoke overheads to a set of `java_interop_*` C
functions (926e4bc), while *also* avoiding the overheads involved
with using `Marshal.GetDelegateForFunctionPointer()` as used by
"JIIntPtrs".

…but it doesn't:

	$ JI_JVM_PATH=$HOME/android-toolchain/jdk-11/lib/jli/libjli.dylib dotnet tests/invocation-overhead/bin/Debug/net6.0/invocation-overhead.dll
	# SafeTiming timing: 00:00:04.2123508
	#	Average Invocation: 0.00042123508ms
	# XAIntPtrTiming timing: 00:00:02.1625501
	#	Average Invocation: 0.00021625500999999998ms
	# JIIntPtrTiming timing: 00:00:02.3620239
	#	Average Invocation: 0.00023620239ms
	# JIPinvokeTiming timing: 00:00:01.8993587
	#	Average Invocation: 0.00018993587ms
	# JIFunctionPointersTiming timing: 00:00:02.0278083
	#	Average Invocation: 0.00020278083ms

(Compare and contrast with 926e4bc, circa 2015!)

Of particular note is that the Average Invocation time for
JIFunctionPointersTiming takes 7% longer than JIPinvokeTiming.

We may not continue investigation of C#9 Function Pointers
for `JNIEnv` binding purposes, but we can preserve this code for
future investigation.
jonpryor added a commit that referenced this issue Jan 5, 2022
Context: 926e4bc
Context: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-9.0/function-pointers
Context? #666

Commit 926e4bc allowed `jnienv-gen` to emit multiple different
JNIEnv invocation strategies at the same time, allowing
`tests/invocation-overhead` to try them "all at once" for side-by-
side comparisons.

Add support for a new JNIEnv invocation strategy which relies on
C#9 Function Pointers, a'la:

	partial struct JNIEnv {
	    public  delegate* unmanaged <IntPtr /* env */, jobject> ExceptionOccurred;
	}
	partial class JniEnvironment {
	    partial class Exceptions {
	        public static unsafe JniObjectReference ExceptionOccurred ()
	        {
	            IntPtr __env = JniEnvironment.EnvironmentPointer;
	            var tmp = (*((JNIEnv**)__env))->ExceptionOccurred (__env);
	            return new JniObjectReference (tmp, JniObjectReferenceType.Local);
	        }
	    }
	}

This *could* allow for performance better than "JIPinvokeTiming",
as it avoids P/Invoke overheads to a set of `java_interop_*` C
functions (926e4bc), while *also* avoiding the overheads involved
with using `Marshal.GetDelegateForFunctionPointer()` as used by
"JIIntPtrs".

…but it doesn't necessarily provide better performance:

	$ JI_JVM_PATH=$HOME/android-toolchain/jdk-11/lib/jli/libjli.dylib dotnet tests/invocation-overhead/bin/Debug/net6.0/invocation-overhead.dll
	# SafeTiming timing: 00:00:04.2123508
	#	Average Invocation: 0.00042123508ms
	# XAIntPtrTiming timing: 00:00:02.1625501
	#	Average Invocation: 0.00021625500999999998ms
	# JIIntPtrTiming timing: 00:00:02.3620239
	#	Average Invocation: 0.00023620239ms
	# JIPinvokeTiming timing: 00:00:01.8993587
	#	Average Invocation: 0.00018993587ms
	# JIFunctionPointersTiming timing: 00:00:02.0278083
	#	Average Invocation: 0.00020278083ms

(Compare and contrast with 926e4bc, circa 2015!)

Of particular note is that the Average Invocation time for
JIFunctionPointersTiming takes 7% longer than JIPinvokeTiming.

Though that's slightly reversed when a *Release* build of
`invocation-overhead.dll` is used:

	% JI_JVM_PATH=$HOME/android-toolchain/jdk-11/lib/jli/libjli.dylib dotnet tests/invocation-overhead/bin/Release/net6.0/invocation-overhead.dll
	# SafeTiming timing: 00:00:03.4128431
	#	Average Invocation: 0.00034128431000000003ms
	# XAIntPtrTiming timing: 00:00:01.8857456
	#	Average Invocation: 0.00018857455999999999ms
	# JIIntPtrTiming timing: 00:00:01.9075412
	#	Average Invocation: 0.00019075412ms
	# JIPinvokeTiming timing: 00:00:01.6993644
	#	Average Invocation: 0.00016993643999999998ms
	# JIFunctionPointersTiming timing: 00:00:01.6561349
	#	Average Invocation: 0.00016561349ms

With a Release build, the Average Invocation time for
JIFunctionPointersTiming takes 97% of the time as JIPinvokeTiming,
i.e. is 3% faster.

We may or may not continue investigation of C#9 Function Pointers
for `JNIEnv` binding purposes.  We will preserve this code for
future investigation.
@jpobst jpobst added proposal Issue raised for discussion, we do not even know if the change would be desirable yet and removed enhancement Proposed change to current functionality labels Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal Issue raised for discussion, we do not even know if the change would be desirable yet
Projects
None yet
Development

No branches or pull requests

3 participants