RFR: JDK-8199781: Don't use naked == for comparing oops

[John Rose]

On Mar 26, 2018, at 2:21 PM, Roman Kennke <rkennke at redhat.com> wrote:
> 
> Am 26.03.2018 um 21:11 schrieb John Rose:
>> ...
>> This is clearly one of those cases.  Let's code against surprises
>> (both present and future) by abstracting simpler operations with
>> inlines.
> 
> Thanks John for clarifying. I generally do agree with you that it makes
> sense to inline something like an operator that will compile to a single
> instruction.
> 
> Unfortunately, this new abstraction will generate a non-inlineable call.
> Thanks Erik's work with the Access API, it should be a straight call to
> the right implementation, not even a virtual call, but it's a call
> nonetheless.

What would it take to inline the call, so there's no new performance
hazard introduced?

This might be a place to add #ifdefs, if the templates don't let us
control the decision statically.

(I confess I find the templates complex and hard to reason about.
But that's a potential maintenance issue with the access API, not
your work.)

> The reason why I'm proposing it is that the GC might want to have a say
> about object equality. In the case of Shenandoah, there may be two
> copies of any one object floating around, and comparing the two copies
> might otherwise result in false negatives.

1. Of course we need a way for some GC algorithms to control
object identity.

2. It's good coding discipline to find all the places where native
op== is used and upgrade them to the needed control.

(2a. We also need a way to avoid having op== creep back in
in an uncontrolled way.)

3. We still need an unsurprising performance model for GC's
which don't require the extra identity hook.

So we're not there yet, I think.  If there's no easy way to adjust
the access API to get the last inlining, I suggest adding an #ifdef
into oopDesc::equals.

I suspect the happy final solution will be to templatize hot loops
using making the inlinable adjusted op== be part of the "decorations"
of those loops.  Then we will have two copies of hot loops in the
JVM, one with the out-of-line hook and one without.

For now, I think it's enough to have separate builds of the JVM,
one with the #ifdef set "the old way" and one that allows more
flexibility.

(Is this the first time we've run into an occasion to make a separate
Shenandaoah build?  Surely not.)

> So... with inlining oopDesc::equals() we will get one straight call
> (from the caller of oopDesc::equals() to the impl), instead of 2 (caller
> -> oopDesc::equals() -> impl).
> 
> It's the best I could come up with that does what (Shenandoah) GC needs,
> and it hasn't shown up in any performance testing that we did, yet.
> 
> Still good?

I think it's too risky to out-of-line the "cmpq" instruction.  Let's
find an agreeable way to handle that via a trade-off that keeps
the old code shape in VM builds which don't need the new code
shape.  I'm speaking up here because I don't believe this is the
last time we'll have to consider adding something gross like an
#ifdef to support new features, and I want to aim at trade-offs
that keep the existing hot paths efficient, while still allowing
new code shapes to become part of the mix.

(I'd be happy to see something cleaner than an #ifdef, of
course.  Seems to me that a constant non-product flag
-XX:+ShortCircuitOopEquals would work too, with a
binding of falseInShenandoah.)

— John