RFR 8009382: Add JVM_Get{Field|Method}TypeAnnotations

Joel Borggrén-Franck joel.franck at oracle.com
Tue Mar 26 23:47:40 PDT 2013


Thanks Rickard, Dan and Erik for the reviews and help with pushing this.

cheers
/Joel

On 26 mar 2013, at 15:40, Rickard Bäckman <rickard.backman at oracle.com> wrote:

> I'll sponsor the change.
> 
> /R
> 
> On Mar 25, 2013, at 4:17 PM, Daniel D. Daugherty wrote:
> 
>> On 3/22/13 8:16 AM, Joel Borggrén-Franck wrote:
>>> Hi,
>>> 
>>> Please review: http://cr.openjdk.java.net/~jfranck/8009382/hotspot/webrev.01/
>> 
>> 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.
>> 
>> 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.
>> 
>>   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
>> 
>> src/share/vm/prims/jvm.h
>>   No comments.
>> 
>> 
>>> 
>>> Bug is here: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8009382
>>> 
>>> In short, this is needed in order to implement the desired semantics for reflecting over type annotations after a class redefine. This is also needed in order to reduce the size overhead introduces on the JDK side in the current type annotations reflection implementation.
>>> 
>>> 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.
>> 
>> Dan
>> 
>> 
>> 
>> 
>>> 
>>> While in the neighborhood I also fixed a handful of unlikely but possible null dereferences and removed an unused resource mark. I have run jdk_lang on a fastdebug build to verify the removal of the resource mark in addition to manual verification (also Dan helped me look into this, but errors are mine).
>>> 
>>> For Oracle reviewers, ccc is approved.
>>> 
>>> testing done:
>>> 
>>> jdk_lang with and without my prototype jdk changes x product/fastdebug
>>> vm.quick.testlist
>>> jprt standard build and test
>>> 
>>> cheers
>>> /Joel
>>> 
>>> 
>> 
> 



More information about the hotspot-dev mailing list