RFR: Eliminate extra forwarding pointer per object

[Roman Kennke]

Hi Aleksey,

>>> 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?
> 
> I don't understand off-hand why the protocol is different. For example, why do you need to drag
> old_mark in SH:evacuate_object? You can still have try_update_forwardee to check that "old" mark is
> not forwarded yet, and use that?
> 
> Pseudocode for try_update_forwardee:
> 
> // returns "true" on success
> bool try_update_forwardee(p, copy_val):
>   markOop old = p->mark_raw();
>   if (old->is_marked()):
>     return false;
>   markOop new_mark = markOopDesc::encode_pointer_as_mark(copy_val);
>   markOop prev_mark = p->cas_set_mark_raw(new_mark, old_mark);
>   return prev_mark == old_mark;
> 
> ...then:
> 
> if (try_update_forwardee(p, copy_val)):
>   shenandoah_assert_correct(NULL, copy_val);
>   return copy_val;
> } else {
>   return ShenandoahForwarding::get_forwardee(p);
> }

Ok right, I got it. I moved all forwarding related code to
ShenandoahForwarding.

I also added ShenandoahForwarding::is_forwarded(oop obj) which can be
used in the GC threads evac path where we don't care about actually
decoding and comparing the oops. Should be slightly more efficient.

>>> *) 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. ?
> 
> Yes, befriending closures would look cleaner, I think.

Done.

>> Updated patch:
>> http://cr.openjdk.java.net/~rkennke/eliminate-fwdptr/webrev.01/
> 
> Other nits:
> 
>  *) The dance like that needs some rationale/pseudo-code?
> 
>  326   Label done;
>  327   __ movptr(tmp, Address(dst, oopDesc::mark_offset_in_bytes()));
>  328   __ notptr(tmp);
>  329   __ testb(tmp, markOopDesc::marked_value);
>  330   __ jccb(Assembler::notZero, done);
>  331   __ orptr(tmp, markOopDesc::marked_value);
>  332   __ notptr(tmp);
>  333   __ mov(dst, tmp);
>  334   __ bind(done);

I added comments.

>  *) In here, you can call get_forwardee_raw_unchecked to have only one shenandoah_assert_correct assert:
> 
>   45 inline oop ShenandoahForwarding::get_forwardee(oop obj) {
>   46   shenandoah_assert_correct(NULL, obj);
>   47   return oop(get_forwardee_raw(obj));
>   48 }

Done.

New webrev:
http://cr.openjdk.java.net/~rkennke/eliminate-fwdptr/webrev.02/

Better?

Roman