<div dir="ltr">Hi Otávio,<div class="gmail_extra"><br></div><div class="gmail_extra">the class src/share/classes/com/sun/jmx/snmp/IPAcl/Parser.java is generated from the grammar </div><div class="gmail_extra"><br></div><div class="gmail_extra">
src/share/classes/com/sun/jmx/snmp/IPAcl/Parser.jjt<br></div><div class="gmail_extra">src/share/classes/com/sun/jmx/snmp/IPAcl/Parser.jj<br></div><div class="gmail_extra"><br></div><div class="gmail_extra">Therefore when you are going to change the Parser class, then you must change the grammar files too. In other case your changes maybe overwritten in the future, when someone decides to regenerate the Parser class.</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">The classes</div><div class="gmail_extra"><br></div><div class="gmail_extra">src/share/classes/com/sun/jmx/snmp/IPAcl/ParseException.java<br></div><div class="gmail_extra">
src/share/classes/com/sun/jmx/snmp/IPAcl/TokenMgrError.java<br></div><div class="gmail_extra"><br></div><div class="gmail_extra">are generated from the templates provided by JavaCC</div><div class="gmail_extra"><br></div>
<div class="gmail_extra"><a href="https://java.net/projects/javacc/sources/svn/show/trunk/src/main/resources/templates?rev=689">https://java.net/projects/javacc/sources/svn/show/trunk/src/main/resources/templates?rev=689</a><br>
</div><div class="gmail_extra"><br></div><div class="gmail_extra">You should consider contributing you changes to the ParseException and TokenMgrError classes to the JavaCC project as well.</div><div class="gmail_extra"><br>
</div><div class="gmail_extra"><br></div><div class="gmail_extra">In the class src/share/classes/javax/management/openmbean/CompositeType.java you have added the annotation @SuppressWarnings("StringConcatenationInsideStringBufferAppend") instead of fixing the concatenation inside the append method. Why?</div>
<div class="gmail_extra"><br></div><div class="gmail_extra"><br></div><div class="gmail_extra">And I would like to suggest to drop explicit usage of StringBuilder in some methods at all to improve code readability. Take look for example at the class src/share/classes/java/lang/management/MemoryUsage.java and its #toString()-method:</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">Before your change:</div><div class="gmail_extra"><br></div><div class="gmail_extra"><div class="gmail_extra"> 239 public String toString() {</div><div class="gmail_extra">
240 StringBuilder buf = new StringBuilder();</div><div class="gmail_extra"> 241 buf.append("init = " + init + "(" + (init >> 10) + "K) ");</div><div class="gmail_extra">
242 buf.append("used = " + used + "(" + (used >> 10) + "K) ");</div><div class="gmail_extra"> 243 buf.append("committed = " + committed + "(" +</div>
<div class="gmail_extra"> 244 (committed >> 10) + "K) " );</div><div class="gmail_extra"> 245 buf.append("max = " + max + "(" + (max >> 10) + "K)");</div>
<div class="gmail_extra"> 246 return buf.toString();</div><div class="gmail_extra"> 247 }</div><div><br></div><div>After your change:</div><div><br></div><div><div> 239 public String toString() {</div><div>
240 StringBuilder buf = new StringBuilder();</div><div> 241 buf.append("init = ").append(init).append('(').append(init >> 10).append("K) ");</div><div> 242 buf.append("used = ").append(used).append('(').append(used >> 10).append("K) ");</div>
<div> 243 buf.append("committed = ").append(committed).append('(').append(committed >> 10).append("K) ");</div><div> 244 buf.append("max = ").append(max).append('(').append(max >> 10).append("K)");</div>
<div> 245 return buf.toString();<br></div><div> 246 }</div></div><div><br></div></div><div class="gmail_extra">And if we drop explicit usage of StringBuilder and let the Java compiler do its magic, then it looks like this:</div>
<div class="gmail_extra"><br></div><div class="gmail_extra"><div class="gmail_extra"> 239 public String toString() {</div><div class="gmail_extra"> 240 return "init = " + init + "(" + (init >> 10) + "K) "</div>
<div class="gmail_extra"> 241 + "used = " + used + "(" + (used >> 10) + "K) "</div><div class="gmail_extra"> 242 + "committed = " + committed + "(" + (committed >> 10) + "K) "</div>
<div class="gmail_extra"> 243 + "max = " + max + "(" + (max >> 10) + "K)";</div><div class="gmail_extra"> 244 }<br></div><div><br></div><div>For me this code is easier to read and understand, then the code before and after your change. But this is only my opinion. Other candidates for such change may be:</div>
<div><br></div><div>src/share/classes/com/sun/crypto/provider/DHParameters.java</div><div>src/share/classes/com/sun/crypto/provider/DHPublicKey.java<br></div><div>src/share/classes/com/sun/crypto/provider/GCMParameters.java<br>
</div><div>src/share/classes/com/sun/crypto/provider/OAEPParameters.java<br></div><div>src/share/classes/com/sun/crypto/provider/RC2Parameters.java<br></div><div>src/share/classes/java/security/CodeSigner.java<br></div><div>
src/share/classes/java/security/cert/CollectionCertStoreParameters.java<br></div><div>src/share/classes/java/security/cert/LDAPCertStoreParameters.java<br></div><div>src/share/classes/java/security/cert/PKIXBuilderParameters.java<br>
</div><div>src/share/classes/java/security/cert/PKIXCertPathBuilderResult.java<br></div><div>src/share/classes/java/security/cert/PKIXCertPathValidatorResult.java<br></div><div>src/share/classes/java/security/cert/PolicyQualifierInfo.java<br>
</div><div>src/share/classes/java/util/Scanner.java<br></div><div>src/share/classes/java/util/regex/Matcher.java<br></div><div>src/share/classes/javax/management/relation/RoleInfo.java<br></div><div>src/share/classes/javax/naming/RefAddr.java<br>
</div><div>src/share/classes/sun/security/pkcs/SigningCertificateInfo.java<br></div><div>src/share/classes/sun/security/provider/certpath/CertId.java<br></div><div>src/share/classes/sun/security/provider/certpath/SunCertPathBuilderParameters.java<br>
</div><div>src/share/classes/sun/security/x509/DistributionPointName.java</div></div><div class="gmail_extra">src/share/classes/sun/security/x509/PolicyInformation.java<br></div><div class="gmail_extra">src/share/classes/sun/security/x509/X509CertImpl.java<br>
</div><div class="gmail_extra"><br></div><div class="gmail_extra"><br></div><div class="gmail_extra">In the #toString()-method of src/share/classes/javax/swing/RepaintManager.java we can avoid creation of StringBuilder if we change it from:</div>
<div class="gmail_extra"><br></div><div class="gmail_extra"><div class="gmail_extra"> 992 public synchronized String toString() {</div><div class="gmail_extra"> 993 StringBuilder sb = new StringBuilder();</div>
<div class="gmail_extra"> 994 if(dirtyComponents != null)</div><div class="gmail_extra"> 995 sb.append(dirtyComponents);</div><div class="gmail_extra"> 996 return sb.toString();</div><div class="gmail_extra">
997 }</div><div><br></div><div>to:</div><div><br></div><div><div class="gmail_extra"> 992 public synchronized String toString() {</div><div class="gmail_extra"> 993 return dirtyComponents != null ? dirtyComponents.toString : "";<br>
</div><div class="gmail_extra"> 994 }</div></div><div><br></div><div><br></div><div>In src/share/classes/sun/net/www/HeaderParser.java please change the line 223 to use StringBuilder instead of StringBuffer.</div><div>
<br></div><div>In the class src/share/classes/sun/security/ssl/ServerNameExtension.java the #toString()-method can be changed from:</div><div><br></div><div><div> 268 public String toString() {</div><div> 269 StringBuilder sb = new StringBuilder();</div>
<div> 270 for (SNIServerName sniName : sniMap.values()) {<br></div><div> 271 sb.append('[').append(sniName).append(']');</div><div> 272 }</div><div> 273 </div><div> 274 return "Extension " + type + ", server_name: " + sb;</div>
<div> 275 }</div></div><div><br></div><div>to:</div><div><br></div><div><div><div> 268 public String toString() {</div><div> 269 StringBuilder sb = new StringBuilder();</div><div> 270 sb.append("Extension ").append(type).append(", server_name: ");</div>
<div> 271 for (SNIServerName sniName : sniMap.values()) {<br></div><div> 272 sb.append('[').append(sniName).append(']');</div><div> 273 }</div><div> 274 </div><div> 275 return sb.toString();</div>
<div> 276 }</div></div></div><div><br></div><div>to avoid creation of an additional StringBuilder-object in the line 274. Similar change can be applied to the class src/share/classes/sun/security/ssl/SignatureAlgorithmsExtension.java.</div>
<div><br></div><div>Best regards,</div><div>Andrej Golovnin</div><div><br></div></div><div class="gmail_extra"><div class="gmail_quote">On Mon, Aug 11, 2014 at 5:29 AM, Otávio Gonçalves de Santana <span dir="ltr"><<a href="mailto:otaviojava@java.net" target="_blank">otaviojava@java.net</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Done.<br>
<br>
<a href="https://dl.dropboxusercontent.com/u/16109193/open_jdk/string_builder_concat_2.zip" target="_blank">https://dl.dropboxusercontent.com/u/16109193/open_jdk/string_builder_concat_2.zip</a><br>
<br>
obs: stay the 2 chars to better performance.<br>
<br>
<br>
On Sun, Aug 10, 2014 at 8:03 PM, Claes Redestad <<a href="mailto:claes.redestad@oracle.com">claes.redestad@oracle.com</a>><br>
wrote:<br>
<div class=""><div class="h5"><br>
> +1<br>
><br>
> Some suggestions (mostly nits):<br>
><br>
> - in places like src/share/classes/java/util/regex/Pattern.java you<br>
> introducesingle-char<br>
> strings which might use a char instead:<br>
><br>
> - result.append("|"+next);<br>
> + result.append('|').append(next);<br>
><br>
> - in places like src/share/classes/sun/security/provider/PolicyFile.java<br>
> you end up with a sequence of String literal appends which could be<br>
> merged into one:<br>
><br>
> - sb.append(principalInfo[i][0] + " " +<br>
> - "\"" + principalInfo[i][1] + "\"");<br>
> + sb.append(principalInfo[i][0]).append(" \"")<br>
> + .append(principalInfo[i][1]).append('"');<br>
><br>
> - in some places like src/share/classes/java/text/ChoiceFormat.java<br>
> you end up doing append(""). I guess the empty string concatenation was<br>
> used as a form<br>
> of primitive-to-string coercion and was probably always redundant<br>
> already, but care needs<br>
> to be taken that doing the append directly behave as intended.<br>
><br>
> I think it should be safe to just remove the empty string append in most<br>
> cases:<br>
><br>
> - result.append(""+choiceLimits[i]);<br>
> + result.append(choiceLimits[i]);<br>
><br>
> Thanks!<br>
><br>
> /Claes<br>
><br>
> On 2014-08-10 23:33, Otávio Gonçalves de Santana wrote:<br>
><br>
>> *Motivation:* Make another append instead of concat String inside of<br>
>> append<br>
>><br>
>> parameter in StringBuilder class. To avoid an extra StringBuilder created<br>
>> for the purpose of concatenating. So it will save memory and will faster<br>
>> than concat String.<br>
>> Doing a code to benchMark[1], the result is:<br>
>><br>
>> Benchmark Mode Samples<br>
>> Mean Mean error Units<br>
>> m.StringBuilderConcatBenchMark.stringBuilder thrpt 10<br>
>> 6317444.705 108673.584 ops/s<br>
>> m.StringBuilderConcatBenchMark.stringBuilderWithConcat thrpt 10<br>
>> 3354554.435 68353.924 ops/s<br>
>><br>
>> The webrev of all code is:<br>
>> <a href="https://dl.dropboxusercontent.com/u/16109193/open_jdk/" target="_blank">https://dl.dropboxusercontent.com/u/16109193/open_jdk/</a><br>
>> string_builder_concat.zip<br>
>> <<a href="https://dl.dropboxusercontent.com/u/16109193/open_jdk/string_" target="_blank">https://dl.dropboxusercontent.com/u/16109193/open_jdk/string_</a><br>
>> builder_concat.zip><br>
>><br>
>> [1]<br>
>><br>
>> @State(Scope.Thread)<br>
>> @OutputTimeUnit(TimeUnit.SECONDS)<br>
>> public class StringBuilderConcatBenchMark {<br>
>><br>
>><br>
>> private static final String F = "!!!!";<br>
>> private static final String E = " running in Java ";<br>
>> private static final String D = " in the code ";<br>
>> private static final String C = " to try impact ";<br>
>> private static final String B = " with some text ";<br>
>> private static final String A = "Doing a test";<br>
>><br>
>> @GenerateMicroBenchmark<br>
>> public void stringBuilder(BlackHole bh) {<br>
>> bh.consume(createBuilder(A, B, C, D, E, F));<br>
>> }<br>
>><br>
>> @GenerateMicroBenchmark<br>
>> public void stringBuilderWithConcat(BlackHole bh) {<br>
>> bh.consume(createBuilderWithConcat(A, B, C, D, E, F));<br>
>> }<br>
>><br>
>> private StringBuilder createBuilder(String... values) {<br>
>> StringBuilder text = new StringBuilder();<br>
>> text.append(values[0]).append(values[1])<br>
>> .append(values[2]).append(values[3])<br>
>> .append(values[4]).append(values[5]);<br>
>> return text;<br>
>> }<br>
>> private StringBuilder createBuilderWithConcat(String... values) {<br>
>> StringBuilder text = new StringBuilder();<br>
>> text.append(values[0] + values[1])<br>
>> .append(values[2] + values[3])<br>
>> .append(values[4]+ values[5]);<br>
>> return text;<br>
>> }<br>
>> }<br>
>><br>
>><br>
><br>
<br>
<br>
</div></div><div class=""><div class="h5">--<br>
Otávio Gonçalves de Santana<br>
<br>
blog: <a href="http://otaviosantana.blogspot.com.br/" target="_blank">http://otaviosantana.blogspot.com.br/</a><br>
twitter: <a href="http://twitter.com/otaviojava" target="_blank">http://twitter.com/otaviojava</a><br>
site: *<a href="http://about.me/otaviojava" target="_blank">http://about.me/otaviojava</a> <<a href="http://about.me/otaviojava" target="_blank">http://about.me/otaviojava</a>>*<br>
55 (11) 98255-3513<br>
</div></div></blockquote></div><br></div></div>