RFR (S) 8135152: Create a G1ParScanThreadStateSet class for managing G1 GC per thread states
Mikael Gerdin
mikael.gerdin at oracle.com
Wed Sep 9 08:29:04 UTC 2015
Hi Erik,
On 2015-09-09 10:13, Erik Helin wrote:
> On 2015-09-08, Mikael Gerdin wrote:
>> All,
>>
>> On 2015-09-07 17:08, Mikael Gerdin wrote:
>>> Hi all,
>>>
>>> I'd like to move management of the G1ParScanThreadState instances into a
>>> ParScanThreadStateSet class, similar to how ParNew works.
>>>
>>> This first change simply moves the code for allocation/deallocation
>>> and flushing of stats from the par scan thread state to the state set
>>> object.
>>>
>>> There is another change coming (8135154) where some additional stats and
>>> counters are moved from other ad-hoc locations to the par scan thread
>>> state set.
>>>
>>> Webrev: http://cr.openjdk.java.net/~mgerdin/8135152/webrev.0/
>>
>> I got some feedback that moving all the flushing code to the pss set was not
>> particularly pretty, I agree with that statement and I've created a flush
>> method on the per thread state which is then called from the pss set flush
>> method to make the flushing process more explicit.
>>
>> Incremental webrev:
>> http://cr.openjdk.java.net/~mgerdin/8135152/webrev.0_to_1/
>> Full webrev: http://cr.openjdk.java.net/~mgerdin/8135152/webrev.1/
>
> Thanks for doing this, the patch looks good, just one minor nit:
> Expressions such as
> G1ParScanThreadState* pss = _pss->state_for_worker(worker_id);
> are a bit confusing given that `_pss` represents multiple
> G1ParScanThreadState's and `pss` is a single G1ParScanThreadState.
> Maybe we should use a different name for _pss?
>
> I'm fine with cleaning up this in a follow-up patch. Reviewed.
I'd prefer to leave this for a follow-up, given that I'm not sure
exactly how this code will settle in the end.
Thanks
/Mikael
>
> Thanks,
> Erik
>
>> /Mikael
>>
>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8135152
>>>
>>> Testing: JPRT
>>>
>>> /Mikael
>>
More information about the hotspot-gc-dev
mailing list