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.sriniva...@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.sriniva...@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 >>>>> >>>>> >>>>> > >
