RFR (M): 8004687: G1: Parallelize object self-forwarding and scanning during an evacuation failure

[Thomas Schatzl]

Hi Jon,


On Fri, 2015-07-17 at 12:57 -0700, Jon Masamitsu wrote:
> Thomas,
> 
> Changes look good.

  thanks for the review.

> 
> Would it work to change
> 
> OopAndMarkOop
> to
> OopAndOldMarkOop
> 
> and change the constructor to
> 
> 866     OopAndMarkOop(oop obj) : _o(obj), _m(markOop(obj))
> 
> so that OopAndOldMarkOop specifically saves the oop and it's
> markword rather than just an oop and (not necessarily related)
> markword?

  the only problem I see is that we earlier (8006025) changed the code
to explicitly not re-read the markoop from the obj multiple times. The
markoop is volatile, so it prevents some optimizations which we want
here.

It may not be a huge problem or one at all in this particular case,
depending on where the OopAndOldMarkOop is instantiated, but requires
checking.

I put a webrev with pure renaming of the struct at
http://cr.openjdk.java.net/~tschatzl/8004687/webrev.1/ (diff webrev:
http://cr.openjdk.java.net/~tschatzl/8004687/webrev.0_to_1/).

I do not mind either version.

> An enhancement to be considered now or later. You have
> OopAndMarkOop now.  Could you pass an OopAndMarkOop
> around instead of  an oop and a markword pair.  For
> example, starting with
> 
> 201 oop copy_to_survivor_space(InCSetState const state, oop const obj, 
> markOop const old_mark);
> 
> substitute passing an OopAndMarkOop in place of the obj and old_mark.

Prototyping this it does not seem to be huge gain. The use and passing
of a struct is only really advantageous in a few places (in
copy_to_survivor_spaces()). In many places the existing code manipulates
the old object and old mark quite a bit, adding a "obj_and_mark" (if we
call the parameter obj_and_mark) prefix in quite a few places only.

I put that change at
http://cr.openjdk.java.net/~tschatzl/8004687/webrev.refactor/.
Particularly lines 250-265 in g1ParScanThreadState.cpp seem to be more
obfuscated than before. There is opportunity to move the entire mark
update method into OopAndOldMarkOop, but unless there is something I
omitted, I would like to defer it (and the necessary performance
checking work) to a later point.

> Reviewed.

Thanks,
  Thomas