[vmkit-commits] [PATCH] Various patches

[Will Dietz]

On Mon, Oct 17, 2011 at 5:41 PM, Will Dietz <willdtz at gmail.com> wrote:
> Hi,
>
> Attached are various patches that I've refactored out from the OpenJDK
> port, as they aren't necessarily part of switching runtimes and help
> the Classpath implementation too.
>
> Hopefully OpenJDK patches will be coming in the semi-near future...
>
> Please let me know if you have any questions or comments, I'm rather
> happy to discuss :).
>
> Thanks, and have a good one!
>
> ~Will
>

Some notes on the patches:

0001 Minor typo fixes.

0002 This fixes an assert that incorrectly bails when resolving a
method through an array--if you look at j3VirtualTableLookup for
example it expects 'cl' can be an array class (see the line after the
resolveMethod() invocation), which suggests to me that this was the
correct fix.  The code that triggers this is buried somewhere in the
bootstrap process of the OpenJDK port, not very reproducible.....yet
:).  If this doesn't immediately seem right/wrong to you let me know
and we'll just leave this as-is (we can re-evaluate once the OpenJDK
stuff is on the table).  Finally, if this patch *is* correct, we might
want to do the same for resolveField for consistency, although I
haven't run into a case where that's needed so that's not included
here.

0003 Adds some sun.misc.Unsafe.* field accessors.  I'm not sure what
kind of error-handling one is supposed to do while implementing
"unsafe" methods, but these are fairly simple methods.  Due to some
win patch-organization on my part, the pieces that make these usable
by defining staticFieldOffset are in the next patch.  Sorry about
that.

0004 More sun.misc.Unsafe.* implementations, although these are a bit
weaker robustness-wise.  In short, it's the minimum functionality to
get what OpenJDK's bootstrapping process needs to run and seems to
work well enough in my indirect testing via mauve and the like. YMMV,
etc, and if anything seems particularly egregious let me know and I'll
do what I can to clean them up some more.

0005 This is simply driven by a desire to improve the develop-debug
cycle, hopefully not too controversial :).  Mostly I got tired of
hitting pieces of JNI that weren't implemented and having to wait for
the extraordinarily slow Debug build to run just to see *what* wasn't
implemented.

0006/0007 Implement a touch more of misc JNI methods--hopefully these
are straightforward, if still lacking some love in the "throw what
exception when" department.  They also begin to make a case for some
abstractions to help with code-duplication (and this gets much worse
with the OpenJDK port stuff), but I'm aiming to have all these patches
almost entirely *add* LOC until the big picture is on the table--and
then we can figure out how to best architect code-sharing across
Unsafe/JNI/OpenJDK/GNUClasspath code.

0008 This is where I'd expect the most discussion.  This is an
incomplete support of registerNatives, but handles the kinds of
registerNatives-ing that OpenJDK likes to do (mostly of the "this java
method is really JVM_FooBar", or same thing with "this method is just
an alias for a particular JNI method..."--all an external library
redefining our Java methods with pointers it got from us either via
symbol resolution of exported through the JNI interface), and should
be AOT-compatible (in the cases it's not, it'll be caught with a
runtime error).  However, the dependency on symbols and in particular
dladdr isn't the best--probably breaks on Darwin, for example.

The upside is that in theory we handle more cases, which might help
with JNI and the like.. I'm not sure.

Thinking about it more, there are a few options/ideas that might be better:
--Check function pointers we're given in registerNatives against a
whitelisted table of functions that we allow/expect to be called on.
Since this is primarily motivated with getting OpenJDK working, this
table could simply consist of all the JVM_* and JNI Interface methods.
The benefit here is that we can encode the mapping with some simpler
identifier that refers to that entry (index in the table, for example)
and that would be completely AOT-compatible since we can just look
them up in the same table.  This is essentially a more hard-coded
version of the above patch that doesn't require dladdr(), and has a
potentially simpler mapping to encode in the JavaMethod definition,
but has no potential for anything beyond this OpenJDK-based use case.
I'm curious to hear your thoughts, but I suppose this is my favorite
at the moment--I'd forgotten amidst everything else that I was using
dladdr and that that would break Darwin support.

--Ignore registerNatives() altogether and statically encode the
mapping (either by defining
Java_java_MyClass_MyNativeMethod_Signature(...)-style methods, or
something more programmatic/table-driven).  This would work, and is
most similar to what is done elsewhere.  That said, it's the least
flexible and the adds the an implementation/maintenance/completeness
burden that the other ideas above don't have.  Well, and minor
correctness issue if someone calls one of the mapped functions before
the corresponding registerNatives is invoked (or after
UnregisterNatives, not implemented yet).

Anyway, hopefully that gives you a bit more context for these patches,
and look forward to your thoughts :).

Thanks for your time, and have a good one!

~Will