Ok, you're right.
On Wed, Nov 12, 2014 at 11:19 PM, Wang Weijun
wrote:
> I hope we can restrict the code change to what the bug description is
> about. IMHO this bug should only include cleanup and introduce no obvious
> behavior change.
>
> Any other fix can go to another bug.
>
> --Max
>
> > O
I hope we can restrict the code change to what the bug description is about.
IMHO this bug should only include cleanup and introduce no obvious behavior
change.
Any other fix can go to another bug.
--Max
> On Nov 13, 2014, at 08:57, Otávio Gonçalves de Santana
> wrote:
>
> But this class is
But this class is an Exception, doesn't make sense an exception get another
Exception.
IMHO: I prefer this way
On Wed, Nov 12, 2014 at 8:36 AM, Ulf Zibis wrote:
> Hi Otávio,
> I now think you could replace
> if (!expected.isEmpty())
> with
> assert !expected.isEmpty();
>
> If e
Hi Otávio,
I now think you could replace
if (!expected.isEmpty())
with
assert !expected.isEmpty();
If expected ever would be empty, the only thing which happens is, that a "'" is missing in a message
which anyway doesn't make sense without arguments.
-Ulf
Am 12.11.2014 um
Thank you Ulf.
http://cr.openjdk.java.net/~weijun/8055723/webrev.01/
On Sat, Nov 8, 2014 at 3:46 PM, Ulf Zibis wrote:
> Hi Otávio,
> in sun/tools/jstat/SyntaxException.java I see a possible enhencement
> (maybe applies to other places too):
>
> 65 public SyntaxException(int lineno, Set exp
Hi Otávio,
in sun/tools/jstat/SyntaxException.java I see a possible enhencement (maybe applies to other places
too):
65 public SyntaxException(int lineno, Set expected, Token found) {
66 StringBuilder msg = new StringBuilder(A + B * expected.size());
67
68 msg.append
Could another reviewer look these codes, please.
http://cr.openjdk.java.net/~weijun/8055723/webrev.00/
On Fri, Oct 24, 2014 at 3:25 AM, Otávio Gonçalves de Santana <
otavioj...@java.net> wrote:
> Thank you Ulf.
> I removed the fix in toString method and in debug classes:
> http://cr.openjdk.java.
Thank you Ulf.
I removed the fix in toString method and in debug classes:
http://cr.openjdk.java.net/~weijun/8055723/webrev.00/
On Mon, Oct 20, 2014 at 10:26 PM, Ulf Zibis wrote:
>
> Am 21.10.2014 um 01:02 schrieb Otávio Gonçalves de Santana:
>
>> BUGURL: https://bugs.openjdk.java.net/browse/JDK
Am 21.10.2014 um 01:02 schrieb Otávio Gonçalves de Santana:
BUGURL: https://bugs.openjdk.java.net/browse/JDK-8055723
WEBREV: http://cr.openjdk.java.net/~weijun/8055723/client/webrev.02/
WEBREV: http://cr.openjdk.java.net/~weijun/8055723/core/webrev.03/
I did not look through all sources.
In
BUGURL: https://bugs.openjdk.java.net/browse/JDK-8055723
WEBREV: http://cr.openjdk.java.net/~weijun/8055723/client/webrev.02/
WEBREV: http://cr.openjdk.java.net/~weijun/8055723/core/webrev.03/
--
Otávio Gonçalves de Santana
blog: http://otaviosantana.blogspot.com.br/
twitter: http://twitt
On 08.09.2014 22:53, Jonathan Gibbons wrote:
Sergey,
Many of the suggestions in the webrev change the semantics of the
code, and so would not be appropriate for javac to perform automagically.
It is changed but I guess it is safe otherwise this ccan be a problem in
the fix, but the fix still u
Am 08.09.2014 um 20:53 schrieb Jonathan Gibbons:
For example, in the first few lines of the patch, I found this:
Do you see any semantics change here?
diff -r dde9f5cfde5f
src/share/classes/sun/tools/jconsole/inspector/XArrayDataViewer.java
--- a/src/share/classes/sun/tools/jconsole/inspector
Much thanks for all your thoughts.
-Ulf
Am 08.09.2014 um 20:53 schrieb Jonathan Gibbons:
Sergey,
Many of the suggestions in the webrev change the semantics of the code, and so would not be
appropriate for javac to perform automagically.
For example, in the first few lines of the patch, I fo
FWIW, Google has gotten noticeable performance benefits from a change to
javac's compilation of normal + concatenation, to just presize the
StringBuilder. I haven't had the bandwidth to forward-port that change
yet, unfortunately, but that's a semantics-preserving change that seemed to
us to be wi
Sergey,
Many of the suggestions in the webrev change the semantics of the code,
and so would not be appropriate for javac to perform automagically.
For example, in the first few lines of the patch, I found this:
diff -r dde9f5cfde5f
src/share/classes/sun/tools/jconsole/inspector/XArrayDataVi
On 08.09.2014 21:50, Jonathan Gibbons wrote:
Yes, but is this really a big enough performance and footprint pain
point to be worth addressing in javac itself?
We're now talking about some specific code construction like
new StringBuilder().append(a + b + c)
Any other case where the string
I have no idea. I was just sticking in my two cents’ worth on what is
possible. What is desirable and worthwhile is a much bigger question.
On Sep 8, 2014, at 1:50 PM, Jonathan Gibbons
wrote:
> Yes, but is this really a big enough performance and footprint pain point to
> be worth addressin
Yes, but is this really a big enough performance and footprint pain
point to be worth addressing in javac itself?
We're now talking about some specific code construction like
new StringBuilder().append(a + b + c)
Any other case where the string builder can be observed externally
cannot be
Good point, but counterpoint: it might be acceptable to have modified the
string buffer in situations where throwing an exception would always cause the
string buffer to become inaccessible.
—Guy
On Sep 8, 2014, at 1:30 PM, Jonathan Gibbons
wrote:
> It would be inappropriate/incorrect to app
It would be inappropriate/incorrect to apply the optimization as described.
The JLS requires that the argument to a method call should be computed
before invoking the method.
Consider the case when one of the expressions in the series of string
concatenations throws an exception. It would be
There was related discussions on the past
http://mail.openjdk.java.net/pipermail/compiler-dev/2014-February/008491.html
And related issues:
https://bugs.openjdk.java.net/browse/JDK-4947460
https://bugs.openjdk.java.net/browse/JDK-4059189
https://bugs.openjdk.java.net/browse/JDK-6709423
On 30.08.2
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
It may be worth to compare the performance of alternative
implementations with -XX:+OptimizeStringConcat option turned on.
Sincerely yours,
Ivan
On 30.08.2014 0:53, Ulf Zibis wrote:
Hi compiler people,
is there some chance that javac could be enhanced to optimize better
as discussed in this
It's probably best to teach the JIT to handle more of these cases. The
reason is I don't think the longer compile time will be worth it
considering that, on a large codebase, there will inevitably be lots of
places where these patterns appear but have no dominance at runtime (either
because they'r
Hi compiler people,
is there some chance that javac could be enhanced to optimize better as discussed in this thread?
Than refactoring of up to now better readable code to ugly StringBuilder@append() code would be
superfluous.
I really like the String concatenation syntax, but unfortunately it
So it's not that the optimization fails but there is no optimization on them
yet.
I do see the .append("x") case will be easy to deal with, but it looks like
historically javac has not been a place to do many optimizations. It mostly
converts the java source to byte codes in a 1-to-1 mapping an
I mean:
It does not output byte code that only uses a single char array to compose the entire String in
question.
With "optimization fails", I also mean, there is used an additional "StringComposer" e.g. another
StringBuilder or a StringJoiner in addition to the 1st StringBuilder.
-Ulf
Am 27.
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
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
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
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/
Could you please explain what you mean by "javac optimization fails" here?
-Pavel
On 27 Aug 2014, at 10:41, Ulf Zibis wrote:
> 4.) Now we see, that javac optimization fails again if StringBuilder,
> concatenation, toString(), append(String), append(Collection) etc. and
> StringJoiner use is m
Hi all,
a critical question!
1.) Why we have String concatenation in Java? ... I would answer: for
_readability_ purpose.
2.) In the early javac times, we were asked to refactor to StringBuffer for
performance critical code.
3.) Since 1.6 ? javac is capable to replace multiple concatenation
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
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
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\([^\)]*\
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:
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
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
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
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
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
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
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
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/
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.
I had a random look at the Webrev for TreeModelEvent.java and saw the
following new code:
sb.append(Integer.toString(childIndices[counter]))
Wouldn't the following be slightly more efficient?
sb.append(childIndices[counter])
since Integer.toString(int) creates a temporary char[] array l
>
>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
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,
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
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,
Brian,
Yes, this works fine in cases when 'whatever' is a j.u.Collection (though some may argue that it's an overkill already). However we
need even more thinking and transformations in case when 'whatever' is one of
these:
* j.u.Iterator
* j.l.Iterable
* j.u.Enumeration
* CharSeque
What you should have written was:
String s = whatever.stream().collect(joining(", "));
On 8/11/2014 1:31 PM, Pavel Rappo wrote:
Sorry, I should've written this:
Iterable whatever = ...
StringJoiner joiner = new StringJoiner(", ");
whatever.forEach(w -> joiner.ad
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
fyi,
There's a Perl script normalizer.pl that detects/fixes most of the
simple tab/white space issues.
The script is in the /make/scripts/normalizer.pl
Roger
On 8/12/2014 3:48 AM, Andrej Golovnin wrote:
Hi Otávio,
I think you should fix the indentation in a lot of classes. You use the
tab-c
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
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
Sorry, I should've written this:
Iterable whatever = ...
StringJoiner joiner = new StringJoiner(", ");
whatever.forEach(w -> joiner.add(w.toString()));
String s = joiner.toString();
-Pavel
On 11 Aug 2014, at 17:17, Pavel Rappo wrote:
> Iterable wha
Ulf,
My point was simply that none of these classes provide methods that accept
java.lang.Object. IMO, the scenario when we join elements into a String by
calling toString on each element prior to append it -- is the most common
scenario (maybe except for when element is a String itself). That
Am 11.08.2014 um 16:33 schrieb Pavel Rappo:
Unfortunately, neither java.util.StringJoiner nor String.join have perfect (but
who has?) APIs. So it's up to us to figure out the best way of joining elements.
Maybe remember my thoughts from here:
http://mail.openjdk.java.net/pipermail/core-libs-d
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
Am 11.08.2014 um 15:12 schrieb Andrej Golovnin:
In the most classes I mentioned in my previous mail only the
#toString()-methods would be affected by the proposal. And in the most
cases, maybe in all cases, the #toString()-methods in this classes exists
only to provide nice output.
So why not
Hi Otávio!
A few days ago I've posted another cleanup request, which included
modifications to src/share/classes/sun/net/www/MimeEntry.java
http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-August/028131.html
As our changes overlap, one should be reverted back.
I believe using StringJo
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
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
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
> 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
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
'\"' 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
Am 11.08.2014 um 01:03 schrieb Claes Redestad:
- in places like src/share/classes/sun/security/provider/PolicyFile.java
you end up with a sequence of String literal appends which could be merged
into one:
-sb.append(principalInfo[i][0] + " " +
-"\"" + prin
+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
Hi Otávio,
this is a great proposal.
Little nit:
In cases where only 1 character is to append, I guess append(char) would be faster and at least will
save footprint in contrast to append(String).
-Ulf
Am 10.08.2014 um 23:33 schrieb Otávio Gonçalves de Santana:
*Motivation:* Make another ap
72 matches
Mail list logo
