Re: [TRIVIAL] Fast-path for String.subsring(n,n)

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

Hello,

I've moved this fast-path into both 
StringUTF16.newString/StringLatin1.newString.

Patch is attached.

Test-tier1 is OK, though I have a question about 
StringLatin1/StringUTF16.stripLeading()
and StringLatin1/StringUTF16.stripTrailing():

Currently we have 

public static String stripLeading(byte[] value) {
  int left = indexOfNonWhitespace(value);
  if (left == value.length) {
    return "";
  }
  return (left != 0) ? newString(value, left, value.length - left) : null;
}

With the patch we change behaviour of this method for the case when 
value.length == 0:

public static String stripLeading(byte[] value) {
  int left = indexOfNonWhitespace(value);
  return (left != 0) ? newString(value, left, value.length - left) : null;
}

Unlike original method this code returns null instead of "" for empty array.
This does not affect caller (String.stripLeading()) i. e. visible behaviour 
remains the same for String user, but is it OK in general?

If yes, is there anyone who could sponsor this change?

With best regards,
Sergey Tsypanov

25.02.2020, 18:41, "Ivan Gerasimov" <ivan.gerasi...@oracle.com>:
> Hi Sergei!
>
> Wouldn't it make sense to incorporate this fast path into
> StringUTF16.newString/StringLatin1.newString?
>
> This way other callers of these methods will also benefit from it.
>
> In particular, StringLatin1/StringUTF16.stripLeading(),
> StringLatin1/StringUTF16.stripTrailing() and maybe others could be
> slightly simplified.
>
> With kind regards,
>
> Ivan
>
> On 2/25/20 7:42 AM, Сергей Цыпанов wrote:
>>  Hello,
>>
>>  current implementation of String.substring(int,int) has a fast-path for the 
>> case when beginIndex = 0 and endIndex = length.
>>
>>  I think there should be similar fast-path for the case beginIndex = 
>> endIndex (asuming both are valid):
> --
> With kind regards,
> Ivan Gerasimov
diff --git a/src/java.base/share/classes/java/lang/String.java b/src/java.base/share/classes/java/lang/String.java
--- a/src/java.base/share/classes/java/lang/String.java
+++ b/src/java.base/share/classes/java/lang/String.java
@@ -1819,7 +1819,7 @@
      * @param   src         the characters being searched.
      * @param   srcCoder    coder handles the mapping between bytes/chars
      * @param   srcCount    count of the source string.
-     * @param   tgt         the characters being searched for.
+     * @param   tgtStr      the characters being searched for.
      * @param   fromIndex   the index to begin searching from.
      */
     static int lastIndexOf(byte[] src, byte srcCoder, int srcCount,
@@ -1900,10 +1900,10 @@
     public String substring(int beginIndex, int endIndex) {
         int length = length();
         checkBoundsBeginEnd(beginIndex, endIndex, length);
-        int subLen = endIndex - beginIndex;
         if (beginIndex == 0 && endIndex == length) {
             return this;
         }
+        int subLen = endIndex - beginIndex;
         return isLatin1() ? StringLatin1.newString(value, beginIndex, subLen)
                           : StringUTF16.newString(value, beginIndex, subLen);
     }
diff --git a/src/java.base/share/classes/java/lang/StringLatin1.java b/src/java.base/share/classes/java/lang/StringLatin1.java
--- a/src/java.base/share/classes/java/lang/StringLatin1.java
+++ b/src/java.base/share/classes/java/lang/StringLatin1.java
@@ -630,17 +630,11 @@
 
     public static String stripLeading(byte[] value) {
         int left = indexOfNonWhitespace(value);
-        if (left == value.length) {
-            return "";
-        }
         return (left != 0) ? newString(value, left, value.length - left) : null;
     }
 
     public static String stripTrailing(byte[] value) {
         int right = lastIndexOfNonWhitespace(value);
-        if (right == 0) {
-            return "";
-        }
         return (right != value.length) ? newString(value, 0, right) : null;
     }
 
@@ -764,6 +758,9 @@
     }
 
     public static String newString(byte[] val, int index, int len) {
+        if (len == 0) {
+            return "";
+        }
         return new String(Arrays.copyOfRange(val, index, index + len),
                           LATIN1);
     }
diff --git a/src/java.base/share/classes/java/lang/StringUTF16.java b/src/java.base/share/classes/java/lang/StringUTF16.java
--- a/src/java.base/share/classes/java/lang/StringUTF16.java
+++ b/src/java.base/share/classes/java/lang/StringUTF16.java
@@ -1016,18 +1016,12 @@
     public static String stripLeading(byte[] value) {
         int length = value.length >>> 1;
         int left = indexOfNonWhitespace(value);
-        if (left == length) {
-            return "";
-        }
         return (left != 0) ? newString(value, left, length - left) : null;
     }
 
     public static String stripTrailing(byte[] value) {
         int length = value.length >>> 1;
         int right = lastIndexOfNonWhitespace(value);
-        if (right == 0) {
-            return "";
-        }
         return (right != length) ? newString(value, 0, right) : null;
     }
 
@@ -1132,6 +1126,9 @@
     }
 
     public static String newString(byte[] val, int index, int len) {
+        if (len == 0) {
+            return "";
+        }
         if (String.COMPACT_STRINGS) {
             byte[] buf = compress(val, index, len);
             if (buf != null) {