RFR: 6979279: remove special-case code for ParallelGCThreads==0

[Jesper Wilhelmsson]

Yes, this looks really nice! Thanks for removing the non-parallel case!
Ship it!
/Jesper


Marcus Larsson skrev 9/10/14 14:22:
> Hi Jesper,
>
>
> On 08/10/14 15:44, Jesper Wilhelmsson wrote:
>> Hi,
>>
>> Looks good in general. Two comments:
>>
>> * In compactibleFreeListSpace.cpp the code:
>>
>>   630           MutexLockerEx x(parDictionaryAllocLock(),
>>   631                           Mutex::_no_safepoint_check_flag);
>>
>> was previously executed for all values of ParallelGCThreads > 0, now it
>> is not executed when ParallelGCThreads = 1. Seems legit, just want to
>> make sure this is intentional. Are there any side effects to creating
>> this lock that is expected to happen for PGCT = 1?
>>
>
> Rather than making sure this was the case, I figured I might as well remove the
> special handling of the non-parallel case altogether, like the bug suggests. The
> only time this special case is used is if CMS+DefNew is used, which is a
> deprecated combination.
>
> I also took the opportunity and changed a few other similar cases where some
> locks were avoided if PGCT==0.
>
>> * In g1HotCardCache you remove the variable _hot_cache_par_chunk_size
>> and read directly from the flag in G1HotCardCache::drain() . I would
>> prefer to keep the variable and only read from the flag during
>> initialization. I think it would be nice if we were moving towards not
>> reading directly from command line flags during GC. There have been a
>> few cases lately where we have been in need of changing settings for the
>> GC at runtime (when making flags manageable for instance) and I think we
>> will see more of this in the future if we want to see a GC that can
>> behave differently in different situations. It is a lot easier to do
>> this if the flags can be changed at any time without needing to worry
>> about them being read during GC.
>>
>
> Fixed!
>
>> Thanks!
>> /Jesper
>>
>
>
> Additionally, I noticed that I had previously missed the fact that
> ConcurrentMark::use_parallel_marking_threads() will now always be true, so the
> updated change also removes that function and its uses.
>
> New webrev:
> http://cr.openjdk.java.net/~mlarsson/6979279/webrev.01/
>
> Incremental:
> http://cr.openjdk.java.net/~mlarsson/6979279/webrev.00-01/
>
> Thanks,
> Marcus