RFR: 6979279: remove special-case code for ParallelGCThreads==0
marcus.larsson at oracle.com
Thu Oct 9 12:22:45 UTC 2014
On 08/10/14 15:44, Jesper Wilhelmsson wrote:
> 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.
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.
More information about the hotspot-gc-dev