On Mon, 25 Sep 2023 12:20:36 GMT, Shaojin Wen <d...@openjdk.org> wrote:
>>> The reason why I split it into multiple small methods is to avoid a single
>>> method codeSize > 325. After merging small methods, the performance will
>>> decrease.
>>
>> Yes, I can refactor to keep the same structure and verify performance is
>> neutral or better. I can pick that up as a low-priority follow-up if you
>> don't mind.
>
>> > The reason why I split it into multiple small methods is to avoid a single
>> > method codeSize > 325. After merging small methods, the performance will
>> > decrease.
>>
>> Yes, I can refactor to keep the same structure and verify performance is
>> neutral or better. I can pick that up as a low-priority follow-up if you
>> don't mind.
>
> Thank you very much for your very patient review. I will be happy to see you
> make improvements. I see no slowdown in JMH, but I see slowdowns when I run
> the loop directly in the code. I am also confused. It may be that codeSize >
> 325 cannot be inline, which will cause some scenes to be slower.
>
>
> import org.openjdk.jmh.infra.Blackhole;
>
> public class StringFormatBench {
> public static String s = "str";
> public static int i = 17;
> public static long l = 1234567L;
> public static float f = 123.37f;
>
> public void complexFormat(Blackhole bh) {
> bh.consume("%3s %10d %4S %04X %4S %04X %4S %04X".formatted(s, i, s,
> i, s, i, s, i));
> }
> }
>
> public class StringFormatBenchTest {
> public static void complexFormat() throws Throwable {
> for (int j = 0; j < 5; j++) {
> long start = System.currentTimeMillis();
> for (int i = 0; i < 10_000_000; ++i) {
> benchmark.complexFormat(BH);
> }
> long millis = System.currentTimeMillis() - start;
> System.out.println("StringFormatBench-complexFormat millis : " +
> millis);
> // zulu8.58.0.13 :
> // zulu11.52.13 :
> // zulu17.38.21 :
> // jdk22-ea : 4644
> // jdk22-baseline : 16040
> }
> }
>
> public static void main(String[] args) throws Throwable {
> complexFormat();
> }
> }
@wenshao Can we consider the code in `FormatSpecifierParser` stable enough for
a thorough review, or do you plan to make other changes there?
-------------
PR Comment: https://git.openjdk.org/jdk/pull/15776#issuecomment-1737325308
