RFR: Eliminate extra forwarding pointer per object

Aleksey Shipilev shade at redhat.com
Tue May 7 16:20:55 UTC 2019


On 5/7/19 5:59 PM, Roman Kennke wrote:
> 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?

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

*) 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?

*) 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).

*) 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.

*) In ShenandoahMarkCompact, do these need to be public?

  76 public:
  77   void preserve_mark(oop obj);
  78   void restore_marks();
  79   void adjust_marks();

-Aleksey



More information about the shenandoah-dev mailing list