RFR: 8306889: Convert MethodTypeDescImpl parameter storage from array to immutable list [v4]
Chen Liang
liach at openjdk.org
Fri May 26 22:13:55 UTC 2023
On Wed, 24 May 2023 02:29:09 GMT, Chen Liang <liach at 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
More information about the core-libs-dev
mailing list