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