RFR: 8148937: (str) Adapt StringJoiner for Compact Strings

Сергей Цыпанов github.com+10835776+stsypanov at openjdk.java.net
Mon Mar 15 18:51:11 UTC 2021


On Mon, 15 Mar 2021 14:07:29 GMT, Claes Redestad <redestad at openjdk.org> wrote:

>> I was thinking about `StringBuilder` at the very beginning but then decided to have no bounds checks and reallocations. Indeed from maintainability point of view your solution is much more attractive. I have one minor comment about `append()`: I think we should do more chaining, e.g. 
>> StringBuilder sb = new StringBuilder(len);
>> sb.append(elts[0]);
>> should be
>> StringBuilder sb = new StringBuilder(len).append(elts[0]);
>> etc. to have less bytecode.
>> 
>> E.g. if we take 
>> public String sb() {
>>   StringBuilder sb = new StringBuilder();
>>   
>>   sb.append("a");
>>   sb.append("b");
>>   
>>   return sb.toString();
>> }
>> `sb.append()` gets compiled into
>> L1
>>  LINENUMBER 23 L1
>>  ALOAD 1
>>  LDC "a"
>>  INVOKEVIRTUAL java/lang/StringBuilder.append (Ljava/lang/String;)Ljava/lang/StringBuilder;
>>  POP
>> L2
>>  LINENUMBER 24 L2
>>  ALOAD 1
>>  LDC "b"
>>  INVOKEVIRTUAL java/lang/StringBuilder.append (Ljava/lang/String;)Ljava/lang/StringBuilder;
>>  POP
>>  ```
>>  and in case we have
>>  ```
>>  sb.append("a").append("b");
>>  ```
>>  the amount of byte code is reduced:
>>  ```
>>  L1
>>  LINENUMBER 23 L1
>>  ALOAD 1
>>  LDC "a"
>>  INVOKEVIRTUAL java/lang/StringBuilder.append (Ljava/lang/String;)Ljava/lang/StringBuilder;
>>  LDC "b"
>>  INVOKEVIRTUAL java/lang/StringBuilder.append (Ljava/lang/String;)Ljava/lang/StringBuilder;
>>  POP
>>  ```
>>  
>>  Also I'd change
>> if (addLen == 0) {
>>     compactElts();
>>     return size == 0 ? "" : elts[0];
>> }
>> to
>> if (size == 0) {
>>     if (addLen == 0) {
>>         return "";
>>     }
>>     return prefix + suffix;
>> }
>> The second variant is more understandable to me and likely to be slightly faster.
>> 
>> And finally, should I close this PR and you'll create a new one from your branch, or I need to copy your changes here?
>
> Up to you. If you adapt your PR to use a StringBuilder as suggested I can review and sponsor.

I'll reuse existing PR then.

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

PR: https://git.openjdk.java.net/jdk/pull/2627


More information about the core-libs-dev mailing list