set capacity for StringBuilder in String#repeat

[tuchida]

repeat.js

for (var i = 0; i < 10000; i++) {
  'abcde'.repeat(10000);
}

non capacity

$ time java -jar buildGradle/libs/rhino-1.7.7-SNAPSHOT.jar repeat.js 

real    0m1.279s
user    0m1.543s
sys 0m0.101s

set capacity

$ time java -jar buildGradle/libs/rhino-1.7.7-SNAPSHOT.jar repeat.js

real    0m1.147s
user    0m1.432s
sys 0m0.111s

[sainaen]

Good catch!

But

long size = str.length() * (int) cnt;
// Check for overflow
if (size >= Integer.MAX_VALUE) {
  size = Integer.MAX_VALUE;
}

is not a correct way to check for overflow, because str.length() returns int and cnt is casted to it, the multiplication will be performed on int-s first (with possible overflow) and only result will be widened to long. What we want is to at least one operand be of type long, to force widening before multiplication.

[tuchida]

@sainaen Thank you! I fixed.

[sainaen]

@tuchida
👍

[tuchida]

Possibly, it might be able to more quickly. (Japanese page, sorry)
http://d.hatena.ne.jp/teramako/20140403/p1

[sainaen]

Huh.

Warning: My performance-testing-fu is really, really weak (it's literally my first use of JMH), so please don't take results seriously without looking into benchmark. (plus, I would really like to get some feedback on it.)

With that said, here is my benchmarking results (JDK 1.8.0_45):

Benchmark                             Mode  Cnt      Score     Error  Units
MyBenchmark.testMethod_Doubling      thrpt   60  10647.154 ± 287.211  ops/s
MyBenchmark.testMethod_Naive         thrpt   60   4124.734 ± 118.466  ops/s
MyBenchmark.testMethod_NaivePrealoc  thrpt   60   4960.324 ± 171.472  ops/s

So while preallocating StringBuilder helps, using more advanced (a.k.a. complex) implementation doubles the throughput*.

PS. Interestingly Groovy has this multiply() method added to String, and it does simple iteration, even without specifying StringBuilder capacity. Maybe it's not worth it to do something complex?

UPD:
Found repeat() in V8 and ported** it to Java, updated benchmark and added results for Java 6 and 8 to readme. Doubling method is still the fastest.

UPD2:
And we have a new winner: same as doubling, but now use single substring() instead of a second while loop. Also for loop was replaced with a while in original doubling method. (results were updated accordingly.)

UPD3:
With your repeat.js test, I got these numbers:
current master version:

real    0m2.200s
user    0m2.312s
sys     0m0.416s

doubling with substring method:

real    0m0.829s
user    0m1.268s
sys     0m0.224s

looks like an improvement to me. 📈 :)

* — probably. (please, read that warning again ↑)
** — more or less.

[gbrail]

The last implementation is very clever and seems to help a lot with performance.

@tuchida do you want to try and update the PR?

[sainaen]

Yes, it is. I wish I could say that I came up with it myself, but I found it in the jsperf test linked in the article from @tuchida, so all thanks go to him for the link and original author for the example. :)

Now, I see an issue in how we treat the overflow: when size is bigger than Integer.MAX_VALUE we still can overflow during iteration, even though we will always create StringBuilder with maximum allocated size of Integer.MAX_VALUE. I think we should throw RangeError in that if, as we cannot create String with more characters than Integer.MAX_VALUE, but right now I cannot find in the spec is it allowed or not and, if it is, in what form it should be (maybe we should throw some other error instead of RangeError).

[tuchida]

I tried to improve the https://github.com/sainaen/StringRepeatBenchmark/blob/master/src/main/java/com/sainaen/V8Repeater.java

JavaScript String is Rope String.
https://github.com/mozilla/rhino/blob/master/src/org/mozilla/javascript/ConsString.java

So it is to combine string is fast.
Where I tried in Java8, https://github.com/sainaen/StringRepeatBenchmark/blob/master/src/main/java/com/sainaen/DoublingRepeater.java was most fast.

[sainaen]

JavaScript String is Rope String.

It's true, but here we always cast it to regular Java String, so no gain with that, IMO.

Where I tried in Java8, https://github.com/sainaen/StringRepeatBenchmark/blob/master/src/main/java/com/sainaen/DoublingRepeater.java was most fast.

How did you tested that? I'm asking, because the worst case for doubling method is when the counter has value 2^n - 1, which will effectively cost us log2(counter) + 2^(n-1) - 1 calls to append(). Doubling with substring doesn't have that flaw. (Actually, I don't think I know the worst case for that method, except 2^n + 1 counter, when we do substring() call instead of using original string, but it could be optimized as a special case too. Maybe you see what I'm missing here?)

I've tried to run your repeat.js with str.repeat(Math.pow(2, 14) - 1), and on my machine results look like this (JDK 1.8.0_45):

// unoptimized:
real    0m3.079s
user    0m3.348s
sys     0m0.256s

// doubling with the substring():
real    0m1.214s
user    0m1.492s
sys     0m0.356s

// doubling:
real    0m1.889s
user    0m2.228s
sys     0m0.260s

But ok, your current version is good enough for me. And we can address that issue with overflow in a separate PR later.
Thanks for bringing this up! 👍

[gbrail]

Good point about "ConsString." If we want this (and other NativeString methods) to be really efficient, we should do something smart with that. Otherwise, we first have to flatten the ConsString (also using StringBuilder) before we start to repeat it.

A good way to test would be to see if a string like:

'foo' + 'bar' + 'baz'

performs differently than just 'foobarbaz'.

Also, for settling these small performance issues, the best way to know for sure is to test with a framework like Google Caliper:

https://code.google.com/p/caliper/

It makes sure that you run the micro-benchmark enough times until it gets a repeatable result (due to compilation, etc), that GC doesn't have a big effect, and so on. It's worth trying.

[sainaen]

@gbrail
About Caliper — I'd really recommend JMH over Caliper. The former is written by Oracle's own performance engineer and works around a lot of common pitfalls of OpenJDK/Hotspot. Here's the slides of the talk about JMH and the video if you interested. Also, for example here's an issue when Caliper vs. JMH discussed for testing RxJava.

A good way to test would be to see if a string like:
'foo' + 'bar' + 'baz'
performs differently than just 'foobarbaz'.

Here's a difference between "abcde".repeat(Math.pow(2, 14) - 1) vs ("ab" + "cd" + "e").repeat(Math.pow(2, 14) - 1) (using good old time as it's not really a micro-benchmark):

// ("ab" + "cd" + "e").repeat(Math.pow(2, 14) - 1)
real    0m1.213s
user    0m1.528s
sys     0m0.300s

// "abcde".repeat(Math.pow(2, 14) - 1)
real    0m1.166s
user    0m1.480s
sys     0m0.296s

so, it looks like it doesn't affect performance much.

[gbrail]

Thanks -- will check out JMH next time. Just making sure that we don't
repeat common performance mistakes of running microbenchmarks.

On Fri, May 8, 2015 at 1:48 PM, Ivan Vyshnevskyi notifications@github.com
wrote:

@gbrail https://github.com/gbrail
About Caliper — I'd really recommend JMH over Caliper. The former is
written by Oracle's own performance engineer and works around a lot of
common pitfalls of OpenJDK/Hotspot. Here's the slides
http://shipilev.net/talks/devoxx-Nov2013-benchmarking.pdf of the talk
about JMH and the video https://vimeo.com/78900556 if you interested.
Also, for example here's an issue
ReactiveX/RxJava#776 when Caliper vs. JMH
discussed for testing RxJava.

A good way to test would be to see if a string like:
'foo' + 'bar' + 'baz'
performs differently than just 'foobarbaz'.

Here's a difference between "abcde".repeat(Math.pow(2, 14) - 1) vs ("ab"

• "cd" + "e").repeat(Math.pow(2, 14) - 1):

// ("ab" + "cd" + "e").repeat(Math.pow(2, 14) - 1)
real 0m1.213s
user 0m1.528s
sys 0m0.300s

// "abcde".repeat(Math.pow(2, 14) - 1)
real 0m1.166s
user 0m1.480s
sys 0m0.296s

so, it looks like it doesn't affect performance much.

—
Reply to this email directly or view it on GitHub
#188 (comment).

greg brail | apigee https://apigee.com/ | twitter @gbrail
http://twitter.com/gbrail

[tuchida]

Although this my idea is uncool way...

When replace StringBuilder with char[] and use System.arraycopy,
https://github.com/tuchida/StringRepeatBenchmark/blob/arraycopy/src/main/java/com/sainaen/str_repeat/ArrayCopy_DoublingWithSubstringRepeater.java

fast a little.
https://github.com/tuchida/StringRepeatBenchmark/blob/arraycopy/README.md#java-180_45

MyBenchmark.testMethod_ArrayCopy_DoublingWithSubstring  thrpt  100  19215.778 ± 178.935  ops/s
MyBenchmark.testMethod_Doubling                         thrpt  100  15285.772 ± 155.174  ops/s
MyBenchmark.testMethod_DoublingWithSubstring            thrpt  100  17511.073 ± 150.687  ops/s

Maybe, ConsString can be faster in the same way.
https://github.com/mozilla/rhino/blob/master/src/org/mozilla/javascript/ConsString.java#L64

[gbrail]

This has been a good discussion and in general this sounds like a good change to make.

Would you like to consolidate the thread above into one change that you think is best?

[gbrail]

OK -- we can keep trying additional optimizations, but the "doubling" one that tuchida posted seems like a good and simple improvement so I pushed it. We can keep elaborating if you'd like.

We can also improve other areas of performance if you guys are interested -- remember that the first commit in this repo is from 1999 -- we have learned a little bit about Java performance since then!

[tuchida]

Thanks!

And then, ConsString did not make fast.
https://github.com/tuchida/ConsStringBenchmark