RFR: 8186476: Generalize Atomic::add with templates

[Erik Österlund]

Hi Coleen,

On 2017-08-25 00:17, coleen.phillimore at oracle.com wrote:
>
>
> On 8/21/17 5:17 PM, Kim Barrett wrote:
>>> On Aug 21, 2017, at 11:57 AM, Erik Österlund 
>>> <erik.osterlund at oracle.com> wrote:
>>>> I’m fairly sure the runtime folks would say you should be 
>>>> surprised, and would object to such a change.  The layout of symbol 
>>>> is carefully constructed, and I think such a change would increase 
>>>> the size of a symbol by 6 byes (not the obvious two) on a 64 bit 
>>>> platform.
>>> I find that surprising. By making body jbyte[0] instead, it seems to 
>>> me (after a glance) to be possible to retain the size of the header 
>>> from 8 bytes to 8 bytes after turning refcount to an int. This 
>>> leaves only the difference in payload and its effect on the 
>>> allocator and its alignment, which should seemingly not require 6 
>>> extra bytes. But perhaps I have misunderstood something about the 
>>> layout of Symbol.
>> No, I misremembered.  Yes, that change would cost an average of 2 
>> additional bytes per symbol, assuming symbol name lengths mod 8 are 
>> fairly uniformly distributed, which seems likely.
>>
>> That’s still 2 extra bytes per symbol, and I doubt the runtime folks 
>> would like that, since they’ve been going to some trouble to reduce 
>> the size of symbols:
>> 8009575: Reduce Symbol::_refcount from 4 bytes to 2 bytes
>> 8087143: Reduce Symbol::_identity_hash to 2 bytes
>
> Yes, the runtime folks would not like that after going through the 
> trouble to reduce the size of Symbol.  There can be lots of symbols.

Thank you for clarifying that.  Perhaps coming from GC, my view on this 
memory optimization is different. But I will respect your view and not 
question this decision.
In that case, I see no current objections to the solution Kim handed off 
to me. Are we good to go?

Thanks,
/Erik

>
> Coleen
>
>>
>>>>> 2) Implement it like jbyte cmpxchg. That would entail both calling 
>>>>> a generalized AddShortUsingInt function object from the platform 
>>>>> layer, and preferrably also using an int CAS loop for emulating 
>>>>> the add operation rather than the current emulation using atomic 
>>>>> int add, that relies on two short fields being placed next to each 
>>>>> other, depending on endianness of the machine, using the 
>>>>> ATOMIC_SHORT_PAIR macro for declaring the short pair, that may or 
>>>>> may not in fact enforce the intended alignment at compile time.
>>>>> 3) Continue emulating jshort Atomic::add with jint Atomi::add, but 
>>>>> turn it into an AddShortUsingInt function object, called from the 
>>>>> platform layer, like the similar CmpxchgByteUsingInt operation.
>>>>>
>>>>> So, I am curious if anyone would have loud objections if I was to 
>>>>> remove Atomic::add support for jshort, and changed its single use 
>>>>> (Symbol::_refcount), to use int instead. And if there are such 
>>>>> objections, I wonder if we really want to continue using the 
>>>>> ATOMIC_SHORT_PAIR macro and emulation using jint Atomic::add, or 
>>>>> it is okay to rewrite it to use an cmpxch loop instead and get rid 
>>>>> of the ATOMIC_SHORT_PAIR macro (that I find makes for a very weird 
>>>>> API).
>>>> I don’t think it’s worth spending a lot of effort generalizing the 
>>>> short case right now.  As you noticed, there is *exactly* one use 
>>>> of it, for the symbol refcount. Reconsider if that ever changes.
>>>>
>>> Okay, I agree. I will drop that for now. This single use case is 
>>> probably not worth the effort.
>> Okay.
>>
>