RFR: JDK-8199781: Don't use naked == for comparing oops (original) (raw)

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::unsafeequals() 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::safeequals(). 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

-------------- 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/20180321/32ddaf2d/signature.asc>

More information about the hotspot-gc-dev mailing list