RFR: 8336831: Optimize StringConcatHelper.simpleConcat [v2]

[Shaojin Wen]

On Fri, 19 Jul 2024 20:43:42 GMT, Chen Liang <liach at openjdk.org> wrote:

>> Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   share newArray
>
> As annoying and risky as this first appeared, this patch is actually in quite good shape: The usage of `getBytes` is similar to those in StringUTF16, and there's no reason to further complicate this by splitting the handling into StringLatin1 and StringUTF16. 👍 
> 
> Another question for you: can you check out the original form over here too? https://github.com/openjdk/jdk/commit/781fb29580b08fa659019a2da09488ee4711c017#diff-f8131d8a48caf7cfc908417fad241393c2ef55408172e9a28dcaa14b1d73e1fbL1968-L1981
> 
> `simpleConcat` is required to create new String objects and `String.concat` can just return the argument/original when one part is not empty. Is there any value to extract a common `doConcat` handling both non-empty strings, called by both methods after handling the empty cases?

@liach is this what you want to change it to?

class String {
    public String concat(String str) {
        if (str.isEmpty()) {
            return this;
        }
        return StringConcatHelper.doConcat(this, str);
    }
}

class StringConcatHelper {
    @ForceInline
    static String simpleConcat(Object first, Object second) {
        String s1 = stringOf(first);
        String s2 = stringOf(second);
        if (s1.isEmpty()) {
            // newly created string required, see JLS 15.18.1
            return new String(s2);
        }
        if (s2.isEmpty()) {
            // newly created string required, see JLS 15.18.1
            return new String(s1);
        }
        return doConcat(s1, s2);
    }

    @ForceInline
    static String doConcat(String s1, String s2) {
        byte coder = (byte) (s1.coder() | s2.coder());
        int newLength = (s1.length() + s2.length()) << coder;
        byte[] buf = newArray(newLength);
        s1.getBytes(buf, 0, coder);
        s2.getBytes(buf, s1.length(), coder);
        return new String(buf, coder);
    }
}

-------------

PR Comment: https://git.openjdk.org/jdk/pull/20253#issuecomment-2240106042