Request for review: 7132690 InstanceKlass:_reference_type should be u1 type

Vitaly Davidovich vitalyd at gmail.com
Tue Jan 24 16:17:06 PST 2012


I think you'll be fully covered (including negative values) by doing the
following instead of hard coding max value in the assert:

assert(t == (u1)t, "doesn't fit")

Sent from my phone
On Jan 24, 2012 6:33 PM, "Jiangli Zhou" <jiangli.zhou at oracle.com> wrote:

> **
> The updated webrev is :
> http://cr.openjdk.java.net/~jiangli/7132690/webrev.01/
>
> Thanks,
> Jiangli
>
> On 01/24/2012 02:54 PM, Jiangli Zhou wrote:
>
> Hi Vitaly,
>
> An assert in the setter probably is a good idea. I should have added it
> when making the change. Thanks for the comments!
>
> Thanks,
> Jiangli
>
> On 01/24/2012 02:13 PM, Vitaly Davidovich wrote:
>
> Hi Jiangli,
>
> This is probably overly paranoid so feel free to ignore, but should the
> setter in InstanceKlass assert that the passed in ReferenceType fits into a
> u1 instead of silently narrowing it? Or change the setter to take a u1 and
> make caller do the cast? This would prevent someone defining another member
> of the enum with an explicit value that doesn't fit into u1.  Like I said,
> paranoia ... :)
>
> Thanks
>
> Sent from my phone
> On Jan 24, 2012 3:06 PM, "Jiangli Zhou" <jiangli.zhou at oracle.com> wrote:
>
>> Hi,
>>
>> Please review the change for 7132690:
>>
>>  http://cr.openjdk.java.net/~jiangli/7132690/webrev.00/
>>
>> It changes InstanceKlass::_reference_type from ReferenceType to u1. The
>> ReferenceType is defined as an enum with 6 values
>> (src/share/vm/utilities/globalDefinitions.hpp). The change saves 4-byte for
>> each class.
>>
>> Tested with runThese and ute vm.quick.testlist. Ran jprt.
>>
>> Thanks,
>>
>> Jiangli
>>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20120124/3c2e91f2/attachment.html 


More information about the hotspot-runtime-dev mailing list