RFR(s): 8068582: UseSerialGC not always set up properly

[Per Liden]

Hi Bengt,

On 2015-04-20 09:45, Bengt Rutisson wrote:
>
> Hi Per,
>
> On 2015-04-17 16:05, Per Liden wrote:
>> Hi,
>>
>> When no Use*GC flag is specified on the command-line we make a default
>> GC selection. In case we select SerialGC we fail to set UseSerialGC to
>> true (unlike when we select ParallelGC where we set UseParallelGC).
>> This means we can run with all Use*GC set to false (which implicitly
>> means we're using serial). It also means we can't use UseSerialGC as
>> the sole indicator that we're using serial.
>>
>> This patch makes sure that we always set the corresponding Use*GC for
>> the GC we have selected.
>>
>> A typical case where the VM selects serial but fails to set
>> UseSerialGC is when running a client VM without specifying a Use*GC
>> option on the command-line.
>>
>> Testing: Added a new jtreg test, passes jprt
>>
>> Webrev: http://cr.openjdk.java.net/~pliden/8068582/webrev.0/
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8068582
>
> The changes look good, but there are a couple of comments in the code
> that would be nice to clean up now that you enforce that UseSerialGC is
> properly set  up:
>
> genCollectedHeap.hpp:
>
>    virtual bool can_elide_initializing_store_barrier(oop new_obj) {
>      // We wanted to assert that:-
>      // assert(UseSerialGC || UseConcMarkSweepGC,
>      //       "Check can_elide_initializing_store_barrier() for this
> collector");
>      // but unfortunately the flag UseSerialGC need not necessarily always
>      // be set when DefNew+Tenured are being used.
>      return is_in_young(new_obj);
>    }
>
>
> arguments.cpp:
>
>    if (UseParNewGC && !UseConcMarkSweepGC) {
>      // !UseConcMarkSweepGC means that we are using serial old gc.
> Unfortunately we don't
>      // set up UseSerialGC properly, so that can't be used in the check
> here.
>      jio_fprintf(defaultStream::error_stream(),
>          "It is not possible to combine the ParNew young collector with
> the Serial old collector.\n");
>      return false;
>    }
>
>
> In both these cases I think we can just remove the comments. The
> suggested assert in can_elide_initializing_store_barrier() is not
> interesting to add in my opinion since it is code in GenCollectedHeap,
> which we know is only used by Serial and CMS.

Good catch. I will remove both of these.

I'll also adjust the error message there from:

  "It is not possible to combine the ParNew young collector with the 
Serial old collector.

to:

"It is not possible to combine the ParNew young collector with anything 
other than the CMS collector."

Because it doesn't makes sense to mention SerialOld when you say e.g. 
-XX:+UseParNewGC -XX:+UseParallelOldGC

Here's an updated webrev:
http://cr.openjdk.java.net/~pliden/8068582/webrev.1/

Diff against previous webrev:
http://cr.openjdk.java.net/~pliden/8068582/webrev.1-diff/

Thanks for reviewing!

/Per

>
> Thanks,
> Bengt
>
>>
>> /Per
>