Re: Review request: JDK-8055723 Replace concat String to append in StringBuilder parameters

Yes, He did. On Thu, Dec 4, 2014 at 1:20 AM, Wang Weijun wrote: > > > > On Dec 3, 2014, at 22:47, Sergey Bylokhov > wrote: > > > > Hello. > > On 03.12.2014 17:40, Wang Weijun wrote: > >> Thanks for reviewing the codes. I am the sponsor for this bug. Do you > think it's OK for me to push the cha

Re: Review request: JDK-8055723 Replace concat String to append in StringBuilder parameters

> > On Dec 3, 2014, at 22:47, Sergey Bylokhov wrote: > > Hello. > On 03.12.2014 17:40, Wang Weijun wrote: >> Thanks for reviewing the codes. I am the sponsor for this bug. Do you think >> it's OK for me to push the changes for java.desktop to jdk9/dev/jdk? You didn't answer the question above,

Re: Review request: JDK-8055723 Replace concat String to append in StringBuilder parameters

Hi Sergey Thanks for reviewing the codes. I am the sponsor for this bug. Do you think it's OK for me to push the changes for java.desktop to jdk9/dev/jdk? Otherwise, can I push to jdk9/client/jdk myself or do I need to find someone in your group doing it? I am on the corelibs side and has only

Re: Review request: JDK-8055723 Replace concat String to append in StringBuilder parameters

Hello. On 03.12.2014 17:40, Wang Weijun wrote: Thanks for reviewing the codes. I am the sponsor for this bug. Do you think it's OK for me to push the changes for java.desktop to jdk9/dev/jdk? Otherwise, can I push to jdk9/client/jdk myself Yes, you can push it yourself to the jdk9/client/jdk

Re: Review request: JDK-8055723 Replace concat String to append in StringBuilder parameters

Hello, The changes in java.desktop looks good. On 18.11.2014 11:58, Otávio Gonçalves de Santana wrote: BUGURL: https://bugs.openjdk.java.net/browse/JDK-8055723 WEBREV: http://cr.openjdk.java.net/~weijun/8055723/webrev.01/ -- Otávio Go

Review request: JDK-8055723 Replace concat String to append in StringBuilder parameters

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8055723 WEBREV: http://cr.openjdk.java.net/~weijun/8055723/webrev.01/ -- Otávio Gonçalves de Santana blog: http://otaviosantana.blogspot.com.br/ twitter: http://twitter.com/otaviojava site: *http://about.me/otaviojava

Re: Replace concat String to append in StringBuilder parameters

I believe yes. Using the -XX:+OptimizeStringConcat: java -jar -XX:+OptimizeStringConcat target/microbenchmarks.jar ".*StringBuilderConcatBenchMark.*" -wi 10 -i 10 -f 1 The same thing happened: Benchmark Mode Samples Mean Mean errorU

Re: Replace concat String to append in StringBuilder parameters

On 28.08.2014 3:15, Wang Weijun wrote: OK, I'll remember that. Thanks. So you will include the StringBuilder changes into your fix? No. I reimplemented MimeEntry.toProperty() with StringJoiner: http://hg.openjdk.java.net/jdk9/dev/jdk/diff/8be081fb8db1/src/java.base/share/classes/sun/net/ww

Re: Replace concat String to append in StringBuilder parameters

Hi Max! The core part is updated again at http://cr.openjdk.java.net/~weijun/8055723/core/webrev.03/ Can you please revert changes to src/java.base/share/classes/sun/net/www/MimeEntry.java, as they're conflicting with the fix for JDK-8054714? Apologizing for adding you more work. Sinc

Re: Replace concat String to append in StringBuilder parameters

OK, I'll remember that. So you will include the StringBuilder changes into your fix? --Max On Aug 28, 2014, at 2:10, Ivan Gerasimov wrote: > Hi Max! > >> The core part is updated again at >> >> http://cr.openjdk.java.net/~weijun/8055723/core/webrev.03/ > > Can you please revert changes to

Re: Replace concat String to append in StringBuilder parameters

On Aug 27, 2014, at 10:07, Wang Weijun wrote: > Webrev updated again, this time include more changes. > > http://cr.openjdk.java.net/~weijun/8055723/client/webrev.02/ > http://cr.openjdk.java.net/~weijun/8055723/core/webrev.02/ The core part is updated again at http://cr.openjdk.java.net/

Re: Replace concat String to append in StringBuilder parameters

I found an error: diff --git a/src/jdk.dev/share/classes/com/sun/tools/hat/internal/util/Misc.java b/src/jdk.dev/share/classes/com/sun/tools/hat/internal/util/Misc.java --- a/src/jdk.dev/share/classes/com/sun/tools/hat/internal/util/Misc.java +++ b/src/jdk.dev/share/classes/com/sun/tools/hat/int

Re: Replace concat String to append in StringBuilder parameters

Webrev updated again, this time include more changes. http://cr.openjdk.java.net/~weijun/8055723/client/webrev.02/ http://cr.openjdk.java.net/~weijun/8055723/core/webrev.02/ The change to a demo file is removed because that file itself is already removed. *Otávio*: I believe Andrej's follow

Re: Replace concat String to append in StringBuilder parameters

I see no problem from the core part of the webrev. However, I am not sure how you find all the occurrences of "+" in StringBuilder, but I just run the following command in jdk/src find . -type f -name *.java -print | xargs grep -n StringBuilder | perl -ne 'print if /new StringBuilder\([^\)]*\

Re: Replace concat String to append in StringBuilder parameters

Hi all, src/java.base/share/classes/com/sun/crypto/provider/DHPublicKey.java: 269 .append(LINE_SEP + "g:" + LINE_SEP) should be changed to: 269 .append(LINE_SEP).append( "g:").append(LINE_SEP) src/java.base/share/classes/sun/security/x509/PolicyInformation.java:

Re: Replace concat String to append in StringBuilder parameters

Pavel, thanks for the checking and confirm. Look back to the benchmark code: private StringBuilder createBuilder(String... values) { StringBuilder text = new StringBuilder(); text.append(values[0]).append(values[1]) .append(values[2]).append(values[3]) .append(values[4]).append(va

Re: Replace concat String to append in StringBuilder parameters

It's exactly the way it's been working since 1.6 I believe. public class Optimization { public String concat(String... strings) { return "#: " + strings[0] + strings[2] + strings[3] + "..."; } } public class Optimization { public Optimization(); Code: 0: aload_0

Re: Replace concat String to append in StringBuilder parameters

I was wondering, is it nice to address it in Java compiler to use string builder for the string "+" operator? Xuelei On 8/26/2014 11:28 AM, Wang Weijun wrote: > New webrevs available at > > http://cr.openjdk.java.net/~weijun/8055723/client/webrev.01/ > http://cr.openjdk.java.net/~weijun/805

Re: Replace concat String to append in StringBuilder parameters

New webrevs available at http://cr.openjdk.java.net/~weijun/8055723/client/webrev.01/ http://cr.openjdk.java.net/~weijun/8055723/core/webrev.01/ There are only 2 now. Everything non-client is in core. Everyone, please do code review quickly because the patch touches too many files and any

Re: Replace concat String to append in StringBuilder parameters

On 25/08/2014 03:03, Wang Weijun wrote: New webrevs updated http://cr.openjdk.java.net/~weijun/8055723/core/webrev.00/ Includes modules java.base and security-related modules and the jarsigner tool http://cr.openjdk.java.net/~weijun/8055723/client/webrev.00 Includes the java.desktop mo

Re: Replace concat String to append in StringBuilder parameters

New webrevs updated http://cr.openjdk.java.net/~weijun/8055723/core/webrev.00/ Includes modules java.base and security-related modules and the jarsigner tool http://cr.openjdk.java.net/~weijun/8055723/client/webrev.00 Includes the java.desktop module http://cr.openjdk.java.net/~weijun/8055

Re: Replace concat String to append in StringBuilder parameters

I also see a lot of .toString() and String.valueOf() calls. $ cat string_concat_updated.patch | perl -ne 'print if /^\+ .*append.*(String\.valueOf|\.toString\(\))/' | wc 62 2104626 Wrapped lines not indented correctly in src/java.xml.crypto/share/classes/com/sun/org/apache/xml/int

Re: Replace concat String to append in StringBuilder parameters

On Aug 21, 2014, at 21:18, Andrej Golovnin wrote: >https://bugs.openjdk.java.net/browse/JDK-8038277 > > This is not the right bug report. The subject of this bug report is "Improve > the bootstrap performance of carets keystore". Oh, my mistake, it should be https://bugs.openjdk.java.net/

Re: Replace concat String to append in StringBuilder parameters

Hi Martin, you are right. And in the line 297: 297 sb.append(getClass().getName()).append(' ').append(Integer.toString(hashCode())); Integer.toString() can be removed too. Best regards, Andrej Golovnin On Thu, Aug 21, 2014 at 3:26 PM, Martin Desruisseaux < martin.desruisse...@geomatys.

Re: Replace concat String to append in StringBuilder parameters

> >https://bugs.openjdk.java.net/browse/JDK-8038277 This is not the right bug report. The subject of this bug report is "Improve the bootstrap performance of carets keystore". > >http://cr.openjdk.java.net/~weijun/8038277/client/webrev.00 TABs are still used for indentation. I looked

Re: Replace concat String to append in StringBuilder parameters

I filed a bug at https://bugs.openjdk.java.net/browse/JDK-8038277 Webrev in 3 parts at http://cr.openjdk.java.net/~weijun/8038277/client/webrev.00 http://cr.openjdk.java.net/~weijun/8038277/core/webrev.00/ http://cr.openjdk.java.net/~weijun/8038277/extra/webrev.00/ --Max On Aug 21,

Re: Replace concat String to append in StringBuilder parameters

Hi Otávio I see TABs in the first page of sun_security.diff, too long line in javax_security.diff. Also, it's unfortunate that you will need to rename the file names to the new style with modules. See http://cr.openjdk.java.net/~chegar/docs/portingScript.html for how to do this. I can create

Re: Replace concat String to append in StringBuilder parameters

Hi Otávio, The new alignment in DataLine.java/JColorChooser.java looks strange. Wrong change in BasicTableUI.java: -plainStr.deleteCharAt(plainStr.length() - 1).append("\n"); +plainStr.deleteCharAt(plainStr.length() - 1).append('\t'); On 13.08.2014 3:01,

Re: Replace concat String to append in StringBuilder parameters

Could anyone help me as sponsor, please? On Tue, Aug 12, 2014 at 8:01 PM, Otávio Gonçalves de Santana < otavioj...@java.net> wrote: > Thank you Roger. > Done > > https://dl.dropboxusercontent.com/u/16109193/open_jdk/string_builder_concat_6.zip > > > On Tue, Aug 12, 2014 at 10:15 AM, roger riggs

Re: Replace concat String to append in StringBuilder parameters

No TAB, no \r, and no trailing space are hard requirements enforced by jcheck. Otherwise it's only styles, including 4-space-indentation. "{" at the end of a line, 8-space wrap indentation... --Max (an Oracle dev) On Aug 12, 2014, at 15:48, Andrej Golovnin wrote: > As far as I know we should

Re: Replace concat String to append in StringBuilder parameters

> 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? +1 Moreover, I wonder where this value comes from

Re: Replace concat String to append in StringBuilder parameters

Otavio, Just skimmed through your changes. It looks good. But there are some things we can make a little bit better though. IMO, it's not always a performance that matters (looking around to see if Alexey Shipilev is somewhere near) but readability. It's good to estimate performance requirement

Re: Replace concat String to append in StringBuilder parameters

Hi Otávio, I think you should fix the indentation in a lot of classes. You use the tab-character for the indentation. As far as I know we should use the space character for the indentation in the JDK sources (Oracle devs feel free to correct me if I'm wrong. And it would be really nice if the styl

Re: Replace concat String to append in StringBuilder parameters

Hi, About readable of code I just renamed this class to sb instead of buf, > strbuf, etc. > I doubt that renaming variables does really improve the code readability. And you changed the indentation (take look at other classes too): 239 public String toString() { 240 StringBuilder s

Re: Replace concat String to append in StringBuilder parameters

Hi Otávio, please ignore the previous diff. I'm sorry, there was a small mistake. I have attached the corrected version. Best regards, Andrej Golovnin On Mon, Aug 11, 2014 at 1:55 PM, Andrej Golovnin wrote: > Hi Otávio, > > About the template in Parser.jjt, TokenMgrError.java, etc. I don't k

Re: Replace concat String to append in StringBuilder parameters

Hi Otávio, About the template in Parser.jjt, TokenMgrError.java, etc. I don't know how > can do that. Can anyone help me? > See attached diff for the changes in Parser.jjt and Parser.jj. For the TokenMgrError and ParserException you can just subscribe here: https://java.net/projects/javacc/lists

Re: Replace concat String to append in StringBuilder parameters

Hi Otávio, the class src/share/classes/com/sun/jmx/snmp/IPAcl/Parser.java is generated from the grammar src/share/classes/com/sun/jmx/snmp/IPAcl/Parser.jjt src/share/classes/com/sun/jmx/snmp/IPAcl/Parser.jj Therefore when you are going to change the Parser class, then you must change the grammar

Re: Replace concat String to append in StringBuilder parameters

+1 Some suggestions (mostly nits): - in places like src/share/classes/java/util/regex/Pattern.java you introducesingle-char strings which might use a char instead: -result.append("|"+next); +result.append('|').append(next); - in places like src/share/classes

Re: Replace concat String to append in StringBuilder parameters

'\"' can be written as '"': com_sun.diff:209:+sb.append(' ').append(nodeName).append("=\"").append(att.getNodeValue()).append('\"'); java_lang.diff:31:+ sb.append('\"').append(getThreadName()).append('\"') java_security.diff:78:+.append('\"'); s