Hi Roger!
On 4/1/19 8:06 AM, Roger Riggs wrote:
Hi Ivan,
Thanks for running the micro benchmarks.
This version has more code duplication than Andrew's original
proposal that calculated the coder only CharSequence and had
a single AbstractStringBuilder constructor for computing the size
and allocating the byte[]/
http://cr.openjdk.java.net/~aleonard/8221430/webrev.00/
I'd be curious to know the JMH tests for that version compared.
That variant appeared to be slightly slower, comparing to the latest
variant:
Here are fresh benchmarks for both variants
(1) cr.openjdk.java.net/~aleonard/8221430/webrev.00 :
Benchmark Mode Cnt Score Error Units
StringBuilders.fromLatin1String avgt 18 15.217 ± 0.157 ns/op
StringBuilders.fromLatin1StringBuilder avgt 18 19.169 ± 0.086 ns/op
StringBuilders.fromUtf16String avgt 18 17.593 ± 0.180 ns/op
StringBuilders.fromUtf16StringBuilder avgt 18 21.786 ± 0.158 ns/op
(2) cr.openjdk.java.net/~igerasim/8221430/02/webrev :
Benchmark Mode Cnt Score Error Units
StringBuilders.fromLatin1String avgt 18 14.655 ± 0.133 ns/op
StringBuilders.fromLatin1StringBuilder avgt 18 18.059 ± 0.161 ns/op
StringBuilders.fromUtf16String avgt 18 16.675 ± 0.124 ns/op
StringBuilders.fromUtf16StringBuilder avgt 18 20.761 ± 0.116 ns/op
One reason might be that (2) avoids a redundant check for negative
length of the String argument.
Another comment is whether the 'instanceof' code is the
best performer for checking if the argument is a String.
I might think that 'seq.getClass().equals(String.class)' is faster.
Interesting. I don't see examples of such pattern in JDK.
Anyhow, I think that the case when a StringBuilder is constructed from a
String down-cast to CharSequence should be rare in practice, so it is
sensible to keep the code more ideomatic, i.e. use instanceof.
And in this most recent webrev that has separated the
AbstractStringBuilder
constructors for String from CharSequence, is it more likely that the
argument
will be an AbstractStringBuilder than a String so that comparison
should be done first.
Yes, it's a good point! I reordered the branches in the latest webrev.
With kind regards,
Ivan
