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

[Jiangli Zhou]

Hi Vitaly,

Thanks for catching it! A range check seems to be more explicit in this 
case:

   http://cr.openjdk.java.net/~jiangli/7132690/webrev.02/

Thanks,

Jiangli

On 01/24/2012 04:17 PM, Vitaly Davidovich wrote:
>
> 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 
> <mailto:jiangli.zhou at oracle.com>> wrote:
>
>     The updated webrev is :
>     http://cr.openjdk.java.net/~jiangli/7132690/webrev.01/
>     <http://cr.openjdk.java.net/%7Ejiangli/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
>>>     <mailto:jiangli.zhou at oracle.com>> wrote:
>>>
>>>         Hi,
>>>
>>>         Please review the change for 7132690:
>>>
>>>         http://cr.openjdk.java.net/~jiangli/7132690/webrev.00/
>>>         <http://cr.openjdk.java.net/%7Ejiangli/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-compiler-dev/attachments/20120124/c49e9a33/attachment.html