RFR (S) 8009575 (2nd) - Reduce Symbol::_refcount from 4 bytes to 2 bytes

[Ioi Lam]

On 06/21/2013 12:14 PM, Yumin Qi wrote:
> Ioi,
>
>   As Dan indicates, Symbol now is derivative from SymbolBase, so it 
> should be reflected in SA code --- more changes needed.
>   The name split is for calculation of size? The only difference is
>    jbyte _body[1];
>
>    now it is in Symbol, not in SymbolBase.
>
>    Originally,
>    size = sizeof(Symbol)  include sizeof(jbyte) which should be 
> excluded if the symbol length > sizeof(jbyte).
>    So the should calculation  if no change to Symbol inheritance be:
>
>    size_t sz = heap_word_size(sizeof(Symbol) + (length > sizeof(jbyte) ? length - sizeof(jbyte) : 0));
>   Thanks
>   Yumin
>
Yumin,

As I wrote in 
http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2013-March/006385.html, 
sizeof(Symbol) include extra padding (3 bytes in most cases, but that 
could be compiler dependent??). So, if we use sizeof(Symbol), we have to 
explicitly do a "-3" instead of "-sizeof(jbyte)". Splitting SymbolBase 
out will avoid this hackiness.

Thanks
- Ioi

> On 6/21/2013 10:15 AM, Ioi Lam wrote:
>> Hi,
>>
>> Could anyone review this? Coleen has already reviewed it off-list, so 
>> I just need one more reviewer.
>>
>> Thanks
>> - Ioi
>>
>> On 06/20/2013 11:20 AM, Ioi Lam wrote:
>>> Please review my 2nd trial for this bug fix. It's much simplified 
>>> thanks
>>> to Coleen's suggestion.
>>>
>>> http://cr.openjdk.java.net/~iklam/8009575/symbol_refcount_002/
>>>
>>> Bug: Reduce Symbol::_refcount from 4 bytes to 2 bytes
>>>
>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8009575
>>> https://jbs.oracle.com/bugs/browse/JDK-8009575
>>>
>>> Summary of fix:
>>>
>>>     As noted in the bug report, the main problem is _refcount needs 
>>> to be
>>>     atomically incremented, but the smallest unit that 
>>> Atomic::incr() had
>>>     supported until now was 32-bit int.
>>>
>>>     With Coleen's help, I have created a platform-independent version
>>>     of Atomic::incr(short*) that's based on Atomic::add(int*, n).
>>>
>>>     Essentially, Atomic::incr(short*) can be implemented as
>>>     Atomic::add(int*, 0x10000), as long as the short occupies the most
>>>     significant 16 bits of the int. I added the macro ATOMIC_
>
>>> _PAIR
>>>     to ensure the proper alignment.
>>>
>>>     Also Symbol::size(int) is fixed to calculate the correct space 
>>> needed
>>>     for a Symbol. (Same as in the previos webrev --
>>>     see 
>>> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2013-March/006385.html)
>>>
>>> Tests:
>>>
>>>     JPRT
>>>     UTE:
>>>        - vm.tmtools.testlist nsk.sajdi.testlist vm.runtime.testlist
>>>          vm.quick.testlist vm.parallel_class_loading.testlist
>>>        - Serviceability Agent tested are included in these lists.
>>>     MedRec
>>>
>>> Thanks
>>> - Ioi
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130621/5bdda8f2/attachment.html