RFR 8009382: Add JVM_Get{Field|Method}TypeAnnotations

Joel Borggrén-Franck joel.franck at oracle.com
Tue Mar 26 10:32:59 UTC 2013


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)->method_with_idnum(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 build-dev mailing list