<i18n dev> RFR: 8335366: Improve String.format performance with fastpath [v9]

[Claes Redestad]

On Mon, 1 Jul 2024 06:18:55 GMT, Chen Liang <liach at openjdk.org> wrote:

>> Shaojin Wen has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   code style
>
> As promising as the performance number is, I think we need to ensure two things:
> 1. Correctness: this patch adds a lot of special cases; not sure if the current test cases already cover all of them. In addition, format is i18n stuff, which will need extra review besides the review for performance gains.
> 2. Validity: the existing benchmarks don't have profile pollution: see `ReflectionSpeedBenchmark` https://github.com/openjdk/jdk/blob/d9bcf061450ebfb7fe02b5a50c855db1d9178e5d/test/micro/org/openjdk/bench/java/lang/reflect/ReflectionSpeedBenchmark.java#L291 where the `Method.invoke` and `Constructor.newInstance` are called to tamper JIT's profiling, as JIT can conclude that only one format shape is ever used in your benchmark, which is unlikely in production. You should call `String.format` and `String.formatted` with varied format strings and arguments in setup for profile pollution.<br>
>   An extreme example would be at https://github.com/openjdk/jdk/pull/14944#issuecomment-1644050455, where `Arrays.hashCode(Object[])` where every element is an `Integer` is extremely fast, but slows down drastically once different arrays are passed.

Fully agree with the points @liach made above that this needs to be scrutinized for correctness and validity. For example we need to better explore cases where detection for the fast path might be costlier, for example a longer format string with or without some acceptable specifiers early, then something that forces us into the slow path only at the end.

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

PR Comment: https://git.openjdk.org/jdk/pull/19956#issuecomment-2199478317