RFR: 8067790: Better support for native implementations of Atomic::cmpxchg

[Paul Hohensee]

Looks good to me, but then I reviewed it awhile back. :)

Paul

On Wed, Dec 17, 2014 at 5:38 PM, Erik ?sterlund <erik.osterlund at lnu.se>
wrote:
> Hi all,
>
> Previously, jbyte Atomic::cmpxchg had one general implementation for all
platforms. A macro based solution was introduced, adding specialized
implementation for the x86 platforms without changing code for the ports
for other architectures, making the specialization optional (from ).
>
> However, macros are not very nice and this new fix gets rid of the macros
and replaces it with a cleaner solution using inheritance and templates.
The core idea is simple: split the Atomic class into three layers of
inheritance.
>
> 1. AtomicBase is the base class where general operations for all
platforms should go, now it contains the general implementation for jbyte
cmpxchg, but in the future other similar methods are welcome too.
> 2. AtomicPlatform is an optional class that a platform can define, but
does not have to (and hence does not break existing ports), allowing
specialized variants to override the behaviour of AtomicBase. Now it
contains the specialized platform dependent x86 variants of jbyte cmpxchg.
> 3. Atomic is the fascade of all atomics as expected. Its jbyte cmpxchg
implementation simply forwards to its super class AtomicSuper which is
either AtomicBase or AtomicPlatform if provided for the specific platform.
>
> All in all, it?s based on simple inheritance with the twist that the
AtomicPlatform part is optional. This is allowed using some C++
metaprogramming idioms. Concretely, a trait was implemented called
SelectBaseClass that allows you to specify the desired base class, and
another one to fall back to in case the desired base class was not defined
(forward declaration). The super class of Atomic is hence defined as:
>
> typedef SelectBaseClass<
AtomicPlatform, AtomicBase>::type AtomicSuper;
>
> To implement this trait, an equivalent variant of the C++11/boost
is_base_of trait was implemented, called IsBaseOf to follow our naming
conventions. It is very similar to the standard version but works on all of
our compilers. The only difference so far between IsBaseOf and is_base_of
is that IsBaseOf will return true when comparing two unions that are the
same, while is_base_of will not. The reason for this is that I found no
portable way of detecting unions. A bunch of dependent traits were
implemented too, and it can all be found in utilities/traits/ since I like
reasonably fine grained headers. All these traits are built to be reusable
and perhaps we will have more uses for metaprogramming in the future where
this comes in handy. The intended behaviour and contract of the traits are
documented in comments.
>
> RFE: https://bugs.openjdk.java.net/browse/JDK-8067790
> Webrev: http://cr.openjdk.java.net/~jwilhelm/8067790/webrev.00/
>
> In summary this allows us to get rid of the ugly macro used before and
provide a better general mechanism for having generalized and specialized
atomics without breaking existing ports for those platforms that do not
need it. And hopefully the metaprogramming tools will come in useful in the
future for perhaps similar situations.
>
> Testing:
> * All new traits have test cases using STATIC_ASSERT nailing down the
intended behaviour and contract also reflected in the comments.
> * The changes passed the jprt testing ok. (Thanks to Jesper for running
it!)
>
> Thanks,
> /Erik