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

Roman Kennke rkennke at redhat.com
Wed Mar 21 16:58:01 UTC 2018


Am 20.03.2018 um 17:06 schrieb 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
> 

The problem here is that it involves oop.hpp and handles.hpp which are
included very broadly all over the code base, and that my original
attempt was to make equals inline.

The obvious way out is to avoid that and make oopDesc::equals and the
Handle::== operators non-inline and put them in the regular .cpp files.
Looking at all the uses of equals, I don't see much that looks like it
would benefit from inlining anyway. I believe that inline is over-used
in OpenJDK too. It might be better to start out with regular methods and
*if* they show up in profiles, consider to inline them. Would that be
acceptable?

Differential:
http://cr.openjdk.java.net/~rkennke/JDK-8199781/webrev.01.diff
Full:
http://cr.openjdk.java.net/~rkennke/JDK-8199781/webrev.01

Give it a little while to upload, it's lots of stuff, and my connection
is tiny ;-)

Roman



More information about the hotspot-runtime-dev mailing list