Skip to content

Commit 46b7aee

Browse files
jonpryorjonpryor
committed
[Java.Interop, Java.Interop.Dynamic] Array Performance Investigation
Remember commit 775b34f? > Con: OMFG IS THIS SLOW. > > Like, wow, incredibly slow, like ~11s to lookup the names and > parameter types of all methods on java.util.Arrays. So... WTF is going on there? Object identity is what's going on there. For "sane"/"correct"/"normal" semantics, JavaObjectArray<T> follows object identity semantics, e.g. JavaVM.GetObject() will always return the same (registered) wrapper for Java handle that refers to the "same" Java instance, as per JavaVM.RegisteredInstances. Supporting Object identity is...not fast. See the new JavaArrayTiming.ObjectArrayEnumerationTiming() and MiscRuntimeTiming.ObjectCreationTiming() tests. ObjectArrayEnumerationTiming() uses Java reflection to grab all of the methods of java.util.Arrays. The stunning output? # methodHandles(JavaObjectArray<JavaObject>) creation timing: 00:00:00.6106453 Count=114 # methodHandles(JavaVM.GetObject) creation timing: 00:00:00.5959359 Count=114 # methodHandles(JavaObject[]) creation timing: 00:00:00.0890776 Count=114 # methodHandles(JniGlobalReference) creation timing: 00:00:00.1536897 Count=114 Given a JNI handle to the java.lang.reflect.Method[] returned by Class.getMethods(), the above shows four different ways of reading that Method[] and turning it into a List<Something>. The JavaObjectArray<JavaObject> implementation takes 0.61 *seconds* just to grab each element from the JavaObjectArray<JavaObject> and copy it into a List<JavaObject>. For 115 items. Which is *deplorable*. Why is it so bad? Next is an implementation which grabs the JNI handle for each element in the array and passes it through JavaVM.GetObject<T>() before adding to a List<JavaObject>. Note that it's VERY CLOSE in time to the JavaObjectArray<T> version. The methodHandles(JavaObject[]) version grabs each element from the array, then immediately constructs a new JavaObject instance around it. NO OBJECT IDENTITY is involved; aliases can (will) be created, and that's OKAY (here). Note that we drop from ~0.6s to ~0.089s -- nearly 7x faster, simply by skipping object identity. The final methodHandles(JniGlobalReference) is the same-but-different as methodHandles(JavaObject[]): instead of a new JavaObject, it greates a new JNI Global Reference. (Why a GREF? Futureproof!) It's slower than JavaObject (must be Local references vs. Global refs), but still much faster than our object identity infrastructure. TL;DR: Object identity is problematic. There is an issue to attempt to address it: #4 ...but we're not fixing that here. The ObjectCreationTiming() tests are a different take on the above results, comparing the timing of JNIEnv::AllocObject(), JNIEnv::NewObject(), `new JavaObject()`, and JavaVM.GetObject<JavaObject>(): ## ObjectCreationTiming Timing: Total=00:00:35.1368016 AllocObject=00:00:02.8751501 NewObject=00:00:02.8724492 `new JavaObject()`=00:00:06.0586476 JavaVM.GetObject()=00:00:14.5206758 This somewhat agrees with the previous discussion: JavaVM.GetObject<T>() (object identity) is slower than direct object creation. With that in mind, we can turn our attention back to Java.Interop.Dynamic and its terrible performance by replacing it's JavaObjectArray<JavaObject> use with low-level array access. This in turn necessitates making the JniEnvironment.Arrays type public (previously `internal`) so that Java.Interop.Dynamic can skip/bypass the object identity semantics that JavaObjectArray<T> enforces. This improves things from taking ~11s to lookup the names to taking ~4s to run the entire Java.Interop.Dynamic-Tests assembly. Total time : 3.9523533 seconds This is much more reasonable. (Perhaps not perfect, but reasonable.) While investigating, fix a variety of other minor issues: * Fix JavaObjectArray<T> so that it doesn't needlessly re-access the Length property all the damn time, which SHOULD reduce JNI calls and thus improve performance. * Fix JavaVM.CreateWrapperType() to not create so many JniType instances. Investigation was suggesting that the JniType creation and disposal was adding a bit of overhead, so avoiding the JniType abstraction provides *some* (small) speedup. * Cleanup JniType.Name (commit 9f380eb), moving its logic into JniEnvironment.Types.GetJniTypeNameFromClass(). This further increases consistency.
1 parent 8f7d339 commit 46b7aee

File tree

8 files changed

+280
-60
lines changed

8 files changed

+280
-60
lines changed

‎src/Java.Interop.Dynamic/Java.Interop.Dynamic/DynamicJavaClass.cs

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -114,23 +114,26 @@ void LookupMethods ()
114114

115115
StaticMethods = new Dictionary<string, List<JavaMethodInvokeInfo>> ();
116116

117-
using (var methods = new JavaObjectArray<JavaObject> (Class_getMethods.CallVirtualObjectMethod (members.JniPeerType.SafeHandle), JniHandleOwnership.Transfer)) {
118-
foreach (var method in methods) {
119-
var s = Member_getModifiers.CallVirtualInt32Method (method.SafeHandle);
117+
using (var methods = Class_getMethods.CallVirtualObjectMethod (members.JniPeerType.SafeHandle)) {
118+
int len = JniEnvironment.Arrays.GetArrayLength (methods);
119+
for (int i = 0; i < len; ++i) {
120+
var method = JniEnvironment.Arrays.GetObjectArrayElement (methods, i);
121+
var s = Member_getModifiers.CallVirtualInt32Method (method);
120122
if ((s & JavaModifiers.Static) != JavaModifiers.Static) {
121123
method.Dispose ();
122124
continue;
123125
}
124126

125-
var name = JniEnvironment.Strings.ToString (Method_getName.CallVirtualObjectMethod (method.SafeHandle), JniHandleOwnership.Transfer);
127+
var name = JniEnvironment.Strings.ToString (Method_getName.CallVirtualObjectMethod (method), JniHandleOwnership.Transfer);
126128

127129
List<JavaMethodInvokeInfo> overloads;
128130
if (!StaticMethods.TryGetValue (name, out overloads))
129131
StaticMethods.Add (name, overloads = new List<JavaMethodInvokeInfo> ());
130132

131-
var rt = new JniType (Method_getReturnType.CallVirtualObjectMethod (method.SafeHandle), JniHandleOwnership.Transfer);
132-
var m = new JavaMethodInvokeInfo (name, true, rt, method);
133+
var rt = new JniType (Method_getReturnType.CallVirtualObjectMethod (method), JniHandleOwnership.Transfer);
134+
var m = new JavaMethodInvokeInfo (name, true, rt, method);
133135
overloads.Add (m);
136+
method.Dispose ();
134137
}
135138
}
136139
}
@@ -356,18 +359,19 @@ sealed class JavaMethodInvokeInfo : IDisposable {
356359

357360
public string Name;
358361
public JniType ReturnType;
359-
public List<JniType> Arguments;
360362
public bool IsStatic;
361-
public JavaObject Method;
363+
364+
public List<JniGlobalReference> Arguments;
365+
public JniGlobalReference Method;
362366

363367
public string Signature;
364368

365-
public JavaMethodInvokeInfo (string name, bool isStatic, JniType returnType, JavaObject method)
369+
public JavaMethodInvokeInfo (string name, bool isStatic, JniType returnType, JniReferenceSafeHandle method)
366370
{
367371
Name = name;
368372
IsStatic = isStatic;
369373
ReturnType = returnType;
370-
Method = method;
374+
Method = method.NewGlobalRef ();
371375
}
372376

373377
public void Dispose ()
@@ -383,8 +387,6 @@ public void Dispose ()
383387
return;
384388

385389
for (int i = 0; i < Arguments.Count; ++i) {
386-
if (Arguments [i] == null)
387-
continue;
388390
Arguments [i].Dispose ();
389391
Arguments [i] = null;
390392
}
@@ -400,13 +402,14 @@ public void LookupArguments ()
400402
var sb = new StringBuilder ();
401403
sb.Append (Name).Append ("\u0000").Append ("(");
402404

403-
Arguments = new List<JniType> ();
404-
using (var methodParams = new JavaObjectArray<JavaObject> (DynamicJavaClass.Method_getParameterTypes.CallVirtualObjectMethod (Method.SafeHandle), JniHandleOwnership.Transfer)) {
405-
foreach (var p in methodParams) {
406-
var pt = new JniType (p.SafeHandle, JniHandleOwnership.DoNotTransfer);
407-
Arguments.Add (pt);
408-
sb.Append (vm.GetJniTypeInfoForJniTypeReference (pt.Name).JniTypeReference);
409-
p.Dispose ();
405+
using (var methodParams = DynamicJavaClass.Method_getParameterTypes.CallVirtualObjectMethod (Method)) {
406+
int len = JniEnvironment.Arrays.GetArrayLength (methodParams);
407+
Arguments = new List<JniGlobalReference> (len);
408+
for (int i = 0; i < len; ++i) {
409+
using (var p = JniEnvironment.Arrays.GetObjectArrayElement (methodParams, i)) {
410+
sb.Append (JniEnvironment.Types.GetJniTypeNameFromClass (p));
411+
Arguments.Add (p.NewGlobalRef ());
412+
}
410413
}
411414
}
412415
sb.Append (")").Append (vm.GetJniTypeInfoForJniTypeReference (ReturnType.Name).JniTypeReference);
@@ -425,10 +428,10 @@ public bool CompatibleWith (List<JniType> args, DynamicMetaObject[] dargs)
425428
for (int i = 0; i < Arguments.Count; ++i) {
426429
if (args [i] == null) {
427430
// Builtin type -- JNIEnv.FindClass("I") throws!
428-
if (Arguments [i].Name != vm.GetJniTypeInfoForType (dargs [i].LimitType).JniTypeReference)
431+
if (JniEnvironment.Types.GetJniTypeNameFromClass (Arguments [i]) != vm.GetJniTypeInfoForType (dargs [i].LimitType).JniTypeReference)
429432
return false;
430433
}
431-
else if (!Arguments [i].IsAssignableFrom (args [i]))
434+
else if (!JniEnvironment.Types.IsAssignableFrom (Arguments [i], args [i].SafeHandle))
432435
return false;
433436
}
434437
return true;

‎src/Java.Interop/Java.Interop/JavaArray.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ public T[] ToArray ()
5050

5151
public virtual IEnumerator<T> GetEnumerator ()
5252
{
53-
for (int i = 0; i < Length; ++i)
53+
int len = Length;
54+
for (int i = 0; i < len; ++i)
5455
yield return this [i];
5556
}
5657

‎src/Java.Interop/Java.Interop/JavaObjectArray.cs

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public JavaObjectArray (IList<T> value)
3333
: this (CheckLength (value))
3434
{
3535
for (int i = 0; i < value.Count; ++i)
36-
this [i] = value [i];
36+
SetElementAt (i, value [i]);
3737
}
3838

3939
public JavaObjectArray (IEnumerable<T> value)
@@ -45,29 +45,50 @@ public override T this [int index] {
4545
get {
4646
if (index < 0 || index >= Length)
4747
throw new ArgumentOutOfRangeException ("index", "index < 0 || index >= Length");
48-
var lref = JniEnvironment.Arrays.GetObjectArrayElement (SafeHandle, index);
49-
return JniMarshal.GetValue<T> (lref, JniHandleOwnership.Transfer);
48+
return GetElementAt (index);
5049
}
5150
set {
5251
if (index < 0 || index >= Length)
5352
throw new ArgumentOutOfRangeException ("index", "index < 0 || index >= Length");
54-
using (var h = JniMarshal.CreateLocalRef (value))
55-
JniEnvironment.Arrays.SetObjectArrayElement (SafeHandle, index, h);
53+
SetElementAt (index, value);
54+
}
55+
}
56+
57+
T GetElementAt (int index)
58+
{
59+
var lref = JniEnvironment.Arrays.GetObjectArrayElement (SafeHandle, index);
60+
return JniMarshal.GetValue<T> (lref, JniHandleOwnership.Transfer);
61+
}
62+
63+
void SetElementAt (int index, T value)
64+
{
65+
using (var h = JniMarshal.CreateLocalRef (value))
66+
JniEnvironment.Arrays.SetObjectArrayElement (SafeHandle, index, h);
67+
}
68+
69+
public override IEnumerator<T> GetEnumerator ()
70+
{
71+
int len = Length;
72+
for (int i = 0; i < len; ++i) {
73+
yield return GetElementAt (i);
5674
}
5775
}
5876

5977
public override void Clear ()
6078
{
6179
int len = Length;
62-
for (int i = 0; i < len; i++)
63-
this [i] = default (T);
80+
using (var v = JniMarshal.CreateLocalRef (default (T))) {
81+
for (int i = 0; i < len; i++) {
82+
JniEnvironment.Arrays.SetObjectArrayElement (SafeHandle, i, v);
83+
}
84+
}
6485
}
6586

6687
public override int IndexOf (T item)
6788
{
6889
int len = Length;
6990
for (int i = 0; i < len; i++) {
70-
var at = this [i];
91+
var at = GetElementAt (i);
7192
try {
7293
if (EqualityComparer<T>.Default.Equals (item, at) || JniMarshal.RecursiveEquals (item, at))
7394
return i;
@@ -92,7 +113,7 @@ internal override void CopyToList (IList<T> list, int index)
92113
{
93114
int len = Length;
94115
for (int i = 0; i < len; i++) {
95-
var item = this [i];
116+
var item = GetElementAt (i);
96117
list [index + i] = item;
97118
if (forMarshalCollection) {
98119
var d = item as IJavaObject;

‎src/Java.Interop/Java.Interop/JavaVM.cs

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -496,9 +496,11 @@ protected virtual IJavaObject CreateObjectWrapper (JniReferenceSafeHandle handle
496496
return (IJavaObject)Activator.CreateInstance (targetType, handle, transfer);
497497
}
498498

499-
Type GetWrapperType (JniReferenceSafeHandle handle)
499+
Type GetWrapperType (JniReferenceSafeHandle instance)
500500
{
501-
var jniTypeName = handle.GetJniTypeName ();
501+
var klass = JniEnvironment.Types.GetObjectClass (instance);
502+
var jniTypeName = JniEnvironment.Types.GetJniTypeNameFromClass (klass);
503+
502504
Type type = null;
503505
while (jniTypeName != null) {
504506
type = GetTypeForJniTypeRefererence (jniTypeName);
@@ -512,15 +514,22 @@ from c in type.GetConstructors (bindingFlags)
512514
where p [0].ParameterType == typeof(JniReferenceSafeHandle) &&
513515
p [1].ParameterType == typeof(JniHandleOwnership)
514516
select c;
515-
if (ctors.Any ())
517+
if (ctors.Any ()) {
518+
klass.Dispose ();
516519
return type;
520+
}
517521
}
518522

519-
using (var jniType = new JniType (jniTypeName))
520-
using (var super = jniType.GetSuperclass ()) {
521-
jniTypeName = super == null ? null : super.Name;
522-
}
523+
var super = JniEnvironment.Types.GetSuperclass (klass);
524+
jniTypeName = (super == null || super.IsClosed || super.IsInvalid)
525+
? null
526+
: JniEnvironment.Types.GetJniTypeNameFromClass (super);
527+
528+
klass.Dispose ();
529+
klass = super;
523530
}
531+
if (klass != null)
532+
klass.Dispose ();
524533
return null;
525534
}
526535

‎src/Java.Interop/Java.Interop/JniEnvironment.Types.cs

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,25 @@
11
using System;
2+
using System.Collections.Generic;
23
using System.Text;
34

45
namespace Java.Interop
56
{
67
partial class JniEnvironment {
78
static partial class Types {
89

10+
readonly static KeyValuePair<string, string>[] BuiltinMappings = new KeyValuePair<string, string>[] {
11+
new KeyValuePair<string, string>("byte", "B"),
12+
new KeyValuePair<string, string>("boolean", "Z"),
13+
new KeyValuePair<string, string>("char", "C"),
14+
new KeyValuePair<string, string>("double", "D"),
15+
new KeyValuePair<string, string>("float", "F"),
16+
new KeyValuePair<string, string>("int", "I"),
17+
new KeyValuePair<string, string>("long", "J"),
18+
new KeyValuePair<string, string>("short", "S"),
19+
new KeyValuePair<string, string>("void", "V"),
20+
};
21+
22+
923
public static JniType GetTypeFromInstance (JniReferenceSafeHandle value)
1024
{
1125
var lref = JniEnvironment.Types.GetObjectClass (value);
@@ -17,13 +31,22 @@ public static JniType GetTypeFromInstance (JniReferenceSafeHandle value)
1731
public static string GetJniTypeNameFromInstance (JniReferenceSafeHandle handle)
1832
{
1933
using (var c = GetObjectClass (handle)) {
20-
var s = JniEnvironment.Current.Class_getName.CallVirtualObjectMethod (c);
21-
return JavaClassToJniType (Strings.ToString (s, JniHandleOwnership.Transfer));
34+
return GetJniTypeNameFromClass (c);
2235
}
2336
}
2437

38+
public static string GetJniTypeNameFromClass (JniReferenceSafeHandle handle)
39+
{
40+
var s = JniEnvironment.Current.Class_getName.CallVirtualObjectMethod (handle);
41+
return JavaClassToJniType (Strings.ToString (s, JniHandleOwnership.Transfer));
42+
}
43+
2544
static string JavaClassToJniType (string value)
2645
{
46+
for (int i = 0; i < BuiltinMappings.Length; ++i) {
47+
if (value == BuiltinMappings [i].Key)
48+
return BuiltinMappings [i].Value;
49+
}
2750
return value.Replace ('.', '/');
2851
}
2952
}

‎src/Java.Interop/Java.Interop/JniType.cs

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,6 @@ namespace Java.Interop {
1111

1212
public sealed class JniType : IDisposable {
1313

14-
readonly static KeyValuePair<string, string>[] BuiltinMappings = new KeyValuePair<string, string>[] {
15-
new KeyValuePair<string, string>("byte", "B"),
16-
new KeyValuePair<string, string>("boolean", "Z"),
17-
new KeyValuePair<string, string>("char", "C"),
18-
new KeyValuePair<string, string>("double", "D"),
19-
new KeyValuePair<string, string>("float", "F"),
20-
new KeyValuePair<string, string>("int", "I"),
21-
new KeyValuePair<string, string>("long", "J"),
22-
new KeyValuePair<string, string>("short", "S"),
23-
new KeyValuePair<string, string>("void", "V"),
24-
};
25-
2614
public static unsafe JniType DefineClass (string name, JniReferenceSafeHandle loader, byte[] classFileData)
2715
{
2816
fixed (byte* buf = classFileData) {
@@ -57,14 +45,7 @@ public string Name {
5745
get {
5846
AssertValid ();
5947

60-
var s = JniEnvironment.Current.Class_getName.CallVirtualObjectMethod (SafeHandle);
61-
var n = JniEnvironment.Strings.ToString (s, JniHandleOwnership.Transfer)
62-
.Replace ('.', '/');
63-
for (int i = 0; i < BuiltinMappings.Length; ++i) {
64-
if (n == BuiltinMappings [i].Key)
65-
return BuiltinMappings [i].Value;
66-
}
67-
return n;
48+
return JniEnvironment.Types.GetJniTypeNameFromClass (SafeHandle);
6849
}
6950
}
7051

0 commit comments

Comments
 (0)