Request for review (s) : 8017462: G1: guarantee fails with UseDynamicNumberOfGCThreads

[Bengt Rutisson]

On 2015-03-27 05:33, Jon Masamitsu wrote:
> The fix for 8017462 has been changed to adopt the suggestion from
> Bengt (thanks, Bengt).  The patch from Bengt has been enhanced to
> include some of the changes in this first version that did not make
> it into Bengt's original patch.  The test remains the same.  A CR has 
> been
> created to add another test to check the consistency of one of
> the per thread statistics with the sum of the statistics over all the
> threads (e.g., per thread object copy time and total object copy time)
> https://bugs.openjdk.java.net/browse/JDK-8076140
>
> http://cr.openjdk.java.net/~jmasa/8017462/webrev.01/

Thanks for adopting this patch. I think it looks good now. :)

A couple of questions about the test case.

The proposed test will run with any GC configuration since it just 
discards the JTreg configuration and selects its own GC. You have 
specified @requires vm.gc=="null", which makes sense since we don't have 
to run this test in every configuration. The test will anyway always run 
with the GCs that it specifies itself. However, the @requires tag will 
mean that the test is only run if no GC is specified. Currently there is 
a "noopt" baseline in our nightly testing that runs without explicitly 
setting a GC, so this test will be run there. But if we change the 
nightly baselines to always specify a GC we will never run this test.

I guess this is a more general question. Should we use @requires to 
avoid running the same test many times or should we only use it when the 
test actually requires a specific GC configuration?

Also, the test use a lot of flags:

"-XX:+UnlockExperimentalVMOptions", "-XX:G1LogLevel=finest", "-XX:+" + 
gcFlag, "-Xmx10M", "-showversion", "-XX:+PrintGCDetails", 
"-XX:+UseDynamicNumberOfGCThreads", "-XX:+TraceDynamicGCThreads"


I think this should be enough:

"-XX:+" + gcFlag, "-Xmx10M", "-XX:+UseDynamicNumberOfGCThreads", 
"-XX:+TraceDynamicGCThreads"

Is there a particular reason to have the other flags?

Thanks,
Bengt

>
> On 3/10/2015 9:26 AM, Jon Masamitsu wrote:
>> 8017462: G1: guarantee fails with UseDynamicNumberOfGCThreads
>>
>> https://bugs.openjdk.java.net/browse/JDK-8017462
>>
>> When fewer than the maximum number of threads was being used for
>> a parallel section, phase times for the threads that did not execute and
>> averages for the phase were misleading.  The fix passes the active 
>> number of
>> GC threads  to the G1 phase timers.
>>
>> http://cr.openjdk.java.net/~jmasa/8017462/webrev.00/
>>
>> Tested with gc_test_suite.
>