RFR 8009382: Add JVM_Get{Field|Method}TypeAnnotations (original) (raw)

Daniel D. Daugherty daniel.daugherty at oracle.com
Tue Mar 26 06:05:43 PDT 2013

Thumbs up on the re-review.

Dan

On 3/26/13 4:32 AM, Joel Borggrén-Franck wrote:

Hi,

Also including build-dev since this touches some makefiles. Thanks Dan! see inline, also new webrev: http://cr.openjdk.java.net/~jfranck/8009382/hotspot/webrev.02/ On 03/25/2013 04:17 PM, Daniel D. Daugherty wrote: On 3/22/13 8:16 AM, Joel Borggrén-Franck wrote:

make/bsd/makefiles/mapfile-vers-debug make/linux/makefiles/mapfile-vers-debug make/linux/makefiles/mapfile-vers-product make/solaris/makefiles/mapfile-vers It looks like the other entries are in alpha order so these two new entries should also be added in alpha order.

Fixed. I also noticed I forgot to update one of the bsd makefiles. src/share/vm/prims/jvm.cpp line 1510: return NULL; nit: indent should be two spaces line 1529: assert(false, "cannot find method"); line 1530: return NULL; // robustness Normally "robustness" comments flag logic after an assert() to indicate what we do in product mode to deal with the "bad" situation without crashing. In this case, we don't try to use 'm' after discovering that it is NULL so this could be simpler: Method* m = InstanceKlass::cast(k)->methodwithidnum(slot); assert(m != NULL, "cannot find method"); return m; // caller has to deal with NULL in product mode Yes, I realize you only touched the comment here, but it served to point out the messiness of the existing code. Good suggestion. Fixed. line 1551: return NULL; line 1565: return NULL; line 1579: return NULL; nit: indent should be two spaces line 1632: return NULL; line 1613: return NULL; nit: indent should be two spaces Fixed all indent nits.

A prototype of the jdk changes can be found here: http://cr.openjdk.java.net/~jfranck/8009382/jdk/webrev.00/ You should use a separate bug ID for the jdk repo changes. This will prevent confusion between when HSX with these changes is integrated into JDK8 and when the jdk repo changes are integrated into JDK8. The JDK changes have a separate bug, sorry if this was unclear. The jdk changes shown are just a proof of concept, intended to highlight that my HotSpot changes have actually been run. I will send out a separate review request in the future for the JDK changes. cheers /Joel

More information about the hotspot-dev mailing list