Please review -XshowSettings a java launcher option.

[David Schlosnagle]

Kumar,

No problem, if you have time pressures, I definitely understand. This
is welcome functionality, we have our own similar implementation
internally but it will be nice to have in OpenJDK.

I am curious though about the concern for string concatenation on
something like (INDENT + INDENT). As INDENT is defined as a static
final String INDENT = "    ", this concatenation is done at
compilation time since "the expression is a compile-time constant
expression" per JLS 15.18.1 [1] and 15.28 [2]. Any concatenations of
non-constant String would be transformed to an implicit StringBuilder.

In regards to additional I/O, doesn't OpenJDK's java.io.PrintStream
have its own BufferedWriter, so the additional writes would already be
buffered anyway, and writes wouldn't be ?

[1] http://java.sun.com/docs/books/jls/third_edition/html/expressions.html#39990
[2] http://java.sun.com/docs/books/jls/third_edition/html/expressions.html#5313

- Dave

On Thu, Nov 11, 2010 at 9:20 PM, Kumar Srinivasan
<kumar.x.srinivasan at oracle.com> wrote:
> Hi David,
>
> Good idea, but  I did not use that method  because of all the String
> concatenation  PrintStream.print has  to perform, if so then we can
> simply use a String.
>
> So my thinking was to use the StringBuilder to assemble the String and
> print it out in one shot.
>
> ostream(INDENT + INDENT);
>
> or instead:
> ostream.print(INDENT);
> ostream.print(INDENT);
> ...
> ....
>
> that would mean more io calls.
>
> I have a time pressure to get this in, I can fix/address this later as a bug
> if need be.
>
> Thanks for your review.
>
> Kumar
>
>> Kumar,
>>
>> Do you need all of the StringBuilder/StringBuffer to buffer the output
>> prior to writing to the PrintStream? Using the PrintStream directly
>> would also allow it to handle the newlines. I was envisaging something
>> like this:
>>
>> private static void printVmSettings(PrintStream ostream, long maxHeapSize,
>>         long stackSize, boolean isServer) {
>>
>>     String maxHeapSizeString = scaleValue((maxHeapSize == 0)
>>             ? Runtime.getRuntime().maxMemory()
>>             : maxHeapSize);
>>
>>     String stackSizeString = (stackSize == 0)
>>             ? "OS default"
>>             : scaleValue(stackSize);
>>
>>     ostream.println();
>>     ostream.println(VM_SETTINGS);
>>     ostream.print(INDENT + "Stack Size (-Xss): ");
>>     ostream.println(stackSizeString);
>>     ostream.print(INDENT + "Max. Heap Size (-Xmx): ");
>>     ostream.println(maxHeapSizeString);
>>     ostream.print(INDENT + "Ergonomics Class (server or client): ");
>>     ostream.println((isServer) ? "server" : "client");
>>     ostream.print(INDENT + "Using VM: ");
>>     ostream.println(System.getProperty("java.vm.name"));
>> }
>>
>> private static boolean isPath(String key) {
>>     return key.endsWith(".dirs") || key.endsWith(".path");
>> }
>>
>> private static void prettyPrintValue(PrintStream ostream, String key,
>> String value) {
>>     ostream.print(INDENT);
>>     ostream.print(key);
>>     ostream.print(" = ");
>>
>>     if (!isPath(key)) {
>>         ostream.println(value);
>>         return;
>>     }
>>
>>     // pretty print the path values as a list
>>     String pathsep = System.getProperty("path.separator");
>>     String[] values = value.split(pathsep);
>>     int len = values.length;
>>     int lastline = len - 1;
>>     for (int i = 0 ; i<  len ; i++) {
>>         if (i == 0) { // first line treated specially
>>             ostream.print(values[i]);
>>             ostream.println(pathsep);
>>         } else { // following lines prefix with indents
>>             ostream.print(INDENT + INDENT);
>>             ostream.print(values[i]);
>>             if (i != lastline) { // if not the last line suffix the
>> pathsep
>>                 ostream.print(pathsep);
>>             }
>>             ostream.println();
>>         }
>>     }
>> }
>>
>> private static void printProperties(PrintStream ostream) {
>>     Properties p = System.getProperties();
>>     List<String>  sortedPropertyKeys = new ArrayList<>();
>>     sortedPropertyKeys.addAll(p.stringPropertyNames());
>>     Collections.sort(sortedPropertyKeys);
>>     ostream.println();
>>     ostream.println(PROP_SETTINGS);
>>     for (String x : sortedPropertyKeys) {
>>         prettyPrintValue(ostream, x, p.getProperty(x));
>>     }
>> }
>>
>> private static void prettyPrintLocales(PrintStream ostream) {
>>     ostream.println();
>>     ostream.print(INDENT + "available locales = ");
>>     Locale[] locales = Locale.getAvailableLocales();
>>     int len = locales.length;
>>     int lastline = len - 1 ;
>>     for (int i = 0 ; i<  len ; i++) {
>>         ostream.print(locales[i]);
>>         if (i != lastline) {
>>             ostream.print(",");
>>         }
>>         // print columns of 8
>>         if ((i + 1) % 8 == 0) {
>>             ostream.println();
>>             ostream.print(INDENT + INDENT);
>>         } else {
>>             ostream.print(" ");
>>         }
>>     }
>> }
>>
>> private static void printLocale(PrintStream ostream) {
>>     Locale locale = Locale.getDefault();
>>     ostream.println();
>>     ostream.println(LOCALE_SETTINGS);
>>     ostream.print(INDENT+"default locale: ");
>>     ostream.print(locale.getDisplayLanguage());
>>     prettyPrintLocales(ostream);
>>     ostream.println();
>> }
>>
>> - Dave
>>
>> On Thu, Nov 11, 2010 at 7:42 PM, Kumar Srinivasan
>> <kumar.x.srinivasan at oracle.com>  wrote:
>>>
>>> Hi Mandy,
>>>
>>>> java.c
>>>> line 1031: this doesn't catch invalid option if it has the
>>>> -XshowSettings prefix e.g. -XshowSettingsJunk.
>>>
>>> Will fix it.
>>>
>>>> line 1032: Perhaps you could store the suboption ("all", "vm", etc)
>>>> rather than the entire option string.
>>>
>>> I want to keep all the parsing logic as much as possible  in java.
>>>
>>>> line 1507-1511: should these lines align with the argument "env"  to
>>>> CallStaticVoidMethod method in line 1506?
>>>
>>> Fixed.
>>>>
>>>> LauncherHelper.java
>>>> line 106-109: if optionFlag is just the suboption to -XshowSettings,
>>>> these lines can be removed.
>>>> line 150-152: Runtime.maxMemory() is not equivalent to -Xmx value.
>>>>  Perhaps -XshowSettings always prints Runtime.maxMemory() and print -Xmx if
>>>> set in the command line?
>>>
>>> That is exactly what is happening ie. if the launcher has been given the
>>> -Xmx flag
>>> then that is displayed, if not LauncherHelper will use some means to
>>> display any
>>> value it can get from Java libraries so for now we use
>>> Runtime.maxMemory().
>>>
>>>
>>>> line 176, 188, 190-191, 195, and other lines in printPrintLocales and
>>>> printLocale methods:
>>>> - the assignment to the buf and out variable to itself (returned from
>>>> StringBuffer.append() method) is not necessary.
>>>
>>> Yes fixed, I missed these.
>>>
>>>> line 174: would it be better to pass the StringBuilder to the
>>>> printPrintValue method so that it can avoid creating a new StringBuilder
>>>> instance for each property?
>>>
>>> We have to create the StringBuilder somewhere it is either in the
>>> prettyPrintValue or the calling
>>> method, I will convert it to StringBuilder and pass it into
>>> prettyPrintValue.
>>>
>>>> line 213-214: same comment as the above.   And should it use
>>>> StringBuilder instead?
>>>
>>> Yes.
>>>
>>> Thanks for the review.!
>>>
>>> Kumar
>>>
>>>> Mandy
>>>>>
>>>>> This will print all the known settings/properties/locales
>>>>> supported and known  to Java, this has  been a long standing request.
>>>>>
>>>>> A sample output attached below.
>>>>>
>>>>> Note: the -X option specifically is being used so we can evolve this
>>>>> option
>>>>> and add more useful information, in future versions of java.
>>>>>
>>>>> Thanks
>>>>> Kumar
>>>>>
>>>>>
>>>>>
>
>