RFR [8054221] StringJoiner imlementation optimization

[Martin Buchholz]

I have more comments, but this is good to go.

  69     @Test(expectedExceptions = {NullPointerException.class})


Controversial - many people, including myself, think this is too magical
without adding much value.  My own preference is assertThrows e.g. in
http://gee.cs.oswego.edu/cgi-bin/viewcvs.cgi/jsr166/src/test/tck/JSR166TestCase.java?revision=1.121&view=markup

Microbenchmarks undermeasure the impact of allocation, which increases with
heap size.  Real world java applications struggle with allocation/gc, not
cpu.  So don't measure allocation against cpu directly in microbenchmarks.





On Wed, Aug 6, 2014 at 2:47 AM, Ivan Gerasimov <ivan.gerasimov at oracle.com>
wrote:

>
> On 06.08.2014 10:14, Martin Buchholz wrote:
>
>    39     private final static String PREFIXES[] = {"", "{", "@#$%"};
>
> This C style syntax is not good Java style - use String[] instead.
>
>   Thanks! fixed
>
>
>    177         if (addLen == 0) { 178             compactElts(); 179             return size == 0 ? "" : elts[0]; 180         }
>
> I'm concerned about the extra String[8] created by compactElts.  We assume
> that StringJoiners are all temporary objects, so don't bother creating that
> shorter String[8] to hold the one sole element on compaction - just reuse
> the original array (but should we null out the entries? probably)
>
>
> The benchmark showed that allocating a new array is faster than nullifying
> the existing one.
> But if we set the entries to null in the loop that is already there, it
> will probably be no slower than allocation.
>
>
> Here's the updated webrev:
> http://cr.openjdk.java.net/~igerasim/8054221/2/webrev/
>
> Sincerely yours,
> Ivan
>
>
>
> On Tue, Aug 5, 2014 at 12:19 PM, Ivan Gerasimov <ivan.gerasimov at oracle.com
> > wrote:
>
>>
>>  But if we do that, I think we should optimize size == 0 as well, thus:
>>>>
>>>> if (addLen == 0 && size <= 1)
>>>>  return (size == 0) ? "" : elts[0];
>>>>
>>> Yes, done.
>>>
>>>  Or we can just call compactElts() if addLenn == 0, so it will work for
>> any size.
>>
>> Updated the webrev at the same location:
>> http://cr.openjdk.java.net/~igerasim/8054221/1/webrev/
>>
>> Sincerely yours,
>> Ivan
>>
>>
>
>