RFR: 8336856: Optimize String Concat [v5]

[Shaojin Wen]

On Tue, 23 Jul 2024 13:35:41 GMT, Claes Redestad <redestad at openjdk.org> wrote:

>> Shaojin Wen has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains six commits:
>> 
>>  - Merge remote-tracking branch 'origin/optim_simple_concat_202407' into optim_concat_factory_202407
>>    
>>    # Conflicts:
>>    #	src/java.base/share/classes/java/lang/StringConcatHelper.java
>>    #	src/java.base/share/classes/java/lang/System.java
>>    #	src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java
>>  - reduce change & refactor
>>  - fix TRUSTED not work
>>  - non-MH-based implementation
>>  - non-MH-based implementation
>>  - optimize string concat
>
> While a reasonable start I'd suggest:
> 
> - adding a flag to switch over to the new implementation and leaving the existing strategy in place initially, then when we have something that is superior in every way we can clean out the old strategy
> - AFAICT this mimics the `SimpleStringBuilderStrategy` and generates and installs a new class for each expression. This is likely fine for high-arity concatenations which are likely to be one-of-a-kind (the sort of expressions the `SimpleStringBuilderStrategy` was brought back to life for), but could generate a proliferation of classes for low-arity expressions. Instead we probably need to aim for sharing
> - such shareable classes could be made simpler if we put them in java.lang so that they can link directly with package private methods in `StringConcatHelper` and `String` rather than using trusted `MethodHandles` to cross such boundaries
> 
> Again: I am working on and have a prototype for this which I aim to present and discuss in two weeks as JVMLS. I appreciate the effort here, but I'll need to focus on my talk and prototype for now.

Now, according to @cl4es's suggestion, this is just an improvement to the original SimpleStringBuilderStrategy's bytecode-spinning implementation, which will be used when HIGH_ARITY_THRESHOLD is reached. Such an improvement will be very low risk.

Now the question is, the default HIGH_ARITY_THRESHOLD is 20, which is too large. Should it be reduced, such as 8?

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

PR Comment: https://git.openjdk.org/jdk/pull/20273#issuecomment-2249066313