dotnet / java-interop

Java.Interop provides open-source bindings of Java's Java Native Interface (JNI) for use with .NET managed languages such as C#
Other
189 stars 48 forks source link

[Java.Interop] Add JniMemberInfoLookup #1208

Open jonpryor opened 3 months ago

jonpryor commented 3 months ago

Context: c6c487b62dab4ffec45e61b09dd43afc89898caf Context: 312fbf439ed874bb5f4f25ee6d2c9a2b3c2f5a8b Context: 2197579478152fbc815eb15195977f808cd6bde4 Context: https://github.com/xamarin/xamarin-android/issues/7276

There is a desire to remove the "marshal-ilgen" component from .NET Android, which is responsible for all non-blittable type marshaling within P/Invoke (and related) invocations.

The largest source of such non-blittable parameter marshaling was with string marshaling: JNIEnv::GetFieldID() was "wrapped" by java_interop_jnienv_get_field_id:

JI_API jfieldID java_interop_jnienv_get_field_id (JNIEnv *env, jthrowable *_thrown, jclass type, const char* name, const char* signature);

which was P/Invoked within JniEnvironment.g.cs:

partial class NativeMethods {
    internal static extern unsafe IntPtr java_interop_jnienv_get_field_id (IntPtr jnienv, out IntPtr thrown, jobject type, string name, string signature);
}

and string parameter marshaling is not blittable.

Turns out™ that this particular usage of non-blittable parameter marshaling was fixed and rendered moot by:

That said, this code path felt slightly less than ideal: the "top-level abstraction" for member lookups is an "encoded member", a string containing the name of the member, a ., and the JNI signature of the member, e.g.:

_members.InstanceFields.GetBooleanValue("propogateFinallyBlockExecuted.Z", this)

The "encoded member" would need to be split on ., and with c6c487b6 the name and signature would be separately passed to Marshal.StringToCoTaskMemUTF8(), which performs a memory allocation and converts the UTF-16 string to UTF-8.

Meanwhile, C# 11 introduced UTF-8 string literals, which allows the compiler to deal with UTF-8 conversion and memory allocation.

Enter `JniMemberInfoLookup``:

public ref struct JniMemberInfoLookup {
    public  string                  EncodedMember   {get;}
    public  ReadOnlySpan<byte>      MemberName      {get;}
    public  ReadOnlySpan<byte>      MemberSignature {get;}

    public JniMemberInfoLookup (string encodedMember, ReadOnlySpan<byte> memberName, ReadOnlySpan<byte> memberSignature);
}

JniMemberInfoLookup removes the need to call Marshal.StringToCoTaskMemUTF8() entirely, at the cost of a more complicated member invocation:

// Old and busted:
bool value = _members.InstanceFields.GetBooleanValue("propogateFinallyBlockExecuted.Z", this);

// Eventual new hawtness:
var lookup = new JniMemberInfoLookup (
    "propogateFinallyBlockExecuted.Z",
    "propogateFinallyBlockExecuted"u8,
    "Z"u8);
bool value = _members.InstanceFields.GetBooleanValue(lookup, this);

Is It Worth It™? Maybe; see the new JniFieldLookupTiming.FieldLookupTiming() test, which allocates a new JniPeerMembers instance and invoke members.InstanceFields.GetFieldInfo(string) and members.InstanceFields.GetFieldInfo(JniMemberInfoLookup). (A new JniPeerMembers instance is required because GetFieldInfo() caches the field lookup.) Using JniMemberInfoLookup is about 4% faster.

# FieldLookupTiming Timing: looking up JavaTiming.instanceIntField 10000 times
#   .InstanceMethods.GetFieldInfo(string):              00:00:02.2780667
#   .InstanceMethods.GetFieldInfo(JniMemberInfoLookup): 00:00:02.2016146

I'm not sure if this is actually worth it, especially as this will imply an increase in code size.

TODO:

jonpryor commented 1 week ago

There isn't much point in continuing this effort, other than to show the idea and -- more importantly -- how it performs; see 840f04a4f4d7375a44f3a935901429c24f92c8ae. Trying to use JniMemberInfoLookup for method invocations results in an order of magnitude performance degradation. It's non-viable.

What's here uses JniMemberInfoLookup alongside ReadOnlySpan<JniArgumentValue>. Locally, on the thought the performance loss was due to ReadOnlySpan<JniArgumentValue>, I changed it all to instead be JniArgumentValue* (which is what the other e.g. Invoke*Method() methods use). This didn't help much: performance was still roughly an order of magnitude worse, e.g.

Method Lookup + Invoke Timing:
           Traditional: 00:00:00.0135348
            No caching: 00:00:00.0145592
          Dict w/ lock: 00:00:00.0132410
        ConcurrentDict: 00:00:00.0165738
        JniPeerMembers: 00:00:00.0150361
            JPM+Lookup: 00:00:00.0145659

              (I)I virtual+traditional: 00:00:00.0000447
           (I)I virtual+JniPeerMembers: 00:00:00.0000439
               (I)I virtual+JPM+Lookup: 00:00:00.0004603

0.0000439 to 0.0004603 is a 10.4x increase in time: an order of magnitude.

Without pulling out a profiler, my guess is that this is due to the use of u8 strings, or due to code size increase. Timing_ToString_JniPeerMembers() is 24 bytes, while Timing_ToString_JniPeerMembers_Lookup() is more than double that, at 55 bytes.