RFR: 8079275: Remove CollectedHeap::use_parallel_gc_threads

Srinivas Ramakrishna ysr1729 at gmail.com
Thu May 7 02:30:46 UTC 2015


Looks good to me too.
-- Ramki

ysr1729

> On May 5, 2015, at 01:16, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
> 
> 
> 
>> On 2015-05-05 10:12, Stefan Johansson wrote:
>> Hi Stefan,
>> 
>>> On 2015-05-04 17:52, Stefan Karlsson wrote:
>>> Hi,
>>> 
>>>> On 2015-05-04 17:28, Stefan Johansson wrote:
>>>> Hi Stefan,
>>>> 
>>>>> On 2015-05-04 15:34, Stefan Karlsson wrote:
>>>>> Hi,
>>>>> 
>>>>> Please review this patch to remove CollectedHeap::use_parallel_gc_threads.
>>>>> 
>>>>> http://cr.openjdk.java.net/~stefank/8079275/webrev.01/
>>>> Change looks good, nice cleanup.
>>>> 
>>>> If I'm not missing anything you could remove the typedef in concurrentMarkSweepGeneration.cpp:
>>>>     typedef CMSParGCThreadState* CMSParGCThreadStatePtr;
>>>> -    _par_gc_thread_states =
>>>> -      NEW_C_HEAP_ARRAY(CMSParGCThreadStatePtr, ParallelGCThreads, mtGC);
>>>> +  _par_gc_thread_states = NEW_C_HEAP_ARRAY(CMSParGCThreadStatePtr, ParallelGCThreads, mtGC);
>>>> 
>>>> The typedef is only used for in the NEW_C_HEAP_ARRAY-call and I can't see any good reason for keeping it.
>>> 
>>> OK. I removed it. I also removed the unused _modUnionClosure.
>>> 
>>> http://cr.openjdk.java.net/~stefank/8079275/webrev.02.delta/
>>> http://cr.openjdk.java.net/~stefank/8079275/webrev.02/
>> Looks good,
> 
> Thanks, Stefan.
> 
> StefanK
> 
>> 
>> StefanJ
>>> Thanks,
>>> StefanK
>>> 
>>>> 
>>>> Thanks,
>>>> Stefan
>>>> 
>>>>> https://bugs.openjdk.java.net/browse/JDK-8079275
>>>>> 
>>>>> This function is used from:
>>>>> 
>>>>> 1) CMS - CMS can't be started with -XX:ParallelGCThreads=0 anymore, so all checks against use_parallel_gc_threads will always be true.
>>>>> 
>>>>> 2) GenCollectedHeap::process_roots - This usage could be replaced with n_par_threads(), the same check the other parallel task (Threads::possibly_parallel_oops_do) uses.
>>>>> 
>>>>> This will change the executed code for CMS but not Serial.
>>>>> 
>>>>> Serial will always return false for CollectedHeap::use_parallel_gc_threads and GenCollectedHeap:n_par_threads(). See:
>>>>> void GenCollectedHeap::set_par_threads(uint t) {
>>>>>  assert(t == 0 || !UseSerialGC, "Cannot have parallel threads");
>>>>>  CollectedHeap::set_par_threads(t);
>>>>> 
>>>>> CMS will always return true for CollectedHeap::use_parallel_gc_threads, but can now return either true or false for n_par_threads() depending on how n_par_threads was setup for the currently executing task. This means that some CMS paths will now call StringTable::oops_do instead of StringTable::possibly_parallel_oops_do, when running single threaded.
>>>>> 
>>>>> Thanks,
>>>>> StefanK
> 


More information about the hotspot-gc-dev mailing list