On Wed, 24 Jul 2024 11:43:45 GMT, Shaojin Wen <d...@openjdk.org> wrote:
>> The current implementation of StringConcat is to mix the coder and length >> into a long. This operation will have some overhead for int/long/boolean >> types. We can separate the calculation of the coder from the calculation of >> the length, which can improve the performance in the scenario of concat >> int/long/boolean. >> >> This idea comes from the suggestion of @l4es in the discussion of PR >> https://github.com/openjdk/jdk/pull/20253#issuecomment-2240412866 > > Shaojin Wen has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 32 commits: > > - Merge remote-tracking branch 'upstream/master' into > optim_concat_factory_202407 > - typo > - change comments > - minor refactor > - minor refactor > - reduce change > - copyright > - reduce change > - refactor based on 8335182 > - use Float.toString & Double.toString > - ... and 22 more: https://git.openjdk.org/jdk/compare/05d88de0...6faecfd7 I will have a more in-depth review later, and help resolve the access issues with Lookup and SecurityManager. src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java line 387: > 385: try { > 386: if (concatType.parameterCount() <= HIGH_ARITY_THRESHOLD) { > 387: mh = generateMHInlineCopy(concatType, constantStrings); You can consider using `concatType.erase()` src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java line 1119: > 1117: > 1118: private static MethodHandle generate(Lookup lookup, MethodType > args, String[] constants) throws Exception { > 1119: lookup = MethodHandles.Lookup.IMPL_LOOKUP; I think you can try moving the inline copy generation to java.lang's StringConcatHelper; this way, we don't need to expose stringSize, coder etc. through JavaLangAccess (we may update String impl detail in the future) and just access the generated MethodHandle. ------------- PR Review: https://git.openjdk.org/jdk/pull/20273#pullrequestreview-2196930455 PR Review Comment: https://git.openjdk.org/jdk/pull/20273#discussion_r1689931670 PR Review Comment: https://git.openjdk.org/jdk/pull/20273#discussion_r1689935833
