RFR(XXS): 8006894: G1: Number of marking threads missing from PrintFlagsFinal output

[John Cuthbertson]

Hi Ramki,

CMS does do the correct thing. Here's the code:

   // Support for multi-threaded concurrent phases
   if (CMSConcurrentMTEnabled) {
     if (FLAG_IS_DEFAULT(ConcGCThreads)) {
       // just for now
       FLAG_SET_DEFAULT(ConcGCThreads, (ParallelGCThreads + 3)/4);
     }
     if (ConcGCThreads > 1) {
       _conc_workers = new YieldingFlexibleWorkGang("Parallel CMS Threads",
                                  ConcGCThreads, true);
       if (_conc_workers == NULL) {
         warning("GC/CMS: _conc_workers allocation failure: "
               "forcing -CMSConcurrentMTEnabled");
         CMSConcurrentMTEnabled = false;
       } else {
         _conc_workers->initialize_workers();
       }
     } else {
       CMSConcurrentMTEnabled = false;
     }
   }

I guess an argument could be made that it should only set ConcGCThreads 
if the value is 0. If a non-zero default was given then we override that 
default. Also I think we should set ConcGCThreads using FLAG_SET_ERGO. 
Using FLAG_SET_ERGO when we're ergonomically turning off 
CMSConcurrentMTEnabled (because the # of threads is 1 or the allocation 
of work gang fails).

But the main answer is I see a non-zero value for ConcGCThreads in the 
PrintFlagsFinal output:

     uintx ConcGCThreads                             = 2               
{product}

JohnC


On 1/24/2013 5:33 PM, John Cuthbertson wrote:
> Hi Ramki,
>
> Thanks for looking at the change. I'll generate the output for CMS 
> later tonight. From conversations with the perf team I believe CMS is 
> OK in this regard.
>
> JohnC
>
> On 1/24/2013 5:29 PM, Srinivas Ramakrishna wrote:
>> Looks good to me too. (Just out of curiosity, what happens with CMS, 
>> is it correctly reported/set, or does it have the same issue -- i am 
>> not suggesting fixing it given the EOL plans for CMS; just wondered. 
>> Hmm, I think in CMS we directly use the flag variable, so should 
>> probably report fine.)
>>
>> -- ramki
>>
>> On Thu, Jan 24, 2013 at 3:01 PM, John Cuthbertson 
>> <john.cuthbertson at oracle.com <mailto:john.cuthbertson at oracle.com>> wrote:
>>
>>     Hi All,
>>
>>     Can I have a couple of volunteers look over this small change?
>>     The webrev can be found at:
>>     http://cr.openjdk.java.net/~johnc/8006894/webrev.0/
>>     <http://cr.openjdk.java.net/%7Ejohnc/8006894/webrev.0/>
>>
>>     Summary:
>>     When G1 calculates the number of marking threads based upon (the
>>     develop-only) G1MarkingOverheadPercent or (more usually)
>>     ParallelGCThreads, we weren't setting the value of ConcGCThreads.
>>     As a result the output of PrintFlagsFinal would always show a
>>     zero if ConcGCThreads wasn't specified on the command line:
>>
>>         uintx ConcGCThreads                             = 0          
>>     {product}
>>
>>     This made it difficult for the performance team to analyze
>>     marking behavior and offer advice. With this change we now get
>>     the calculated number of marking threads:
>>
>>     Using ParallelGCThreads (default: 4):
>>
>>         uintx ConcGCThreads                            := 1          
>>     {product}
>>
>>     Using G1MarkingOverheadPercent (50):
>>
>>         uintx ConcGCThreads                            := 2          
>>     {product}
>>
>>     Testing:
>>     Command line testing; specjvm98 and dacapo with a low IHOP value
>>     (marking threshold).
>>
>>     Thanks,
>>
>>     JohnC
>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20130125/7d9b12fc/attachment.htm>