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