On Wed, 24 May 2023 02:29:09 GMT, Chen Liang <li...@openjdk.org> wrote:
>> This patch moves the parameters to an immutable list, to avoid allocations >> on `parameterList` as well. In addition, it fixes 8304932, the bug where if >> a caller keeps a reference to the array passed into `MethodTypeDesc.of`, the >> caller may mutate the Desc via the array and can create invalid >> MethodTypeDesc. >> >> The latest benchmark results are available here: >> https://github.com/openjdk/jdk/pull/13186#issuecomment-1560378822 >> >> This patch has minor performance gains on `ofDescriptor` factory, even >> compared to Adam's patch that optimized `ofDescriptor` in #12945. >> >> Benchmark of Oracle JDK 20: >> https://gist.github.com/683c6219e183cbc2b336224fc2c0d50a >> Benchmark of this patch: >> https://gist.github.com/22be9371a2370fb4a7b44f1684750ec4 >> Benchmark of [asotona's >> patch](https://github.com/openjdk/jdk/pull/12945/files#diff-ac8e413d3e13532a2b0d34a90253c6ddd7a4f04082f792b9d076e9b5a33f2078): >> https://gist.github.com/eb98579c3b51cafae481049a95a78f80 >> >> [sotona vs >> this](https://jmh.morethan.io/?gists=eb98579c3b51cafae481049a95a78f80,22be9371a2370fb4a7b44f1684750ec4); >> [20 vs >> this](https://jmh.morethan.io/?gists=683c6219e183cbc2b336224fc2c0d50a,22be9371a2370fb4a7b44f1684750ec4); >> [20 vs >> sotona](https://jmh.morethan.io/?gists=683c6219e183cbc2b336224fc2c0d50a,eb98579c3b51cafae481049a95a78f80), >> for reference > > Chen Liang has updated the pull request incrementally with one additional > commit since the last revision: > > reuse immutable list to avoid array allocation The benefits of a list implementation over an array implementation are: 1. `parameterList()` is much faster, allowing more beautiful user code compared to manually unpacked iteration (immutable list factory doesn't allow using non-Object backing arrays) 2. `of(ClassDesc, List)` allows users to share immutable parameter lists than having to copy > With https://github.com/openjdk/jdk/pull/13671, MethodTypeDesc is cached and > I wonder if this is no longer the bottleneck of ClassFile API performance. The parsing is no more, but `descriptorString` is still a bottleneck. The master in the last benchmark already had 8306842 integrated. In fact, I don't think argType array sharing goes far enough: in these modification methods and ofDescriptor, we can skip the array/list iteration validation if we can ensure the change parts alone have not produced an invalid method type (insertion of void, slot changes). I would postpone such in-depth sharing into a later issue, where we track the slot count information as well (to preemptively reject invalid resolveConstantDesc calls) > Converting `argTypes` to an immutable list seems to be an extra step to make > it immutable - we need frozen arrays!! Unfortunately, this is currently the only way general users have access to frozen arrays, where index-based access may be constant-folded, as `@Stable` is not generally available. ------------- PR Comment: https://git.openjdk.org/jdk/pull/13186#issuecomment-1565013833
