RFR (XS): 8211853: Avoid additional duplicate work when a reference in the task queue has already been evacuated

[Stefan Johansson]

Looks great,
Stefan

On 2018-10-16 15:59, Thomas Schatzl wrote:
> Hi all,
>
> On Mon, 2018-10-08 at 16:58 +0200, Thomas Schatzl wrote:
>> Hi all,
>>
>>    can I have reviews for this tiny change that removes G1 doing some
>> duplicate work?
>>
>> Due to benign races we can sometimes get references that have already
>> been evacuated from the task queue.
>>
>> Current code in G1ParScanThreadState::do_oop_evac() needlessly
>> applies follow-up calculations (remembered set, humonguous liveness)
>> on this already evacuated reference - that all has been done the
>> first time around already.
> StefanJ correctly pointed out that there must more than one reason to
> be allowed to do this.
>
> - we can skip the humongous liveness check because G1 never pushes oop*
> with references outside of the collection set onto the task queue.
> Humongous objects are always logically in old gen, so we should never
> see such a reference here.
> I added an additional assert to make that clear.
>
> - the second reason for being able to exit early if the reference in
> the oop* is in old gen is because then we know that that reference has
> already been processed (as we try to only push references into the
> collection set on the task queue), and the remembered set update etc
> already done.
>
> Note that in case of evacuation failure this check is not sufficient,
> i.e. the reference is still a reference into the collection set, so G1
> still does the update_rs() again the next time we get here. However
> this is no problem either - naturally they are not references to
> humongous objects, and we do not care too much about performance in
> that rare case either (i.e. the additional update_rs() call is very
> quick anyway).
>
>> Just exit the method if we detect such a task.
> New webrev at
> http://cr.openjdk.java.net/~tschatzl/8211853/webrev.0_to_1 (diff)
> http://cr.openjdk.java.net/~tschatzl/8211853/webrev.1 (full)
> with the added assert.
>
> Passed hs-tier1-3 as expected.
>
> Thanks,
>    Thomas
>
>