RFR: 8152844: JVM InstanceKlass Methods For Obtaining Package/Module Should Be Moved to Klass
Rachel Protacio
rachel.protacio at oracle.com
Fri Apr 22 15:22:57 UTC 2016
Hi, Coleen,
Thanks for the review! I'll remove the arrayKlass entries and, seeing no
other objections, will commit.
Rachel
On 4/22/2016 9:24 AM, Coleen Phillimore wrote:
>
> Hi,
>
> http://cr.openjdk.java.net/~rprotacio/8152844.00/src/share/vm/oops/arrayKlass.cpp.udiff.html
>
>
> I don't think you need module or package declared in arrayKlass.hpp
> since it should be abstract also, ie. there should be no instances of
> ArrayKlass.
>
> Otherwise, looks great. I love seeing those InstanceKlass::casts go
> away.
>
> Coleen
>
>
> On 4/21/16 10:26 PM, David Holmes wrote:
>> Hi Rachel,
>>
>> On 22/04/2016 6:11 AM, Rachel Protacio wrote:
>>> Hello,
>>>
>>> Please review this fix for the package() and module() functions, making
>>> them pure virtual functions of Klass and implementing them in the
>>> relevant child classes. This cleans up some verbose and repetitive code
>>> that was casting and checking klass types. Also added test cases to
>>> runtime/modules/getModuleJNI/GetModule.java, ensuring that all the
>>> module() functions work as expected.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8152844
>>> Open webrev: http://cr.openjdk.java.net/~rprotacio/8152844.00/
>>
>> The refactoring all looks good to me (though C++ annoys me in needing
>> to re-declare the functions in each subclass :) ).
>>
>> Cheers,
>> David
>>
>>> Tested with jtreg tests in jdk/test (java/lang, java/io, java/security,
>>> java/util, tools/jimage, tools/jlink, and tools/jmod), JPRT, and RBT
>>> hotspot and non-colo tests.
>>>
>>> Thanks!
>>> Rachel
>
More information about the hotspot-runtime-dev
mailing list