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

[Erik Österlund]

Hi Paul,

Thanks for the review! :)

/Erik

On 19 dec 2014, at 21:08, Paul Hohensee <paul.hohensee at gmail.com<mailto:paul.hohensee at gmail.com>> wrote:

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<mailto: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/<http://cr.openjdk.java.net/%7Ejwilhelm/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