RFR (XS): 8059527: Disallow ParallelGCThreads=0 for G1

[Marcus Larsson]

On 02/10/14 10:53, Erik Helin wrote:
> On 2014-10-02 09:22, Marcus Larsson wrote:
>> Hi Erik,
>>
>> Thank you for the input.
>>
>> New webrev:
>> http://cr.openjdk.java.net/~mlarsson/8059527/webrev.01/
>>
>> Incremental:
>> http://cr.openjdk.java.net/~mlarsson/8059527/webrev.00-01/
>
> Changes look good, just a small comment on the message passed to
> vm_exit_during_initialization: instead of writing "The G1 GC", could you
> write "The flag -XX:+UseG1GC"? This will make it even easier for the
> user to understand what is causing the conflict.

Will fix that before pushing. Thanks for reviewing!

Marcus

>
> Thanks,
> Erik
>
>> Regards,
>> Marcus
>>
>> On 01/10/14 16:48, Erik Helin wrote:
>>> Hi Marcus,
>>>
>>> a couple of comments:
>>>
>>> - src/share/vm/runtime/arguments.cpp:
>>>    Could you use vm_exit_during_initialization instead of vm_exit?
>>>    This way, the error message will be printed in a uniform way and the
>>>    VM will also run all shutdown functions (such as os::shutdown,
>>>    os::abort etc).
>>>
>>> - test/gc/arguments/TestParallelGCThreads.java
>>>    + * @build TestParallelGCThreads FlagsValue
>>>    I don't believe this line is needed, jtreg should automically build
>>>    your files
>>>
>>>    + * @run main/othervm TestParallelGCThreads
>>>    Please use `@run driver` instead of `@run main/othervm`. You can use
>>>    'driver' because your tests creates its own processes.
>>>
>>>    Please use assertions from the testlibrary instead of throwing a
>>>    RuntimeException.
>>>
>>> - Please update hotspot/test/TEST.groups to include your new test for
>>>    needs_g1gc, needs_parallelgc and needs_cmsgc
>>>
>>> Thanks,
>>> Erik
>>>
>>> On 2014-10-01 15:22, Marcus Larsson wrote:
>>>> Hi,
>>>>
>>>> Can I have reviews for the following small patch disallowing
>>>> ParallelGCThreads to be 0 when G1 is used? Also included a small
>>>> test to
>>>> verify that the VM will not start if the count is 0 for parallel
>>>> collectors (G1, CMS+ParNew and ParallelGC).
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~mlarsson/8059527/webrev.00/
>>>>
>>>> Bug:
>>>> https://bugs.openjdk.java.net/browse/JDK-8059527
>>>>
>>>> Testing:
>>>> jprt+jtreg
>>>>
>>>> Thanks,
>>>> Marcus