RFR 8009382: Add JVM_Get{Field|Method}TypeAnnotations

Daniel D. Daugherty daniel.daugherty at oracle.com
Tue Mar 26 13:05:43 UTC 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)->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