RFR: 8186476: Generalize Atomic::add with templates

[Kim Barrett]

> On Aug 21, 2017, at 9:10 AM, Erik Österlund <erik.osterlund at oracle.com> wrote:
> 
> Hi Kim,
> 
> Thank you for finding an approach that seems to have reached consensus, and rolling the ball forward while I was on vacation.
> I agree this seems to be a reasonable way of doing things and am delighted about the general approach.
> 
> When it comes to the proposed Atomic::add implementation, it seems to me like the emulation of Atomic::add on jshort using jint is very similar to how Atomic::cmpxchg on jbyte is emulated using jint. So it surprised me a bit that Atomic::add of jshort does not share the same mechanism as the generalized Atomic::cmpxchg of jbyte. In fact, it seems like the proposed implementation does not safely handle CV-qualifiers. For example, if the addend passed in is const qualified, I believe it will not match the AddImpl<jshort, jshort> class specialization, as it is looking for AddImpl<const jshort, jshort> instead.

That’s incorrect.  lvalue-to-rvalue conversion for non-class types drops the const qualifier.

> Therefore, I would propose dealing with Atomic::add on jshort in one of the following ways (numbered according to my preferences):
> 
> 1) Remove it. Atomic::add on jshort is only ever used by a reference counter in the Symbol class that can be placed next to the length. I would be surprised if the memory savings of using short instead of int for the reference counter is considerable.

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.

> 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.