RFR(S): 8068579: Running with -XX:-UseParallelGC does not turn ParalleGC off

Wed Mar 23 16:52:57 UTC 2016

On 03/23/2016 01:02 AM, Per Liden wrote:
> On 2016-03-22 18:51, Jon Masamitsu wrote:
>>
>>
>> On 3/22/2016 12:37 AM, Per Liden wrote:
>>> Hi Jon,
>>>
>>> On 2016-03-21 21:53, Jon Masamitsu wrote:
>>>>
>>>>
>>>> On 03/18/2016 03:12 PM, Per Liden wrote:
>>>>> Hi Jon,
>>>>>
>>>>>> On 18 Mar 2016, at 19:00, Jon Masamitsu <jon.masamitsu at oracle.com>
>>>>>> wrote:
>>>>>>
>>>>>> Per,
>>>>>>
>>>>>> http://cr.openjdk.java.net/~pliden/8068579/webrev.1/src/share/vm/runtime/arguments.cpp.frames.html 
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Why the change to make UseSerialGC as ergonomically selected?
>>>>>>
>>>>>> 1856 FLAG_SET_ERGO_IF_DEFAULT(bool, UseSerialGC, true);
>>>>> This is to have a consistent way of selecting GC across all VM
>>>>> variants. We end up in this code when the user didn’t specify any GC,
>>>>> so the VM selects one based on various rules. The selection of
>>>>> SerialGC here is not really special or different from the other 
>>>>> cases,
>>>>> it just happens to be the outcome of a very simple rule in minimal 
>>>>> VMs.
>>>>
>>>> I didn't ask the question correctly.  You used
>>>> FLAG_SET_ERGO_IF_DEFAULT()
>>>> instead of (I think) FLAG_SET_DEFAULT().  Why did you want to add the
>>>> "ERGO" attribute to the setting?
>>>
>>> My reply above tried to answer this, but let's me rephrase and hope it
>>> becomes more clear.
>>>
>>> Before this change, unless a GC was specified on the command-line the
>>> VM ergonomically select a GC using FLAG_SET_ERGO. There was however
>>> one exception to this, a minimal VM selected the SerialGC using
>>> FLAG_SET_DEFAULT. The change to also use FLAGS_SET_ERGO in minimal VMs
>>> is an attempt to make the way we select GC consistent across all VM
>>> variants. I.e. whenever the VM selects the GC for you we always use
>>> FLAG_SET_ERGO instead of like before where we sometimes use
>>> FLAG_SET_ERGO and sometimes FLAG_SET_DEFAULT. After all, ERGO means
>>> (to me at least) the VM made a decision for me.
>>>
>>> I hope this helps.
>>
>> Ok.  I get it this time.
>>
>> Saying ERGO made a decision in the case of the minimal VM seems to me
>> to say too much.  It says that the VM made a decision when there really
>> wasn't a decision to be made - the only choice is the SerialGC. I don't
>> object
>> to the change integrating as if but it would be simpler in my mind to 
>> leave
>> it as FLAG_SET_DEFAULT.
>
> Ok, thanks Jon. I understand you point. I guess it's a kind of a 
> philosophical question whether you have actually selected something 
> when there was only one this to choose from. 
:-)

> I'm thinking you could make the argument both ways. For the sake of 
> consistency across VM variants I'd like to stick with ERGO here.

Ok.  Enough said on this.  Let it stand.

Jon

>
> thanks,
> Per
>
>>
>> Jon
>>
>>>
>>> cheers,
>>> Per
>>>
>>>>
>>>> Jon
>>>>
>>>>>
>>>>> cheers,
>>>>> Per
>>>>>
>>>>>> Jon
>>>>>>
>>>>>> On 3/17/2016 6:36 AM, Per Liden wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 2016-03-16 09:12, Per Liden wrote:
>>>>>>>> Hi Dima,
>>>>>>>>
>>>>>>>> On 2016-03-15 17:38, Dmitry Fazunenko wrote:
>>>>>>>>> Hi Per,
>>>>>>>>>
>>>>>>>>> A couple of comments:
>>>>>>>>> 1) globals_extension.hpp.frames.html
>>>>>>>>> requires copyright update
>>>>>>>>> 2) such fix should go with a regression test, it could be done
>>>>>>>>> separately.
>>>>>>>> Good point. I was about to add a test for this just now and 
>>>>>>>> realized
>>>>>>>> that the new semantics is not kept for minimal VMs (i.e. when
>>>>>>>> INCLUDE_ALL_GCS==0). I'll rework the patch a bit to have the same
>>>>>>>> semantics also on minimal VMs. New webrev coming.
>>>>>>> Here's an updated webrev. The new semantics is now available on all
>>>>>>> VMs (including minimal). I also added a new test for this.
>>>>>>>
>>>>>>> This change also removes UNSUPPORTED_GC_OPTION() and instead reuses
>>>>>>> the existing UNSUPPORTED_OPTION(), which I tweaked slightly to make
>>>>>>> the output more consistent across the code base. This in turn meant
>>>>>>> that I needed to adjust some other tests.
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~pliden/8068579/webrev.1/
>>>>>>>
>>>>>>> Testing: jprt, manual
>>>>>>>
>>>>>>> thanks,
>>>>>>> Per
>>>>>>>
>>>>>>>> thanks,
>>>>>>>> Per
>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Dima
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 15.03.2016 16:48, Per Liden wrote:
>>>>>>>>>> Summary: Explicitly disabling the default collector, without
>>>>>>>>>> specifying another collector, will silently re-enable the 
>>>>>>>>>> default
>>>>>>>>>> collector. This is a bit counter intuitive. A better approach
>>>>>>>>>> would be
>>>>>>>>>> for the VM to complain about the situation.
>>>>>>>>>>
>>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8068579
>>>>>>>>>> Webrev: http://cr.openjdk.java.net/~pliden/8068579/webrev.0/
>>>>>>>>>> Testing: manual, jprt
>>>>>>>>>>
>>>>>>>>>> Example (assuming it's a platform/configuration with G1 as
>>>>>>>>>> default):
>>>>>>>>>>
>>>>>>>>>> $ java -XX:-UseG1GC ...
>>>>>>>>>>
>>>>>>>>>> Today, the above command will happily start a VM with G1 
>>>>>>>>>> enabled.
>>>>>>>>>>
>>>>>>>>>> With this patch the VM will instead complain, like this:
>>>>>>>>>>
>>>>>>>>>> $ java -XX:-UseG1GC ...
>>>>>>>>>> Error occurred during initialization of VM
>>>>>>>>>> Garbage collector not selected (default collector explicitly
>>>>>>>>>> disabled)
>>>>>>>>>>
>>>>>>>>>> And of course, we're good with disabling the default 
>>>>>>>>>> collector as
>>>>>>>>>> long
>>>>>>>>>> as you specify some other collector, e.g.:
>>>>>>>>>>
>>>>>>>>>> $ java -XX:-UseG1GC -XX:+UseParallelGC ...
>>>>>>>>>>
>>>>>>>>>> cheers,
>>>>>>>>>> Per
>>>>
>>