David Schlosnagle wrote:
On Sun, Apr 17, 2011 at 1:27 PM, Ulf Zibis <ulf.zi...@gmx.de> wrote:
Am 16.04.2011 16:45, schrieb David Schlosnagle:
One minor nit in ProcessEnvironment.java
336 private static void addToEnv(StringBuilder sb, String name,
String val) {
337 sb.append(name+"="+val+'\u0000');
338 }
I think it should be as follows to avoid implicitly constructing
another StringBuilder and the extra copying overhead;
336 private static void addToEnv(StringBuilder sb, String name,
String val) {
337 sb.append(name).append('=').append(val).append('\u0000');
338 }
Because this suggestion was from me, I must admit, that I didn't prove, if
javac actually uses the existing StringBuilder sb, or forces another
StringBuilder to instantiate. I just assumed, javac would use the existing
one. So to be done:
- examine the byte code
- if not yet optimized: then new RFE for javac
Ulf,
I'm not the right person to comment on whether javac could safely
optimize away the StringBuilder creation here per JLS 15.18.1 [1] [2],
but in this situation I'd assume that the expression
name+"="+val+'\u0000' must return a "result is a reference to a String
object (newly created, unless the expression is a compile-time
constant expression (ยง15.28))that is the concatenation of the two
operand strings" in which case I don't think javac can optimize it
away.
Some IDEs such as IntelliJ offer code inspections to find and fix such
items, see "String concatenation inside 'StringBuffer.append()'" [3]:
This inspection reports any instances of String concatenation used as the
argument to StringBuffer.append(), StringBuilder.append() or
Appendable.append(). Such calls may profitably be turned into chained
append calls on the existing StringBuffer/Builder/Appendable, saving the
cost of an extra StringBuffer/Builder allocation.
This inspection ignores compile time evaluated String concatenations, which
when converted to chained append calls would only worsen performance.
Here's a quick `javap -c -private` dump of these two different methods
in a test class, showing that the changes I'm recommending simplify
the bytecode and avoid the extra StringBuilder construction and
copying of underlying characters:
Thanks for confirming that. I'll switch it back to the original
chained .append().
- Michael.
