-
Notifications
You must be signed in to change notification settings - Fork 64
[Java.Interop] Avoid Type.GetType() in ManagedPeer
#1168
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
Changes from all commits
b75ad19
7079166
c0fd2a2
3874a78
0950eb4
ba37b37
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| #nullable enable | ||
| #nullable enable | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
|
|
@@ -12,50 +12,44 @@ | |
| namespace Java.Interop { | ||
|
|
||
| partial class JniRuntime { | ||
| static JniTypeSignature __BooleanTypeArraySignature; | ||
| static JniTypeSignature __SByteTypeArraySignature; | ||
| static JniTypeSignature __CharTypeArraySignature; | ||
| static JniTypeSignature __Int16TypeArraySignature; | ||
| static JniTypeSignature __Int32TypeArraySignature; | ||
| static JniTypeSignature __Int64TypeArraySignature; | ||
| static JniTypeSignature __SingleTypeArraySignature; | ||
| static JniTypeSignature __DoubleTypeArraySignature; | ||
|
|
||
| static bool GetBuiltInTypeArraySignature (Type type, ref JniTypeSignature signature) | ||
| { | ||
| if (type == typeof (JavaArray<Boolean>) || type == typeof (JavaPrimitiveArray<Boolean>)) { | ||
| signature = GetCachedTypeSignature (ref __BooleanTypeArraySignature, "Z", arrayRank: 1, keyword: true); | ||
| return true; | ||
| } | ||
| if (type == typeof (JavaArray<SByte>) || type == typeof (JavaPrimitiveArray<SByte>)) { | ||
| signature = GetCachedTypeSignature (ref __SByteTypeArraySignature, "B", arrayRank: 1, keyword: true); | ||
| return true; | ||
| } | ||
| if (type == typeof (JavaArray<Char>) || type == typeof (JavaPrimitiveArray<Char>)) { | ||
| signature = GetCachedTypeSignature (ref __CharTypeArraySignature, "C", arrayRank: 1, keyword: true); | ||
| return true; | ||
| } | ||
| if (type == typeof (JavaArray<Int16>) || type == typeof (JavaPrimitiveArray<Int16>)) { | ||
| signature = GetCachedTypeSignature (ref __Int16TypeArraySignature, "S", arrayRank: 1, keyword: true); | ||
| return true; | ||
| } | ||
| if (type == typeof (JavaArray<Int32>) || type == typeof (JavaPrimitiveArray<Int32>)) { | ||
| signature = GetCachedTypeSignature (ref __Int32TypeArraySignature, "I", arrayRank: 1, keyword: true); | ||
| return true; | ||
| } | ||
| if (type == typeof (JavaArray<Int64>) || type == typeof (JavaPrimitiveArray<Int64>)) { | ||
| signature = GetCachedTypeSignature (ref __Int64TypeArraySignature, "J", arrayRank: 1, keyword: true); | ||
| return true; | ||
| } | ||
| if (type == typeof (JavaArray<Single>) || type == typeof (JavaPrimitiveArray<Single>)) { | ||
| signature = GetCachedTypeSignature (ref __SingleTypeArraySignature, "F", arrayRank: 1, keyword: true); | ||
| return true; | ||
| } | ||
| if (type == typeof (JavaArray<Double>) || type == typeof (JavaPrimitiveArray<Double>)) { | ||
| signature = GetCachedTypeSignature (ref __DoubleTypeArraySignature, "D", arrayRank: 1, keyword: true); | ||
| return true; | ||
| } | ||
| return false; | ||
|
|
||
| partial class JniTypeManager { | ||
|
|
||
| readonly struct JniPrimitiveArrayInfo { | ||
| public readonly JniTypeSignature JniTypeSignature; | ||
| public readonly Type PrimitiveType; | ||
| public readonly Type[] ArrayTypes; | ||
|
|
||
| public JniPrimitiveArrayInfo (string jniSimpleReference, Type primitiveType, params Type[] arrayTypes) | ||
| { | ||
| JniTypeSignature = new JniTypeSignature (jniSimpleReference, arrayRank: 1, keyword: true); | ||
| PrimitiveType = primitiveType; | ||
| ArrayTypes = arrayTypes; | ||
| } | ||
| } | ||
|
|
||
| static readonly JniPrimitiveArrayInfo[] JniPrimitiveArrayTypes = new JniPrimitiveArrayInfo[]{ | ||
| new ("Z", typeof (Boolean), typeof (Boolean[]), typeof (JavaArray<Boolean>), typeof (JavaPrimitiveArray<Boolean>), typeof (JavaBooleanArray)), | ||
| new ("B", typeof (SByte), typeof (SByte[]), typeof (JavaArray<SByte>), typeof (JavaPrimitiveArray<SByte>), typeof (JavaSByteArray)), | ||
| new ("C", typeof (Char), typeof (Char[]), typeof (JavaArray<Char>), typeof (JavaPrimitiveArray<Char>), typeof (JavaCharArray)), | ||
| new ("S", typeof (Int16), typeof (Int16[]), typeof (JavaArray<Int16>), typeof (JavaPrimitiveArray<Int16>), typeof (JavaInt16Array)), | ||
| new ("I", typeof (Int32), typeof (Int32[]), typeof (JavaArray<Int32>), typeof (JavaPrimitiveArray<Int32>), typeof (JavaInt32Array)), | ||
| new ("J", typeof (Int64), typeof (Int64[]), typeof (JavaArray<Int64>), typeof (JavaPrimitiveArray<Int64>), typeof (JavaInt64Array)), | ||
| new ("F", typeof (Single), typeof (Single[]), typeof (JavaArray<Single>), typeof (JavaPrimitiveArray<Single>), typeof (JavaSingleArray)), | ||
| new ("D", typeof (Double), typeof (Double[]), typeof (JavaArray<Double>), typeof (JavaPrimitiveArray<Double>), typeof (JavaDoubleArray)), | ||
| }; | ||
|
|
||
| static bool GetBuiltInTypeArraySignature (Type type, ref JniTypeSignature signature) | ||
| { | ||
| foreach (var e in JniPrimitiveArrayTypes) { | ||
| if (Array.IndexOf (e.ArrayTypes, type) < 0) | ||
| continue; | ||
|
Comment on lines
+44
to
+46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this happen on device? This looks like it is O(N^2). Is there a way to rework the data to be a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably not, and we're talking O(N**2) on 8 entries, each containing 4 elements within
Yes, but as this is static data, it'll be constructed during app startup, even for .NET Android (unless we make it conditional, and please no?), so I'm trying to figure out how to minimize startup overhead. My assumption (un-timed) is that creating an array of structs will be faster than populating a Dictionary, particularly when we're dealing with so few elements (8). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So, about that init vs. lookup time… Repro App```csharpusing System.Diagnostics; const int CreationCount = 1000000; var a = Stopwatch.StartNew (); var d = Stopwatch.StartNew (); Console.WriteLine ($"Array Creation: {a.ElapsedMilliseconds}ms"); var av = CreateArray (); var bv = CreateDict (); static JniPrimitiveArrayInfo[] CreateArray () static bool TryArrayLookup (JniPrimitiveArrayInfo[] array, Type type) static Dictionary<Type, JniPrimitiveArrayInfo> CreateDict () static bool TryDictLookup (Dictionary<Type, JniPrimitiveArrayInfo> dict, Type type) class JavaArray {} readonly struct JniPrimitiveArrayInfo { struct JniTypeSignature { There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apparently Results: % dotnet run
Array Creation: 646ms
Dict Creation: 1131ms
Array Lookup: 29ms
Dict Lookup: 1131ms
% dotnet run -c Release
Array Creation: 389ms
Dict Creation: 770ms
Array Lookup: 21ms
Dict Lookup: 770msAs expected, array lookup is faster -- half the time! -- and lookup is also much faster with arrays. That said, this investigation did present a bug in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. …and There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seeing the manual work involved to test, should we have a BenchmarkDotNet project in this repo? It seems like it would be useful to commit your benchmark above, and we'd have the option to run it later. BDN also does appropriate warmup, runs out of process, does some math, that might generally make it better than a manual benchmark. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, we should probably have one, and it should probably be in |
||
| signature = e.JniTypeSignature; | ||
| return true; | ||
| } | ||
| signature = default; | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| static readonly Lazy<KeyValuePair<Type, JniValueMarshaler>[]> JniPrimitiveArrayMarshalers = new Lazy<KeyValuePair<Type, JniValueMarshaler>[]> (InitJniPrimitiveArrayMarshalers); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the C# attribute available where we could use
nameof()? The resulting IL would be the same, but might help typos.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this assembly doesn't have a reference to
Java.Interop.dll.