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
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 ulf.zi...@cosoco.de wrote:
Hi Otávio,
I now think you could replace
if (!expected.isEmpty())
with
assert
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 otavioj...@java.net
wrote:
Ok, you're right.
On Wed, Nov 12, 2014 at 11:19 PM, Wang Weijun weijun.w...@oracle.com
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.
Thank you Ulf.
http://cr.openjdk.java.net/~weijun/8055723/webrev.01/
On Sat, Nov 8, 2014 at 3:46 PM, Ulf Zibis ulf.zi...@cosoco.de 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
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, SetString expected, Token found) {
66 StringBuilder msg = new StringBuilder(A + B * expected.size());
67
68
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:
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 ulf.zi...@cosoco.de wrote:
Am 21.10.2014 um 01:02 schrieb Otávio Gonçalves de Santana:
BUGURL:
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:
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
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
---
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
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
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 jonathan.gibb...@oracle.com
wrote:
It would be
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
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 jonathan.gibb...@oracle.com
wrote:
Yes, but is this really a big enough performance and footprint pain
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
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
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
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
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
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 error
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 and
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
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
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
Could you please explain what you mean by javac optimization fails here?
-Pavel
On 27 Aug 2014, at 10:41, Ulf Zibis ulf.zi...@cosoco.de wrote:
4.) Now we see, that javac optimization fails again if StringBuilder,
concatenation, toString(), append(String), append(Collection) etc. and
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.
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 ivan.gerasi...@oracle.com wrote:
Hi Max!
The core part is updated again at
http://cr.openjdk.java.net/~weijun/8055723/core/webrev.03/
Can you please
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:
It's exactly the way it's been working since 1.6 I believe.
source
public class Optimization {
public String concat(String... strings) {
return #: + strings[0] + strings[2] + strings[3] + ...;
}
}
compiled into
public class Optimization {
public Optimization();
Code:
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])
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:
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
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
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
+++
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
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
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/
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
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
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 only
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
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
On Aug 21, 2014, at 21:18, Andrej Golovnin andrej.golov...@gmail.com 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
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
Brian,
Yes, this works fine in cases when 'whatever' is a j.u.Collection? extends
CharSequence (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? extends CharSequence
*
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,
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
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 -
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
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
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 andrej.golov...@gmail.com wrote:
As
fyi,
There's a Perl script normalizer.pl that detects/fixes most of the
simple tab/white space issues.
The script is in the repo/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
'\' 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('\');
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
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? I've
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
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 andrej.golov...@gmail.com
wrote:
Hi Otávio,
About the template in Parser.jjt,
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 sb
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
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
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
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:
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
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 pavel.ra...@oracle.com wrote:
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
+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
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] + +
-\ +
69 matches
Mail list logo
