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

[Roman Kennke]

Am 26.03.2018 um 21:11 schrieb John Rose:
> On Mar 26, 2018, at 6:16 AM, Roman Kennke <rkennke at redhat.com
> <mailto:rkennke at redhat.com>> wrote:
>>
>> However, in order to not cause regressions in any code I might have
>> overlooked, I did change the new stuff to inlining, and hunted down the
>> places where it breaks compilation.
> 
> Thank you; I think this is the right step even if there is evidence
> that an out-of-line oopDesc::equals might be harmless, in today's
> code base with today's benchmarks.
> 
> HotSpot is built (today) using the C++ performance model where
> inlining is the safe way to boil down abstract operations to concrete
> one.  In the case of operator==, the concrete operation is a single
> instruction (or sometimes no instructions at all, with a clever optimizer).
> 
> We know it's often harmless to replace such an optimizable operation
> with a much heavier one (a function call that won't be inlined, in this
> case).  But, in the case of *very simple* concrete operations like op==,
> there is always a risk of a future well-meant code change causing a
> sudden and surprising performance regression, as well as a rarely
> taken code path suddenly failing to scale in the field.
> 
> So our customary treatment of very simple concrete operations
> is to abstract them however we like, but express their abstractions
> in terms of inlines, wherever there might be risk of performance
> surprises.
> 
> 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.

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.

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?

Thanks and best regards,
Roman

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20180326/ebb4b47d/signature.asc>