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

[Ioi Lam]

On 06/20/2013 05:27 PM, David Holmes wrote:
> Hi Ioi,
>
> Reviewed. But see below.
>
> On 21/06/2013 4: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_SHORT_PAIR
>>      to ensure the proper alignment.
>
> I preferred the original version that constrained this to 
> symbol.hpp/cpp. We don't really support Atomic jshort operations in a 
> general sense - we only have inc/dec and even then this only applies 
> when you have the pair of shorts with only one needing atomic access! 
> So I would have preferred not to see this exposed in the Atomic class.
>
I like the new style because there's less clutter. Also, there *may* be 
other future cases when we have similar 2-byte counters, since we're 
trying to reduce the size of the metadata.

> Minor comment: are the friend declarations needed for both Symbolbase 
> and Symbol?
>
I removed the three 'friend' declarations from SymbolBase and it seems 
to work fine with Linux/gcc. So I will remove them and let JPRT tell me 
if it's good for other platforms.

Thanks
- Ioi

> Thanks,
> David
>
>>      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
>>