RFR 8170348: Appendable.appendN(char, int) method to append multiple copies of char

[Claes Redestad]

Hi Ivan,

as this adds a new public API I guess it's too late for 9 at this point, 
but here's a few
comments anyhow:

- you could use Arrays.fill(byte[], int, int, byte) for LATIN-1 case in 
AbstractStringBuilder.
Might not make it much faster (unless there are intrinsics at play, but 
perhaps a bit less
verbose.

- for a convenience API like this, I think it's slightly awkward that a 
negative n throws IAE
since users have to think carefully about whether they need to guard the 
call to appendN
with a range check or not. I'd find this utility more useful if it was 
more forgiving and
allowed simplifying the caller further.

Benchmark comments:

- since you're reusing a StringBuilder you're effectively removing the 
impact of resizing
the underlying buffer, which is typically a significant part of 
appending, so while this
zooms in on the cost of actually appending to a prepared builder, it 
might overstate the
effect. Creating new StringBuilders (of varying sizes) in a setup method 
outside the
@Benchmark method might be more in line with typical use, in addition to 
what you have
now (which is zooming in on the cost of appending without allocation 
overhead).
setLength(0) could also be moved to an invocation level @Setup method)

- seeing that appending a String, which uses System.arraycopy, can be 
slower for small
strings is a bit surprising as I'd assume it'd be completely 
intrinsified. Is the compiled code
making a JNI transition or are things not being inlined properly?

- please use -tu us -bm avgt or annotate benchmarks to output scores 
with a reasonable
number of digits.

Thanks!

/Claes

On 12/04/2016 04:07 AM, Ivan Gerasimov wrote:
> Hello!
>
> There are several places in JDK where the same character is appended 
> to a StringBuilder object multiple times (usually padding).
> With each append there are a few routine checks performed.
> They could have been done only once, if we had a method for appending 
> multiple copies at a time.
> A simple benchmark shows that such method may save us a few machine 
> cycles (see the results below).
>
> In the benchmark, three approaches were compared:
> 0) Using the new appendN(char, int) method to append several chars at 
> once,
> 1) Calling append(char) in a loop,
> 2) Appending a prepared-in-advance string
>
> On my machine, the new method demonstrates better or comparable 
> performance for all sizes up to 20.
>
> In the webrev, there are two changesets included:
> - the new default Appendable.appendN(char, int) method, its overrides 
> in StringBuilder/Buffer and a basic test,
> - several applications of the new method across JDK.
>
> Would you please help review?
> Comments, suggestions are welcome.
>
> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8170348
> WEBREV: http://cr.openjdk.java.net/~igerasim/8170348/00/webrev/
> Benchmark: 
> http://cr.openjdk.java.net/~igerasim/8170348/00/MyBenchmark.java
>
>
> Benchmark                 (size)   Mode  Cnt Score          Error Units
> MyBenchmark.test_0_New         0  thrpt   70  331922128.215 ± 
> 16399254.452  ops/s
> MyBenchmark.test_0_New         1  thrpt   70  209207932.893 ± 
> 14955800.231  ops/s
> MyBenchmark.test_0_New         5  thrpt   70   72926671.621 ± 
> 4841791.555  ops/s
> MyBenchmark.test_0_New        10  thrpt   70   67779575.053 ± 
> 3234366.239  ops/s
> MyBenchmark.test_0_New        20  thrpt   70   59731629.661 ± 
> 2769497.288  ops/s
> MyBenchmark.test_1_Old         0  thrpt   70  333467628.860 ± 
> 15981678.430  ops/s
> MyBenchmark.test_1_Old         1  thrpt   70  156126381.967 ± 
> 9619653.294  ops/s
> MyBenchmark.test_1_Old         5  thrpt   70   46550204.382 ± 
> 2009987.637  ops/s
> MyBenchmark.test_1_Old        10  thrpt   70   23309297.849 ± 
> 1268874.282  ops/s
> MyBenchmark.test_1_Old        20  thrpt   70   13143637.821 ± 
> 662265.103  ops/s
> MyBenchmark.test_2_Solid       0  thrpt   70  138548108.540 ± 
> 6408775.462  ops/s
> MyBenchmark.test_2_Solid       1  thrpt   70   63890936.132 ± 
> 3918274.970  ops/s
> MyBenchmark.test_2_Solid       5  thrpt   70   65838879.075 ± 
> 2701493.698  ops/s
> MyBenchmark.test_2_Solid      10  thrpt   70   65387238.993 ± 
> 3131562.548  ops/s
> MyBenchmark.test_2_Solid      20  thrpt   70   57528150.828 ± 
> 3171453.716  ops/s
>
>
> With kind regards,
> Ivan
>