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
