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

[Jon Masamitsu]

Thomas,

Still your call with regard to using OopAndMarkOop.  You made all
the changes.  If you don't like them, that's fine.

Jon

On 7/21/2015 11:28 AM, Thomas Schatzl wrote:
> Hi Jon,
>
> On Tue, 2015-07-21 at 11:08 -0700, Jon Masamitsu wrote:
>> On 07/20/2015 07:16 AM, Thomas Schatzl wrote:
>>> 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.
>> If I understand this comment correctly, yes it does require
>> checking but just as it requires checking when the read of the
>> markoop was done (the one read of the markoop).  It seemed
> I meant checking as in "do performance checking by running benchmarks on
> all systems and look through logs for changes". Sorry for being obtuse.
>
>> to me to be modestly more clear with now that there was
>> an OopAndMarkOop (renamed OopAndOldMarkOop or
>> OopAndPreservedMarkOop which I like even better) but
>> sometimes less code is better (i.e., simpler not to use
>> OopAndMarkOop data structure) so your call.
> I agree that less code is typically better.
>
>>> 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/).
>> The diff (0_to_1) seem to be only the deletion of some code,
>> not a renaming but maybe a moot point now if you still
>> like explicitly reading the value of the markoop that you
>> want to capture.
>    better link:
> http://cr.openjdk.java.net/~tschatzl/8004687/webrev.refactor/
>
> Sorry, I already overwrote the original with implementation of Mikael's
> suggestion.
>
> Thanks,
>    Thomas
>
>