On 4 Dec 2016, at 08:42, Ivan Gerasimov <ivan.gerasi...@oracle.com> wrote:
Thank you Claes for looking into it!
On 04.12.2016 16:48, Claes Redestad wrote:
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:
Yes, of course.
If people find it useful, I would expect it to go to jdk 10.
- 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.
The do-while loop saves us one comparison, comparing to the loop in
Arrays.fill().
I'd prefer to keep the explicit loop, as it's only three lines of code long.
- 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.
Yes, I understand your point. There are different approaches to handling
arguments.
E.g. for indices it might be allowed to have from > to (treat it as from == to),
and from < 0.
Or, like in Perl, negative index might be treated as offset from the end of a
list or array.
However, in Java, the tradition seems to have formed to have strong argument
checks, not allowing much interpretation.
For example, similarly looking Collections.nCopies(int n, T) also throws IAE
for negative n.
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.
It was intentional, to be honest.
If appending several chars causes reallocation, then appending chars in a loop
can only be slower, comparing to appendN() or append(String).
I didn't want to find the sharp constant of the speedup factor, but just wanted
to be sure that the increase in performance is observable.
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?
I'm not sure exactly why appending short String is slower then filling.
Might it be because the former means both reading from and writing to the
memory, and the later only means writing?
Anyways, I only wanted to make sure that replacing the code in BigInteger and
FDBigInteger won't make things slower.
- please use -tu us -bm avgt or annotate benchmarks to output scores with a
reasonable
number of digits.
Sure. Here you go:
Benchmark (size) Mode Cnt Score Error Units
MyBenchmark.test_0_New 0 avgt 4 0.003 ± 0.004 us/op
MyBenchmark.test_0_New 1 avgt 4 0.005 ± 0.008 us/op
MyBenchmark.test_0_New 5 avgt 4 0.014 ± 0.015 us/op
MyBenchmark.test_0_New 10 avgt 4 0.016 ± 0.019 us/op
MyBenchmark.test_0_New 20 avgt 4 0.018 ± 0.010 us/op
MyBenchmark.test_1_Old 0 avgt 4 0.003 ± 0.001 us/op
MyBenchmark.test_1_Old 1 avgt 4 0.006 ± 0.004 us/op
MyBenchmark.test_1_Old 5 avgt 4 0.023 ± 0.021 us/op
MyBenchmark.test_1_Old 10 avgt 4 0.049 ± 0.071 us/op
MyBenchmark.test_1_Old 20 avgt 4 0.089 ± 0.110 us/op
MyBenchmark.test_2_Solid 0 avgt 4 0.007 ± 0.003 us/op
MyBenchmark.test_2_Solid 1 avgt 4 0.018 ± 0.024 us/op
MyBenchmark.test_2_Solid 5 avgt 4 0.016 ± 0.011 us/op
MyBenchmark.test_2_Solid 10 avgt 4 0.017 ± 0.016 us/op
MyBenchmark.test_2_Solid 20 avgt 4 0.016 ± 0.007 us/op
With kind regards,
Ivan
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
