RFR (S): 8152438: Threads may do significant work out of the non-shared overflow buffer

[Jon Masamitsu]

Thomas,

Looks good.

Jon

On 05/11/2016 06:05 AM, Thomas Schatzl wrote:
> Hi all,
>
> On Wed, 2016-05-11 at 09:35 +0200, Thomas Schatzl wrote:
>> Hi all,
>>
>>    can I have reviews for the following change that fixes signficiant
>> imbalance in work performed during GC?
>>
>> We observed that threads in certain situations put a lot of
>> references
>> into the thread-private overflow queue. Since this overflow queue is
>> thread-local, and other threads may not steal work from this queue,
>> it
>> happens that sometimes the parallel copying phase is severely
>> serialized.
>>
>> This is particularly problematic on large machines with many threads
>> but comparatively little work to do. In one situation on a real world
>> application, a 200% overall throughput improvement has been observed
>> by
>> this change.
>>
>> The CR mentions a micro-benchmark that exhibits the same problems.
>>
>> The proposed solution is to, for a particular thread, if it processes
>> entries from the overflow queue, first try to just move the work item
>> to the shared queue where other threads can steal from. Only process
>> the item directly if the shared queue is full already.
>>
>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8152438
>> Webrev:
>> http://cr.openjdk.java.net/~tschatzl/8152438/webrev/
>> Testing:
>> jprt, regular perf testing with no apparent pause time regressions,
>> some benchmarks
>    had some offline discussion with Erik about the change, and the main
> gripe with the change has mainly been putting additional policy code
> about how the OverflowTaskQueue works somewhere completely unrelated,
> in this case G1ParScanThreadState::trim_queue().
>
> Looking at other uses, we found that all of these do the same, and it
> would be much better to have collector/case specific
> "OverflowTaskQueue"s that encapsulate the behavior. This would also
> allow potential sharing or at least comparison of them.
>
> We felt that looking into that is out of scope for this change (apart
> from one minor renaming of try_push()), and I filed JDK-8156754 for
> this other work.
> Another point came up was why not doing bulk-move of the queue elements
> from the overflow queue to the task queue. Due to already being late
> for FC, we also moved this to an RFE, JDK-8156739.
>
> http://cr.openjdk.java.net/~tschatzl/8152438/webrev.1 (full)
> http://cr.openjdk.java.net/~tschatzl/8152438/webrev.0_to_1 (diff)
>
> Thanks,
>    Thomas