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

[Сергей Цыпанов]

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
Index: src/java.base/share/classes/java/lang/AbstractStringBuilder.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/java.base/share/classes/java/lang/AbstractStringBuilder.java	(revision 0b83fc0150108c7250da3e80ec8f02deb3b5861c)
+++ src/java.base/share/classes/java/lang/AbstractStringBuilder.java	(date 1601034973902)
@@ -157,8 +157,8 @@
             return 0;
         }

-        byte val1[] = value;
-        byte val2[] = another.value;
+        byte[] val1 = value;
+        byte[] val2 = another.value;
         int count1 = this.count;
         int count2 = another.count;

@@ -734,7 +734,7 @@
      *         if {@code offset < 0} or {@code len < 0}
      *         or {@code offset+len > str.length}
      */
-    public AbstractStringBuilder append(char str[], int offset, int len) {
+    public AbstractStringBuilder append(char[] str, int offset, int len) {
         int end = offset + len;
         checkRange(offset, end, str.length);
         ensureCapacityInternal(count + len);
@@ -1239,9 +1239,6 @@
         if (s == null) {
             s = "null";
         }
-        if (s instanceof String) {
-            return this.insert(dstOffset, (String)s);
-        }
         return this.insert(dstOffset, s, 0, s.length());
     }

@@ -1301,7 +1298,11 @@
         ensureCapacityInternal(count + len);
         shift(dstOffset, len);
         count += len;
-        putCharsAt(dstOffset, s, start, end);
+        if (s instanceof String) {
+            putCharsAt(dstOffset, (String) s, start, end);
+        } else {
+            putCharsAt(dstOffset, s, start, end);
+        }
         return this;
     }

@@ -1559,9 +1560,8 @@
     public AbstractStringBuilder reverse() {
         byte[] val = this.value;
         int count = this.count;
-        int coder = this.coder;
         int n = count - 1;
-        if (COMPACT_STRINGS && coder == LATIN1) {
+        if (isLatin1()) {
             for (int j = (n-1) >> 1; j >= 0; j--) {
                 int k = n - j;
                 byte cj = val[j];
@@ -1649,7 +1649,7 @@
      * @param dstBegin  the char index, not offset of byte[]
      * @param coder     the coder of dst[]
      */
-    void getBytes(byte dst[], int dstBegin, byte coder) {
+    void getBytes(byte[] dst, int dstBegin, byte coder) {
         if (this.coder == coder) {
             System.arraycopy(value, 0, dst, dstBegin << coder, count << coder);
         } else {        // this.coder == LATIN && coder == UTF16
@@ -1713,6 +1713,15 @@
             StringUTF16.putCharsSB(this.value, index, s, off, end);
         }
     }
+
+    private void putCharsAt(int index, String s, int off, int end) {
+        if (isLatin1() && s.isLatin1()) {
+            System.arraycopy(s.value(), off, this.value, index, end);
+            return;
+        }
+        inflate();
+        StringUTF16.putCharsSB(this.value, index, s, off, end);
+    }

     private final void putStringAt(int index, String str) {
         if (getCoder() != str.coder()) {