On Thu, 25 Apr 2024 13:34:50 GMT, Claes Redestad <redes...@openjdk.org> wrote:
>> When analyzing (startup) performance of the Classfile API I found this >> opportunity to further improve `MethodTypeDescImpl::descriptorString`. >> >> Performance improves across the board in existing microbenchmarks: >> >> Name >> (descString) Cnt Base Error Test Error Unit Change >> MethodTypeDescFactories.descriptorString >> (Ljava/lang/Object;Ljava/lang/String;)I 6 55,179 ± 2,027 32,920 ± 1,189 >> ns/op 1,68x (p = 0,000*) >> MethodTypeDescFactories.descriptorString >> ()V 6 17,689 ± 1,871 11,060 ± 0,331 ns/op 1,60x (p = 0,000*) >> MethodTypeDescFactories.descriptorString >> ([IJLjava/lang/String;Z)Ljava/util/List; 6 86,627 ± 1,646 41,035 ± 0,636 >> ns/op 2,11x (p = 0,000*) >> MethodTypeDescFactories.descriptorString >> ()[Ljava/lang/String; 6 18,305 ± 1,974 13,110 ± 0,089 ns/op 1,40x (p = >> 0,000*) >> * = significant >> >> >> The improvement is even more pronounced when running with `-Xint`, which is >> relevant for reducing startup overheads of early ClassFile API use: >> >> Name >> (descString) Cnt Base Error Test Error Unit Change >> MethodTypeDescFactories.descriptorString >> (Ljava/lang/Object;Ljava/lang/String;)I 6 5122,061 ± 81,335 2626,481 ± >> 101,466 ns/op 1,95x (p = 0,000*) >> MethodTypeDescFactories.descriptorString >> ()V 6 3481,316 ± 258,904 1489,267 ± 15,506 ns/op 2,34x (p = 0,000*) >> MethodTypeDescFactories.descriptorString >> ([IJLjava/lang/String;Z)Ljava/util/List; 6 7741,081 ± 1628,244 3281,778 ± >> 41,892 ns/op 2,36x (p = 0,000*) >> MethodTypeDescFactories.descriptorString >> ()[Ljava/lang/String; 6 3677,803 ± 63,432 1495,291 ± 8,995 ns/op >> 2,46x (p = 0,000*) >> * = significant >> ``` >> >> I also applied similar approach to `MethodTypeDesc::displayDescriptor`: >> while not performance sensitive I think these are so inter-related that it >> makes sense to implement them in a similar fashion. > > Claes Redestad has updated the pull request incrementally with one additional > commit since the last revision: > > comma-separated Correction: I ran the wrong benchmark for that long descriptor test! 🤦🏽 On the `descriptorString` benchmark there _is_ a regression with my patch on longer descriptors, -30% on that example. Since the strings are all readily available pre-calculating the size is cheap enough to make sense. This makes the patch more or less neutral on large descriptors, extends the gain on few-arg methods, but reduces the win on cases where there are no args (due an extra branch et.c.). On balance this seems like an improvement: Name (descString) Cnt Base Error Test Error Unit Change MethodTypeDescFactories.descriptorString (Ljava/lang/Object;Ljava/lang/String;)I 6 43,180 ± 0,575 30,547 ± 0,387 ns/op 1,41x (p = 0,000*) MethodTypeDescFactories.descriptorString ()V 6 20,717 ± 0,128 17,233 ± 0,242 ns/op 1,20x (p = 0,000*) MethodTypeDescFactories.descriptorString ([IJLjava/lang/String;Z)Ljava/util/List; 6 89,834 ± 1,951 40,149 ± 0,326 ns/op 2,24x (p = 0,000*) MethodTypeDescFactories.descriptorString ()[Ljava/lang/String; 6 21,438 ± 0,490 15,937 ± 0,263 ns/op 1,35x (p = 0,000*) MethodTypeDescFactories.descriptorString (..IIJ)V 6 52,429 ± 0,117 49,396 ± 0,153 ns/op 1,06x (p = 0,000*) MethodTypeDescFactories.descriptorString (.....................). 6 178,205 ± 0,518 176,856 ± 5,577 ns/op 1,01x (p = 0,158 ) * = significant `.` is illegal in descriptor strings I used this as wildcard to be replaced with the descriptor string for the benchmark class as a ways to produce very large descriptors. Since this strays a bit from what's doable in `MethodTypeDesc::displayDescriptor` I'm reverting those changes and focus this PR on the relevant piece of machinery. ------------- PR Comment: https://git.openjdk.org/jdk/pull/18945#issuecomment-2078267325
