Thanks for the review David!

cheers
/Joel

On 12/18/2012 03:22 AM, David Holmes wrote:
Hi Joel,

This all looks fine to me. I have no comments. Didn't even see any nits
to pick. :)

Thanks,
David

On 18/12/2012 2:47 AM, Joel Borggrén-Franck wrote:
Hi,

Here is a webrev for adding VM support for type annotation reflection:
http://cr.openjdk.java.net/~jfranck/8004823/webrev.v4/

Type annotations are coming with JDK 8, the proposed language changes
can be found from here:
http://openjdk.java.net/projects/type-annotations/

Since runtime visible type annotations probably won't  be that common
I have tried to minimise overhead when there are no type annotations.
This is done by adding a pointer to a type annotation Annotations
instance from the regular annotations Annotations instance, see
annotations.hpp. This means that if there are no runtime visible
annotations there is no additional overhead with this patch since no
Annotations instance will be allocated at all. If there is runtime
visible annotations but no runtime visible type annotations there is
an additional overhead of 1 pointer with this patch. If you use type
annotations there will also be storage overhead, but that is to be
expected.

This patch also fixes  that Annotations were never deallocated when
InstanceKlass::deallocate_contents() were called.

There is currently no redefineClass support for type annotations. This
will be added later. In order avoid ship possibly broken bytes to
java-land, type annotations are nulled out and deallocated after a
redefineClass.

This patch also exports a new method from jvm.h,
JVM_GetClassTypeAnnotations, so map files are updated. Build-dev are
included for review.

Testing done:
- vm.quick.testlis both on b67 and on a jdk that is patched with the
new fields to Field, Method and Constructor
- jprt full forrest build
- jdk_lang in both b67 and the patched jdk
- manual tests that we get the correct bytes out from the VM. These
tests can be added once some more changes have propagated from the jdk
repo

FYI, The initial set of jdk changes are being reviewed on
core-libs-dev:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2012-December/013016.html


cheers
/Joel

Reply via email to