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

Marcus Larsson marcus.larsson at oracle.com
Thu Oct 9 12:22:45 UTC 2014


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


More information about the hotspot-gc-dev mailing list