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

[Mikael Gerdin]

Hi Thomas,

On 2015-07-21 19:13, Thomas Schatzl wrote:
> Hi Mikael,
>
> On Tue, 2015-07-21 at 17:07 +0200, Mikael Gerdin wrote:
>> Hi Thomas,
>>
>> On 2015-07-16 11:40, Thomas Schatzl wrote:
>>> (resend with CR number in the subject)
>>> Hi all,
>>>
>>>     can I have reviews for the following change from a student (Walter
>>> Gugenberger; he has signed the OCA) who worked on this issue last
>>> semester?
>>>
>>> The change parallelizes object self-forwarding and scanning during an
>>> evacuation failure by:
>>> - reuse the entire existing evacuation copy closure and work queue
>>> mechanism in case of evacuation failure to store the work items.
>>> This allows evacuation failure to automatically benefit from work
>>> stealing etc.
>>> - remove the dedicate evacuation failure handling closure because it is
>>> not necessary any more.
>>>     - there is one subtle change in behavior: the old evacuation failure
>>> always had a NULL ReferenceProcessor, while now the standard reference
>>> processor for references is applied. Since the NULL ReferenceProcessor
>>> kept all references alive, now potentially less references will be kept
>>> alive after an evacuation failure. I do not see a problem here.
>>
>> Right. Another difference is that the evac failure closure
>> unconditionally called update_rs for each oop while the scanner closure
>> only calls update_rs for non-cset oops and defers the cset updating to
>> when a reference is popped from the work queue.
>
> I did not think they were worth mentioning, but you are right.
>
>>> - implementing per-thread preserved mark buffers
>>>
>>> As for actual performance improvements, there is none: the main problem
>>> is that in case of evacuation failure the code will simply serialize on
>>> the FreeList_lock when trying to get a new allocation region instead of
>>> the EvacFailureStack_lock.
>>>
>>> Evacuation failure and the early-exit for evac failure as suggested in
>>> the CR will only be really effective when the FreeList_lock can be
>>> avoided. This will be added in a follow-up change.
>>>
>>> CR:
>>> https://bugs.openjdk.java.net/browse/JDK-8004687
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~tschatzl/8004687/webrev/
>>
>> Overall, a nice cleanup regardless of its lack of a general performance
>> improvement.
>
> Just waiting that this change gets reviewed, and then we are going look
> at the next one :)
>
>> I think that this change removes the last use of G1BarrierEvac, so the
>> code for that in G1ParCopyClosure::do_oop_work can probably be removed.
>
> I prepared a webrev that removes G1BarrierEvac related code too at
>
>   http://cr.openjdk.java.net/~tschatzl/8004687/webrev.1 (full)
>   http://cr.openjdk.java.net/~tschatzl/8004687/webrev.0_to_1 (diff)

Looks good to me.

/Mikael

>
> More deletions basically :)
>
> Thanks,
>    Thomas
>
>