RFR: Eliminate extra forwarding pointer per object

[Roman Kennke]

Hi Aleksey,

>> I propose to push it to shenandoah/jdk and bake it there a little before upstreaming it into jdk/jdk.
> 
> This plan is fine with me; but let's review it first.
> 
>> Webrev:
>> http://cr.openjdk.java.net/~rkennke/eliminate-fwdptr/webrev.00/
> 
> Brief look:
> 
> *) In shenandoahBarrierSetAssembler_aarch64.cpp there is comment about temp registers in
> SBSA::resolve_forward_pointer_not_null. Yet, new code does clobber them? Maybe the comment is
> obsolete now?

I changed the comment to allow using explicitely passed in tmp 
registers. :-)

> *) Maybe we should fork introducing new tmp to CAS? That would make the patch easier to backport, I
> think.

Not sure. The change would make the register allocators free one more 
register but then not use it. I left it out of this iteration for now.

> *) Is there a compelling reason why we drop the midpath in SBSA::load_reference_barrier_not_null and
> call into slowpath? I.e. is it required, or is it opportunistic optimization? Maybe it just works
> around the complexity of parsing out the fwdptr?

It works around the complexity of getting an extra register for 
decoding. This is much easier in the stub.

> *) In shenandoahAsserts, we deliberately not going via resolves, instead doing
> SBP::get_raw_unchecked. Just replacing them with SBS::resolve_forwarded_not_null does not feel
> sufficient. Instead, asserts/verifier needs to be adjusted to decode the fwdptrs properly (if present).

I re-instated the utility class as ShenandoahForwarding and this updated 
change keeps those utilities.

> *) It seems in SH::evacuate_object, this needs to call some utility method?
> 
>   236   markOop old_mark = p->mark_raw();
>   237   if (old_mark->is_marked()) {
>   238     return (oop) old_mark->clear_lock_bits();
>   239   }
> 
> ...and here?
> 
>   281   markOop new_mark = markOopDesc::encode_pointer_as_mark(copy_val);
>   282   markOop prev_mark = p->cas_set_mark_raw(new_mark, old_mark);
>   283   if (prev_mark == old_mark) {


> I actually think that we can keep ShenandoahBrooksPointer and put all that util code there. It would
> still be doing its job: abstracting away the forwarding data handling.


I reinstated the ShenandoahForwarding utility class. However, it is hard 
to abstract the protocol because the two parts strictly belong together. 
However, this seems all the more reason to abstract it properly. The 
only thing that happens in between the two blocks is actually allocating 
and copying the object. So we could call into, say, 
ShenandoahForwarding::evacuate() which would do the prefix and suffix, 
and call back to say copy = ShHeap::allocate_and_copy(obj); or such for 
the payload. What do you think?

> *) In ShenandoahMarkCompact, do these need to be public?
> 
>    76 public:
>    77   void preserve_mark(oop obj);
>    78   void restore_marks();
>    79   void adjust_marks();

I forgot to mention ShenandoahMarkCompact. This completely overhauls how 
ShMarkCompact stores forwarding information. It's changed to basically 
work like other GCs and stores the forwarding in the mark-word. However, 
the mark word must sometimes be preserved. Therefore we store it to the 
side. The 3 methods above handle that. They are public because they are 
called from closures. Alternatively, we could make the closures friends 
of ShMarkCompact. ?

Updated patch:
http://cr.openjdk.java.net/~rkennke/eliminate-fwdptr/webrev.01/

It's based on:
http://cr.openjdk.java.net/~rkennke/renamebrooksptr/webrev.00/

Still passes all tests.

Roman