On Tue, 23 Jul 2024 13:35:41 GMT, Claes Redestad <redes...@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
