RFR: 8275527: Separate/refactor forward pointer access

[Stefan Karlsson]

On Thu, 14 Oct 2021 16:37:02 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

> Accessing the forward pointer overlay of object headers is currently done in a somewhat crude way. oopDesc::forwardee() and oopeDesc::is_forwarded() both load the header-word. This seems kinda bad, because they are most often used in conjunction, e.g.:
> fwd = obj->forwarded() ? obj->forwardee() : promote_obj();
> 
> or similar constructs.
> 
> Also, in some places, the forwardee is accessed in a more direct fashion using markWord::decode_pointer(), or as HeapWord*, sometimes is_forwarded() is determined by forwardee() == NULL checks (and crude overrides to prepare non-forwarded headers to return NULL in such checks, see g1Full* source files).
> 
> I propose to extract and refactor forward pointer access in a way to better support above pattern. In-fact I originally wrote this with performance-enhancement in mind (see oopForwarding.hpp header for more details), but I could not show any actual performance improvements, so let's consider this purely on cleanliness and separation-of-concerns basis.
> 
> Testing:
>  - [x] tier
>  - [x] tier2
>  - [x] hotspot_gc

A few more comments:

src/hotspot/share/gc/g1/g1FullGCCompactionPoint.cpp line 112:

> 110:       // since it will be restored by preserved marks.
> 111:       object->init_mark();
> 112:     } else {

Could you explain why this was removed?

src/hotspot/share/gc/parallel/psPromotionManager.inline.hpp line 142:

> 140:   OopForwarding fwd(o);
> 141:   if (!fwd.is_forwarded()) {
> 142:     return copy_unmarked_to_survivor_space<promote_immediately>(o, fwd);

I wonder if this copy_unmarked should be renamed to copy_unforwarded?

src/hotspot/share/gc/serial/defNewGeneration.cpp line 56:

> 54: #include "oops/instanceRefKlass.hpp"
> 55: #include "oops/oop.inline.hpp"
> 56: #include "oops/oopForwarding.hpp"

The patch is inconsistent w.r.t. the include order between oop.inline.hpp and oopForwarding.hpp.

src/hotspot/share/oops/oopForwarding.hpp line 83:

> 81: #endif
> 82: #endif
> 83:   }

This should preferable be in a .cpp file, though it seems excessive to introduce one just for this case. Since it is only used by the forward_to functions, maybe add it to the suggested oopForwarding.inline.hpp file?

src/hotspot/share/oops/oopForwarding.hpp line 125:

> 123:       return cast_to_oop(old_mark.decode_pointer());
> 124:     }
> 125:   }

The forward_to functions uses functions in oop.inline.hpp. They need to be split out into a oopForwarding.inline.hpp file.

-------------

PR: https://git.openjdk.java.net/jdk/pull/5955