RFR: 8190408: Run G1CMRemarkTask with the appropriate amount of threads instead of starting up everyone

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8190408: Run G1CMRemarkTask with the appropriate amount of threads instead of starting up everyone

Leo Korinth
Hi,

Removing an if statement. The if-statement did safeguard that the
worker_id was always within the number of active tasks in _cm.

However the active number of tasks in _cm is set directly before the
remark task is created (set_concurrency_and_phase()). The value is
taken from the work gang (g1h->workers()->active_workers()). Thus, as
set_concurrency*() is not called during the actual remarking, the size
of the work-gang will be in sync with the _cm->active_tasks() -- at
least during the remark phase.

The code becomes somewhat easier to read, and hopefully one does not
get confused that we have too many threads running.

Bug:
https://bugs.openjdk.java.net/browse/JDK-8190408

Webrev:
http://cr.openjdk.java.net/~lkorinth/8190408/00/

Testing:
- hs-tier1, hs-tier2

Thanks,
Leo
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8190408: Run G1CMRemarkTask with the appropriate amount of threads instead of starting up everyone

Thomas Schatzl
Hi,

On Tue, 2017-11-21 at 15:56 +0100, Leo Korinth wrote:

> Hi,
>
> Removing an if statement. The if-statement did safeguard that the
> worker_id was always within the number of active tasks in _cm.
>
> However the active number of tasks in _cm is set directly before the
> remark task is created (set_concurrency_and_phase()). The value is
> taken from the work gang (g1h->workers()->active_workers()). Thus, as
> set_concurrency*() is not called during the actual remarking, the
> size
> of the work-gang will be in sync with the _cm->active_tasks() -- at
> least during the remark phase.
>
> The code becomes somewhat easier to read, and hopefully one does not
> get confused that we have too many threads running.
>
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8190408
>
> Webrev:
> http://cr.openjdk.java.net/~lkorinth/8190408/00/
>

Looks good.

Thanks,
  Thomas
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8190408: Run G1CMRemarkTask with the appropriate amount of threads instead of starting up everyone

Stefan Johansson


On 2017-11-21 16:09, Thomas Schatzl wrote:

> Hi,
>
> On Tue, 2017-11-21 at 15:56 +0100, Leo Korinth wrote:
>> Hi,
>>
>> Removing an if statement. The if-statement did safeguard that the
>> worker_id was always within the number of active tasks in _cm.
>>
>> However the active number of tasks in _cm is set directly before the
>> remark task is created (set_concurrency_and_phase()). The value is
>> taken from the work gang (g1h->workers()->active_workers()). Thus, as
>> set_concurrency*() is not called during the actual remarking, the
>> size
>> of the work-gang will be in sync with the _cm->active_tasks() -- at
>> least during the remark phase.
>>
>> The code becomes somewhat easier to read, and hopefully one does not
>> get confused that we have too many threads running.
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8190408
>>
>> Webrev:
>> http://cr.openjdk.java.net/~lkorinth/8190408/00/
>>
> Looks good.
+1
StefanJ
>
> Thanks,
>    Thomas