RFR(M): 8154736: enhancement of cmpxchg and copy_to_survivor for ppc64

[Thomas Schatzl]

Hi Hiroshi,

  first, apologies for taking so long for an answer. Sorry.

On Mon, 2016-10-17 at 10:44 +0900, Hiroshi H Horii wrote:
> Hi David,
> 
> Thank you for your comments.
> 
> > Do you have any metrics on this latest version?
> > [...]
> > 
> > I think the GC experts need to have a discussion to resolve things
> > to their mutual satisfaction.
> 
> Thank you for lots of your comments and suggestions. And lots of my
> mistakes made the discussion long. very sorry. I would like to know
> comments of GC experts.

  we in the gc team have discussed this change quite a bit internally.
Overall, we think this change seems far too risky from both a
functional and performance perspective to go into 9 at this time.

The current proposal lacks some clear analysis on why removing the
barriers is safe, most analysis in this thread has been "it is fine";
and "the code is faster and does not crash" on one particular platform
for one particular application, and that seems too little.

We at least expect the change to be not only analyzed "good" in a
review, but also tested thoroughly on all platforms affected (which are
all of them in the latest change). We can of course help with testing
on platforms we support.

We also think the testing needs to include both functional and
performance testing, and the performance testing ought to be using some
well-chosen benchmarks. (It was pointed out very early in the
discussion of this change that specjbb2013 is deprecated, yet that is
the only benchmark that's been reported out.)

The most recent change also penalizes current platforms that do not
implement the release-CAS with an additional acquire. That might be not
an issue for TSO platforms, but others will be affected.

While we think other platforms could quickly adapt to this, this would
force that the developer that implements this for other platforms
(arm/aarch64) to be stuck with re-analyzing these issues. We
do not think this is fair. We think this is a change (or set of
changes) that needs to be pushed for all platforms at the same time.

There also one (minor) question about the change: why isn't the CAS
result value being used for the failing paths of the CAS, rather than
reloaded in copy_to_survivor_space?

Thanks,
  Thomas