RFR: 8338930: StringConcatFactory hardCoded string concatenation strategy [v4]

[Claes Redestad]

On Mon, 26 Aug 2024 20:51:35 GMT, Shaojin Wen <duke at openjdk.org> wrote:

>> This is a follow-up to PR #20273, which improves performance when the number of parameters exceeds 20.
>> 
>> When the number of parameters is large, the possibility of reuse will be lower, so we can use the static concat method and write the length and coder directly into the bytecode to solve the performance regression problem.
>
> Shaojin Wen has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains nine commits:
> 
>  - optimize for CompactStrings is off
>  - Merge remote-tracking branch 'upstream/master' into optim_concat_factory_202408
>    
>    # Conflicts:
>    #	src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java
>  - add control flag `reuseThreshold`
>  - Revert "Optimize the construction of MethodType and MethodTypeDesc to reduce memory allocation"
>    
>    This reverts commit 3bed7290f5cb987e86407f698fb0598f19d65628.
>  - Optimize the construction of MethodType and MethodTypeDesc to reduce memory allocation
>  - revert code style
>  - from suggest
>  - Update src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java
>    
>    Co-authored-by: ExE Boss <3889017+ExE-Boss at users.noreply.github.com>
>  - support staticConcat

I'm not too keen on `reuseThreshold` (perhaps `cacheThreshold`?) being enabled by default until we can get some tuning data from real world usage here. Does it harm applications by increasing number of generated classes by a large amount? Does the improved performance at each call-site matter, or is it more important to share common concat classes to reduce startup/warmup overheads? I'd be more at ease if we initially added this feature as an opt-in (`reuseThreshold` set to some value above 100) and then do a tuning pass at a future time.

Disabling the generation of the `coder` methods if running with `-XX:-CompactStrings` is probably fine. Perhaps this should be split out to a separate PR. It adds some concerns regarding code pre-generation (might be best to always use the coder-checking `+CompactStrings` form when pre-generating to avoid bugs)

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

PR Comment: https://git.openjdk.org/jdk/pull/20675#issuecomment-2311141871