Skip to content

Commit

Permalink
[Java.Interop.Dynamic] Rethink the whole BindConvert() nonsense
Browse files Browse the repository at this point in the history
Recall commit 9057654:

> Doubly awesome -- and a huge part of the complication! -- is that it
> uses DynamicMetaObject.BindConvert() to detect that the field type
> is `int`, as field type information is required in order to lookup the
> field, and thus we need to delay the actual JNI field lookup and
> access until *after* we know the desired type. Figuring out how to

While it is awesome, it is not without its problems: not only is it
complex, but it's also limiting, in that it requires that you have a
type to convert to. As noted in commit f62858a:

> "Implicit" member access fails:
>
>     dynamic d = new DynamicJavaClass ("java/lang/Object");
>     Assert.AreEqual ("java/lang/Object", d.JniClassName);
>     // BOOM [2]

"Implicit" access -- access where we're not casting or assigning to a
variable -- fails because everything is "delayed" until we know the
target type, until DYnamicMetaObject.BindConvert() is invoked.

Not only does this make things "icky", as the above description shows,
but "thinking forward" it will make invoking `void` methods
"interesting" -- there IS no type to convert to!

This could plausibly be supported/"worked around" by having
DynamicMetaObject.BindInvokeMember() immediately try to invoke the
inferred method with a `void` return type, and if that fails go
through the defer logic...but that won't help with a Fallback method.

Things get ugly fast.

*Then*, there's the problem of inferring "uninferrable" types:
*eventually* we'll need to support invoking instance methods, at
whichi point something like this is desirable:

	dynamic Arrays  = new DynamicJavaClass ("java/util/Arrays");
	dynamic list    = Arrays.asList(new[]{"a", "b", "c"});

java.util.Arrays.asList() returns a java.util.List<T>. If we assume
that java.util.List<T> is unbound (no binding generator!), how is the
above supposed to work? We can't "defer" the invoke until we know the
return type: THERE IS NO RETURN TYPE (other than `object`).

We could in turn "hack" this by using "meta" methods, e.g.
a `.as()` method:

	dynamic list    = Arrays.asList(new[]{"a", "b", "c"}).as("java/util/List");

But that's just fugly and user hostile.

Thus, DUMP the DynamicMetaObject.BindConvert() "deferred lookup"
mechanism from commit 9057654, and replace it with a more
straightforward (if uglier) "use Java Reflection to lookup ALL the
members in advance!" implementation.

This allows DynamicJavaClass to know all the valid Java member names
(methods, fields) and check for validity within the
DynamicJavaClass.MetaObject.Bind*() methods, simplifying the
implementation and allowing "inferred" use: since we know all valid
member names, we can immediately invoke them within the appropriate
DynamicMetaObject.Bind*() method, instead of needing to wrap up a shit
load of data into a DeferredConvert<T> instance for later execution.

Pro: simple and expected stuff now works:

	Assert.AreEqual ("java/lang/Object", d.JniClassName);
	// Look ma, no cast! No explicit variable assignment!

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. Because of this, I
split up/delayed method parameter type lookup until the method
overloads are actually needed, which sped things up some, but
resolving all the parameter and return types for all overloads of
Arrays.binarySearch() is still a ~4s process.

It's terrible.

Furthermore, this is NOT a problem with JNI: running a similar
algorithm on "native" Xamarin.Android is MUCH faster, completing in a
fraction of a second, so it's not JNI, it's our array marshaling.

TODO: Figure out WTF is making array marshaling SO GODDADMN SLOW.
  • Loading branch information
jonpryor committed Sep 24, 2015
1 parent df3dd93 commit 775b34f
Show file tree
Hide file tree
Showing 2 changed files with 285 additions and 151 deletions.
Loading

0 comments on commit 775b34f

Please sign in to comment.