API change proposal: String concatenation boost

[Jesús Viñuales]

Hi Rémi. Thank you for your feedback! My comments...


> You right about the fact the major use case is to use lot of append
> in a loop but i not agree about the fact that this append is always
> a append(String) or a append(Object).
> append(char), append(int) are very popular too and
> doesn't work well with your implementation.
> So i don't think if it worth a new class.

Touché. I know my implementation performs well when appending CharSequences,
char arrays, Objects and Booleans, but pays an overload for appending
primate types (it creates a char[] instance and a String instance). But:

	1.- If you look at the microbenchmark, there is one case emulating
some log entry with 7 String, 1 Date, 1 char, 1 double, 1 boolean. 25% of
primitive variables and beats the Builder version with 11-50% of gain.

	2.- I have just tested only appending 6 char and only appending 6
ints, in that case my results are -80% an d-40% worst, respectively. But
these are irreal cases, I believe.

	3.- I attach a new version of StringAppender with a better
implementation of append(int), append(long) append(char) which has shutcuts
for typical value.

	4.- We should calculate the percentage of those appends of primitive
values (distinct of the shutcuts defined in the new version of the code) to
be able to evaluate the benefits of my approach. I guess they are no more
than 5% of cases, so the proposal would still be valid...

	5.- Whenever appending a constant (char, int...), change the code /
evangelize to change it for its String equivalent:
		append('.') => append(".")
		append(1000) => append("1000");

	6.- Nevertheless, we could work on the implementation of those
cases, surely there is a clever solution, maybe maintaining different arrays
for ints, longs, chars and so on, and do it right in the toString() method.
Surely would impact somehow in the rest of cases...


> Instead of using new constructors, i think implementing a new method 
> named join
> see http://bugs.sun.com/view_bug.do?bug_id=5015163
> is better.
>
>"".join("hello","world"); => "helloworld"
>
>It can be implemented exactly in the same way that your method
>String.copyValuesInto() but i think it is more usefull.

I don't agree, I think they are two different things with two different
goals. It wouldn't an obvious use of the join method...

Of course the join method should be implemented using these new
constructor(s) or StringAppender. By the way, if there are problems for
adding 4 new constructors to the String class, they can be refactored to in
static methods, but I like using constructors for constructing instances
(call me romantic :-)


>About your code:
>
>Please use String[] var instead of String var[], i know this is legal even
>int f() [] { return new int[]{3}; }
>is legal, but it's not the Java way.
>
>In StringAppender.expandListCapacity should use Arrays.copyOf().

Both suggestions applied. Old ways are difficult to change... I started
coding in Java in the JDK 1.0 days... :-)


>StringAppender.size() should not be public, it's error prone and
>not very usefull.

True! In fact, I have deleted it since currently there is no
AbstractStringBuilder(int initialSize) constructor.


I attach the modified source. Cheers from Spain!
Jesús Viñuales
-------------- next part --------------
A non-text attachment was scrubbed...
Name: String.java.diff
Type: application/octet-stream
Size: 5654 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/core-libs-dev/attachments/20080922/299e717a/String.java.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: StringAppender.java
Type: application/octet-stream
Size: 9969 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/core-libs-dev/attachments/20080922/299e717a/StringAppender.java>