-
Notifications
You must be signed in to change notification settings - Fork 53
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
Labels
proposal
Issue raised for discussion, we do not even know if the change would be desirable yet
Comments
Alternatively we might try to modify our current code which uses the |
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
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
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: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 emittingWe would emit "equivalent" code which uses
[NativeCallable]
and function pointers: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.
The text was updated successfully, but these errors were encountered: