Open code review for 8061999 Enhance VM option parsing to allow options to be specified

[David Holmes]

On 26/06/2015 2:49 AM, Daniel D. Daugherty wrote:
> On 6/25/15 12:52 AM, David Holmes wrote:
>> On 25/06/2015 4:38 PM, Dmitry Dmitriev wrote:
>>> Hello David,
>>>
>>> I mean that we can define "-Dmy.property=value" inside VM Option file.
>>
>> Okay - that isn't necessarily a good thing. This is blurring the
>> division of responsibility between the launcher and the VM when it
>> comes to argument processing. I don't see why a VM option file should
>> need to handle -Dxxx arguments - that suggests a need for a more
>> general command-file approach processed by the launcher.
>
> Hmmm... The above makes it sound like you think only the launcher
> handles '-DXXX' options.

No I'm aware that some properties get passed through specifically for 
the VM to act upon (though arguably that was a bad decision back in the 
day and the launcher could have used the properties to set VM specific 
options); and also that the VM will set some properties for the launcher 
and also JDK libs to act upon. This highlights just how messy property 
processing is. By allowing properties to be set in a VMOption file you 
are increasing the level of mess - in particular if a property is 
examined by the launcher before it creates the VM, then that property 
can not be specified in the VMOptions file. So now you have something 
that will often work but for some special cases won't work.

Just makes me wonder whether this is something that should be handled by 
the VM or by the launcher. Or (dare I say it) even by a shell script. ;-)

Cheers,
David
-----

> That is not correct... Here's the current
> code:
>
> src/share/vm/runtime/arguments.cpp
>
>    2691      // -D
>    2692      } else if (match_option(option, "-D", &tail)) {
>    2693        const char* value;
>    2694        if (match_option(option, "-Djava.endorsed.dirs=", &value) &&
>    2695              *value!= '\0' && strcmp(value, "\"\"") != 0) {
>    2696          // abort if -Djava.endorsed.dirs is set
>    2697          jio_fprintf(defaultStream::output_stream(),
>    2698            "-Djava.endorsed.dirs=%s is not supported. Endorsed
> standards
> and standalone APIs\n"
>    2699            "in modular form will be supported via the concept of
> upgradea
> ble modules.\n", value);
>    2700          return JNI_EINVAL;
>    2701        }
>    2702        if (match_option(option, "-Djava.ext.dirs=", &value) &&
>    2703              *value != '\0' && strcmp(value, "\"\"") != 0) {
>    2704          // abort if -Djava.ext.dirs is set
>    2705          jio_fprintf(defaultStream::output_stream(),
>    2706            "-Djava.ext.dirs=%s is not supported.  Use -classpath
> instead.
> \n", value);
>    2707          return JNI_EINVAL;
>    2708        }
>    2709
>    2710        if (!add_property(tail)) {
>    2711          return JNI_ENOMEM;
>    2712        }
>    2713        // Out of the box management support
>    2714        if (match_option(option, "-Dcom.sun.management", &tail)) {
>    2715  #if INCLUDE_MANAGEMENT
>    2716          if (FLAG_SET_CMDLINE(bool, ManagementServer, true) !=
> Flag::SUCC
> ESS) {
>    2717            return JNI_EINVAL;
>    2718          }
>    2719  #else
>    2720          jio_fprintf(defaultStream::output_stream(),
>    2721            "-Dcom.sun.management is not supported in this VM.\n");
>    2722          return JNI_ERR;
>    2723  #endif
>    2724        }
>
>
> L2710 is where we add a property that is not already
> handled by special logic above that.
>
> $ java -showversion -XX:+PrintVMOptions -Dmy.property="This is my
> special property." DumpMyProperty
> VM option '+PrintVMOptions'
> java version "1.9.0-ea"
> Java(TM) SE Runtime Environment (build 1.9.0-ea-b69)
> Java HotSpot(TM) 64-Bit Server VM (build 1.9.0-ea-b69, mixed mode)
>
> my.property=This is my special property.
>
>
> In the above example, the "my.property" Java property is parsed
> by the above code in the VM. The launcher doesn't care about this
> particular property.
>
> Dan
>
>
>
>>
>> David
>> -----
>>
>>
>>> Thanks,
>>> Dmitry
>>>
>>> On 25.06.2015 8:01, David Holmes wrote:
>>>> On 25/06/2015 4:10 AM, Dmitry Dmitriev wrote:
>>>>> Hello Dan,
>>>>> I just want to add following: new VM Options file feature allow to
>>>>> define property inside option file.
>>>>
>>>> What do you mean by that?
>>>>
>>>> David
>>>>
>>>>> Thanks,
>>>>> Dmitry
>>>>>
>>>>> On 24.06.2015 19:19, Daniel D. Daugherty wrote:
>>>>>> On 6/23/15 10:43 PM, John Rose wrote:
>>>>>>> On Jun 23, 2015, at 7:50 PM, David Holmes <david.holmes at oracle.com>
>>>>>>> wrote:
>>>>>>>> On 24/06/2015 3:43 AM, Ron Durbin wrote:
>>>>>>>>> David,
>>>>>>>>>
>>>>>>>>> The '-XX:Flags' option was considered during the research phase
>>>>>>>>> of this project and was rejected because of its limitations. The
>>>>>>>>> '-XX:Flags' option only works for boolean flag options and it
>>>>>>>>> uses a different syntax for the options than the command line.
>>>>>>>>> For example, '-XX:+UseSerialGC' would be specified as
>>>>>>>>> '+UseSerialGC'
>>>>>>>>> in the '-XX:Flags' option file.
>>>>>>>> I'm a little surprised the Flags option wasn't extended to cover
>>>>>>>> these additional requirements. But presumably it should now be
>>>>>>>> deprecated as we (again presumably) don't want to have two
>>>>>>>> mechanisms for doing this.
>>>>>>> I thought the Flags option used to handle any command line options,
>>>>>>> and like David I'm surprised the right answer is to get rid of it
>>>>>>> and
>>>>>>> make a new option that does what I think the Flags option should be
>>>>>>> doing.
>>>>>>>
>>>>>>> — John
>>>>>>
>>>>>> Greetings,
>>>>>>
>>>>>> Ron is out of the office until next week so I'll provide a little
>>>>>> bit of background...
>>>>>>
>>>>>> Ron has a detailed write-up documenting all the research that he
>>>>>> did on the various command line option mechanisms. We had planned
>>>>>> to attach that write-up to the bug report, but we dropped the ball
>>>>>> (Sorry Mary!) After Ron returns, we'll make a final editing pass
>>>>>> on the document and attach it to the bug report.
>>>>>>
>>>>>> I fleshed out the '-XX:Flags' test cases that Ron posted earlier
>>>>>> in the thread:
>>>>>>
>>>>>> $ more flags.input.*::::::::::::::
>>>>>> flags.input.boolean
>>>>>> ::::::::::::::
>>>>>> +UseSerialGC
>>>>>> ::::::::::::::
>>>>>> flags.input.ccstr
>>>>>> ::::::::::::::
>>>>>> ErrorFile=my_error_file
>>>>>> ::::::::::::::
>>>>>> flags.input.uintx
>>>>>> ::::::::::::::
>>>>>> MinRAMFraction=8
>>>>>>
>>>>>> $ java -XX:+PrintVMOptions -XX:Flags=flags.input.boolean -version
>>>>>> VM option '+UseSerialGC'
>>>>>> VM option '+PrintVMOptions'
>>>>>> VM option 'Flags=flags.input.boolean'
>>>>>> java version "1.9.0-ea"
>>>>>> Java(TM) SE Runtime Environment (build 1.9.0-ea-b69)
>>>>>> Java HotSpot(TM) 64-Bit Server VM (build 1.9.0-ea-b69, mixed mode)
>>>>>>
>>>>>> $ java -XX:+PrintVMOptions -XX:Flags=flags.input.ccstr -version
>>>>>> VM option 'ErrorFile=my_error_file'
>>>>>> VM option '+PrintVMOptions'
>>>>>> VM option 'Flags=flags.input.ccstr'
>>>>>> java version "1.9.0-ea"
>>>>>> Java(TM) SE Runtime Environment (build 1.9.0-ea-b69)
>>>>>> Java HotSpot(TM) 64-Bit Server VM (build 1.9.0-ea-b69, mixed mode)
>>>>>>
>>>>>> $ java -XX:+PrintVMOptions -XX:Flags=flags.input.uintx -version
>>>>>> VM option 'MinRAMFraction=8'
>>>>>> VM option '+PrintVMOptions'
>>>>>> VM option 'Flags=flags.input.uintx'
>>>>>> java version "1.9.0-ea"
>>>>>> Java(TM) SE Runtime Environment (build 1.9.0-ea-b69)
>>>>>> Java HotSpot(TM) 64-Bit Server VM (build 1.9.0-ea-b69, mixed mode)
>>>>>>
>>>>>>
>>>>>> It looks like the '-XX:Flags' option does work for 'bool', 'uintx'
>>>>>> and 'ccstr' options. Clearly Ron and I didn't remember the exact
>>>>>> reason that he ruled out using the '-XX:Flags' option. This part
>>>>>> of the review thread will have to be resolved after Ron returns.
>>>>>>
>>>>>> Dan
>>>>>
>>>
>