RFR: 8275527: Separate/refactor forward pointer access
Stefan Karlsson
stefank at openjdk.java.net
Tue Oct 19 21:02:05 UTC 2021
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
I like how the mark/is_marked/decode_pointer code gets cleaned up. And having an OopForwading class probably makes sense for parts of the code.
I'm less excited about the way the explicitly stack allocated OopForwarding objects make the code less readable. Take these two examples:
assert(old->is_objArray(), "invariant");
- assert(old->is_forwarded(), "invariant");
+ assert(OopForwarding(old).is_forwarded(), "invariant");
oop new_obj = obj->is_forwarded() ? obj->forwardee()
: _young_gen->copy_to_survivor_space(obj);
OopForwarding fwd(obj);
oop new_obj = fwd.is_forwarded() ? fwd.forwardee()
: _young_gen->copy_to_survivor_space(obj);
I think both snippets were nicer to read in the earlier code. And there's a lot of those kind of changes in this patch.
Maybe there's a hybrid approach where we could still have oopDesc::is_forwarded, oopDesc::forwardee, etc. but also have OopForwarding objects for those places that really need them? And then minimize the amount of places we use `OopForwarding fwd(obj)` and `OopForwarding(obj)->forwardee()` in the code?
-------------
Changes requested by stefank (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/5955
More information about the shenandoah-dev
mailing list