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

David Holmes david.holmes at oracle.com
Thu Jun 20 17:27:17 PDT 2013


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.

Minor comment: are the friend declarations needed for both Symbolbase 
and Symbol?

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
>


More information about the hotspot-runtime-dev mailing list