admin@glassfish.java.net

Re: Extreme Code Reviewing

From: Byron Nevins <Byron.Nevins_at_Sun.COM>
Date: Thu, 15 Mar 2007 11:55:09 -0700

No Nandini took it. thanks nyway.
Lloyd L Chambers wrote:
> This was yesterday...still need review?
>
> On Mar 14, 2007, at 1:13 PM, Byron Nevins wrote:
>
>> Can someone review this rapidly (< 1 hour)?
>> It's really simple and I have several more changes to make after the
>> review to this same file. I.e. I really need it reviewed quckly.
>>
>> What I did:
>> Context: Launching servers
>>
>> 1)There is a jvm-option order dependency in the JVM iself --
>> -XX:+UnlockDiagnosticVMOptions must appear before certain other -XX
>> options
>> I now do post-processing. If that jvm-option exists I move it to the
>> top of the list. (Note that we are going to start using these
>> options soon for thread dumps).
>> The penalty for not having them in the right order: Death! THe JVM
>> refuses to run.
>>
>> 2)-D options are NOT order-dependent. DAS and instances have a huge
>> hard-to-read number of -D options. As part of the post-processing,
>> I sort the -D's just before creating the server JVM
>>
>> Index:
>> src/java/com/sun/enterprise/admin/servermgmt/launch/ASLauncher.java
>> ===================================================================
>> RCS file:
>> /cvs/glassfish/admin/servermgmt/src/java/com/sun/enterprise/admin/servermgmt/launch/ASLauncher.java,v
>>
>> retrieving revision 1.10
>> diff -b -u -r1.10 ASLauncher.java
>> ---
>> src/java/com/sun/enterprise/admin/servermgmt/launch/ASLauncher.java
>> 14 Mar 2007 19:48:29 -0000 1.10
>> +++
>> src/java/com/sun/enterprise/admin/servermgmt/launch/ASLauncher.java
>> 14 Mar 2007 20:00:24 -0000
>> @@ -43,6 +43,7 @@
>> import java.io.PushbackInputStream;
>> import java.lang.InterruptedException;
>> import java.util.Arrays;
>> +import java.util.Collections;
>> import java.util.Enumeration;
>> import java.util.Iterator;
>> import java.util.StringTokenizer;
>> @@ -657,6 +658,8 @@
>> command=buildExternalCommand(action);
>> }
>> + command.reorder();
>> + // log final command
>> String finalCommand=command.toStringWithLines();
>> getLogger().log(Level.INFO, finalCommand);
>> @@ -2488,6 +2491,46 @@
>> cmd.append("\n" + ret[ii]);
>> }
>> return cmd.toString();
>> + }
>> +
>> + ////////////////////////////////////////////////////////////
>> + + void reorder()
>> + {
>> + // WBN March 2007
>> + reorderJVMOptions();
>> + reorderSystemProps();
>> + }
>> + + private void reorderJVMOptions()
>> + {
>> + /* WBN MARCH 2007
>> + * There *is* an order dependency in JVM options. I.e.
>> I know of one
>> + * such case. The Unlock... option must appear before
>> the other two:
>> + *
>> <jvm-options>-XX:+UnlockDiagnosticVMOptions</jvm-options>
>> + * <jvm-options>-XX:+LogVMOutput</jvm-options>
>> + *
>> <jvm-options>-XX:LogFile=${com.sun.aas.instanceRoot}/logs/jvm.log</jvm-options>
>> + * -- So reorder the list of jvm options (possibly)...
>> + */
>> + // WBN March 2007
>> + String opt = "-XX:+UnlockDiagnosticVMOptions";
>> + int index = _jvmOptions.indexOf(opt);
>> + + if(index > 0) // yes ">" not ">=" -- if
>> index is zero, we are done!
>> + {
>> + // remove it...
>> + _jvmOptions.remove(index);
>> + // add it back to the beginning...
>> + _jvmOptions.add(0, opt);
>> + }
>> + }
>> +
>> + private void reorderSystemProps()
>> + {
>> + // WBN March 2007
>> + // There are a LOT of these. Why not write them out in
>> alphabetical
>> + // order?
>> + Collections.sort(_systemVariables);
>> }
>> }
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
>> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>