[PATCH] AbstractStringBuilder.insert(int dstOffset, CharSequence s, int start, int end) is missing fast-path for String

[Roger Riggs]

Hi Sergey,

Your request can be tracked with the Jira issue:
    https://bugs.openjdk.java.net/browse/JDK-8254082

Please follow the directions for How to Contribute under "Become a 
Contributor".
    https://openjdk.java.net/contribute/

When you get to creating the PR, please include the JMH 
test/micro/org/openjdk/bench/java/lang.

Regards, Roger


On 9/25/20 10:08 AM, Сергей Цыпанов wrote:
> Hello,
>
> while working with StringBuilder.insert() I've spotted that its delegate AbstractStringBuilder.insert() is missing
> a fast-path for the most frequent case when its argument is String.
>
> Previously they did similart optimization for StirngBuilder.append(CharSequence, int, int),
> see https://bugs.openjdk.java.net/browse/JDK-8224986
>
> I'd like to contribute a trivial patch that brings improvement for the case when SB's content is Latin1
> and inserted String is Latin1 as well. I cannot file the issue for that so can I create a PR on GitHub without issue number?
> If not please create one (if the change is relevant of course).
>
> To measure improvement I've used simple benchmark:
>
> @BenchmarkMode(Mode.AverageTime)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
> public class StringBuilderInsertBenchmark {
>
>    @Benchmark
>    public StringBuilder insert(Data data) {
>      String string = data.string;
>      return new StringBuilder().append("ABC").insert(1, string, 1, data.length + 1);
>    }
>
>    @State(Scope.Thread)
>    public static class Data {
>      String string;
>
>      @Param({"true", "false"})
>      private boolean latin;
>
>      @Param({"8", "64", "128", "1024"})
>      private int length;
>
>      @Setup
>      public void setup() {
>        String alphabet = latin
>          ? "abcdefghijklmnopqrstuvwxyz"        // English
>          : "абвгдеёжзиклмнопрстуфхцчшщьыъэюя"; // Russian
>
>        string = new RandomStringGenerator().randomString(alphabet, length + 2);
>      }
>    }
> }
>
> Which gives
>
>              (latin)  (length)       original       patched    Units
>
> insert         true         8    24.2 ±  0.1   22.2 ±  0.0    ns/op
> insert         true        64    53.8 ±  0.2   36.1 ±  0.1    ns/op
> insert         true       128    80.9 ±  0.2   44.6 ±  0.0    ns/op
> insert         true      1024   365.4 ±  0.5  109.8 ±  3.9    ns/op
>
> insert        false         8    33.5 ±  0.5   32.3 ±  0.2    ns/op
> insert        false        64    73.2 ±  0.3   73.2 ±  0.2    ns/op
> insert        false       128   103.9 ±  0.6  103.3 ±  0.1    ns/op
> insert        false      1024   576.5 ±  4.8  569.5 ±  2.0    ns/op
>
> Patch is attached. As of tests tier1 and tier2 are ok
>
> With best regards,
> Sergey Tsypanov