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

[Stefan Karlsson]

Hi Roman,

On 2018-03-19 19:32, Roman Kennke wrote:
> GCs might need to handle object comparison specially, for example, when
> a concurrent GC allows both from- and to-space copies of an object to be
> in use. In this case, object equality cannot be expressed by direct
> address comparsion, but must also take potential different versions of
> an object into account.
> 
> This adds a method oopDesc::equals(oop o1, oop o2), which goes through
> the Access API to compare objects. Default impl does ==
> 
> It is based on our finds from Shenandoah. We do, in fact, have some
> extra code in oopsHierarchy.hpp to detect naked oop==oop (with
> CheckUnhandledOops enabled), but it requires that code that does really
> want naked == (e.g. GCs) to use another method oopDesc::unsafe_equals()
> instead, and code that wants to use == and assert that the comparison is
> safe (e.g. both objects are already in to-space) use
> oopDesc::safe_equals(). Not sure if you want that?
> 
> Also, I'd have preferred to mark equals() as inline in oop.hpp, but that
> pulls in a rats nest of dependencies problems. I hope the way I did it
> is acceptable?


We have actively been working on cleaning up our include dependencies, 
so this should be dealt with properly before this patch can be accepted. 
All files that use oopDesc::equals need to include oop.inline.hpp. The 
usages of oopDesc::equals in .hpp needs to move to either .cpp files or 
.inline.hpp files.

See the files section on this page:
https://wiki.openjdk.java.net/display/HotSpot/StyleGuide

Currently, you need to include access.inline.hpp to use the Access API. 
I wish this wasn't the case, and that we somehow hide the GC internal 
dependencies, so that the call sites didn't have to bring all those 
dependencies. Erik Ö used to have a facility to generate the runtime 
dispatched functions out-of-line. Maybe if we build upon this, we could 
get rid of the need to include access.inline.hpp when using the Access 
API, and oopDesc::equals could be placed in oopDesc::equals. However, we 
don't have this in place (yet) so we need to play along with the set 
rules for .inline.hpp files.

Thanks,
StefanK

> 
> There's one little thing in growableArray.hpp: in order to do the right
> thing for GrowableArray<oop> I needed to provide a special templated
> comparison function, with a specialized version for oop.
> 
> It also overloads the == operator in Handle to do the right thing.
> 
> Webrev:
> http://cr.openjdk.java.net/~rkennke/JDK-8199781/webrev.00/
> 
> Testing: built fastdebug/release, tier1_runtime, tier1_gc
> 
> Please review!
> 
> Thanks, Roman
>