RFE: Refactor java.util.Optional and add NonNull checks
Quân Anh Mai
anhmdq at gmail.com
Mon Aug 28 11:03:24 UTC 2023
Tbh I have a hard time understand what you are trying to convey. Please
write grammatically correct sentences with clear structure.
I notice you attach benchmark results but I don't see what was benchmarked
to produce those results.
Another point is that Value classes has not been delivered yet, so there
are potential performance uplifts of Optional in the future. This may not
be possible if Optional is polymorphic.
Thanks
On Mon, 28 Aug 2023 at 18:27, Oleksii Kucheruk <iselo+openjdk at raccoons.co>
wrote:
> Thank you Quân.
>
> All you saying guys make sense.
> Yes there is a difference calling methods by pointer and involving vtable.
> But one thing didn't came out of my head:
> If virtual dispatch dispatch is 10-time more expensive and polymorphism is
> a performance killer so how combinations of "return optional - isPresent-
> get" or "return empty - isEmpty" with virtual dispatch and memory
> allocation for every empty or sole but polymorph empty could perform
> equally or even beat the near zero-cost abstraction of Value classes
> according to JMH?
>
> Maybe private final static nested EmptyOptional invokevirtual bytecode refers
> a final methods and it need not have a vtable slot allocated. This means
> that, after linking, an invokevirtual bytecode might in fact collapse into
> the equivalent of an invokestatic bytecode.
> The question of full dynamic with anonymous overloading is still unclear
> to me.
>
> Benchmark Mode Cnt
> Score Error Units
>
> *OptionalComboIsPresentGet.opDynamicAnonymous thrpt 30
> 1059868995.937 ± 51273067.116 ops/s*
>
> OptionalComboIsPresentGet.opJdk thrpt 30 1017505800.567
> ± 16007847.872 ops/s
>
> OptionalComboIsPresentGet.opNestedStatic thrpt 30 951493332.957
> ± 12167812.398 ops/s
>
>
> OptionalComboOfIsPresentGetTest.opDynamicAnonymous thrpt 30 1426348334.485
> ± 20889455.433 ops/s
>
> *OptionalComboOfIsPresentGetTest.opJdk thrpt 30
> 1435570789.822 ± 16587538.659 ops/s*
>
> OptionalComboOfIsPresentGetTest.opNestedStatic thrpt 30 1418853644.215
> ± 27755398.645 ops/s
>
>
> *OptionalComboOfNullableIsEmptyTest.opDynamicAnonymous thrpt 30
> 1434399595.592 ± 14749443.903 ops/s*
>
> OptionalComboOfNullableIsEmptyTest.opJdk thrpt 30 1255375023.531
> ± 107403119.573 ops/s
>
> OptionalComboOfNullableIsEmptyTest.opNestedStatic thrpt 30 1415705702.964
> ± 26328318.672 ops/s
>
>
> OptionalEmptyTest.opDynamicAnonymous thrpt 30 1417895521.763
> ± 26311640.557 ops/s
>
> *OptionalEmptyTest.opJdk thrpt 30
> 1423940087.962 ± 18515297.860 ops/s*
>
> OptionalEmptyTest.opNestedStatic thrpt 30 1424779162.084
> ± 20288793.337 ops/s
>
>
> OptionalGetTest.emDynamicAnonymous thrpt 30 569132.235
> ± 11089.903 ops/s
>
> *OptionalGetTest.emJdk thrpt 30
> 549764.038 ± 14768.566 ops/s*
>
> OptionalGetTest.emNestedStatic thrpt 30 568308.722
> ± 6075.671 ops/s
>
>
> OptionalGetTest.opDynamicAnonymous thrpt 30 953788065.414
> ± 7043419.784 ops/s
>
> *OptionalGetTest.opJdk thrpt 30
> 1015679232.049 ± 11726562.032 ops/s*
>
> OptionalGetTest.opNestedStatic thrpt 30 944209513.151
> ± 17718342.519 ops/s
>
>
> OptionalIsPresentTest.emDynamicAnonymous thrpt 30 869086292.481
> ± 11668636.886 ops/s
>
> *OptionalIsPresentTest.emJdk thrpt 30
> 977042244.447 ± 44558020.220 ops/s*
>
> OptionalIsPresentTest.emNestedStatic thrpt 30 953424664.631
> ± 8448564.963 ops/s
>
>
> OptionalIsPresentTest.opDynamicAnonymous thrpt 30 953285872.799
> ± 6562894.941 ops/s
>
> *OptionalIsPresentTest.opJdk thrpt 30
> 1006926903.885 ± 18055487.689 ops/s*
>
> OptionalIsPresentTest.opNestedStatic thrpt 30 954121471.431
> ± 6829528.708 ops/s
>
>
> OptionalOfNullableTest.emDynamicAnonymous thrpt 30 1422939780.266
> ± 21693247.354 ops/s
>
> *OptionalOfNullableTest.emJdk thrpt 30
> 1423340184.237 ± 21296444.017 ops/s*
>
> OptionalOfNullableTest.emNestedStatic thrpt 30 1414965236.385
> ± 26021734.344 ops/s
>
>
> OptionalOfNullableTest.opDynamicAnonymous thrpt 30 1412594454.538
> ± 28123612.298 ops/s
>
> *OptionalOfNullableTest.opJdk thrpt 30
> 1427924599.589 ± 23469517.835 ops/s*
>
> OptionalOfNullableTest.opNestedStatic thrpt 30 1420053175.637
> ± 21170929.571 ops/s
>
>
> OptionalOfTest.emDynamicAnonymous thrpt 30 549907.559
> ± 14560.954 ops/s
>
> OptionalOfTest.emJdk thrpt 30 557858.903
> ± 2851.076 ops/s
>
> *OptionalOfTest.emNestedStatic thrpt 30
> 548655.854 ± 10875.674 ops/s*
>
>
> OptionalOfTest.opDynamicAnonymous thrpt 30 1411307270.435
> ± 19832207.408 ops/s
>
> OptionalOfTest.opJdk thrpt 30 1400651442.726
> ± 23641065.234 ops/s
>
> *OptionalOfTest.opNestedStatic thrpt 30
> 1433953744.298 ± 21414147.936 ops/s*
>
>
> On Mon, Aug 28, 2023 at 11:13 AM Quân Anh Mai <anhmdq at gmail.com> wrote:
>
>> Hi,
>>
>> Polymorphism is a performance killer. Normally this does not matter much,
>> but for thin wrappers such as Optional, this is one of the most important
>> factor.
>>
>> Behaviour polymorphism requires virtual dispatch and prevents inlining.
>> This is really detrimental, as simple operations such as a getter which is
>> previously only consisted of a memory load, in the presence of polymorphism
>> would have to go through a 10-time more expensive virtual dispatch.
>> Function calls, especially virtual ones, are also opaque and prohibit
>> compiler optimisations.
>>
>> Layout polymorphism prevents direct accesses and requires indirection.
>> This means that for every instance of Optional created a memory allocation
>> is required. Optional is expected to be a near zero-cost abstraction in the
>> presence of Value classes, so making it polymorphic is unacceptable.
>>
>> Thanks.
>>
>> On Wed, 23 Aug 2023 at 22:43, Oleksii Kucheruk <iselo+openjdk at raccoons.co>
>> wrote:
>>
>>> Hi there.
>>> I have found that `java.util.Optional` is written procedural style and
>>> has `ifnonnull` checks in each method. I propose to refactor `Optional` in
>>> accordance to OOP-style. This will eliminates all unnecessary
>>> `if`-statements, removes duplications and reduces bytecode size more then
>>> twice.
>>>
>>> I have two solutions:
>>> 1. Completely dynamic that avoids single static `EMPTY` instance and
>>> unchecked casting of each `Optional.empty()`
>>> 2. Preserving original single static `EMPTY` per VM.
>>>
>>> Also there are couple methods that throws NPE due to calling methods on
>>> null-objects and requires to add `Objects.requireNonNull(...)`.
>>>
>>> OptionalInt, OptionalDouble, OptionalLong could be refactored same way
>>> even with remove of additional boolean variable `isPresent`.
>>>
>>> Since I'm new here any guidance will be helpful.
>>> Thank you in advance.
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/core-libs-dev/attachments/20230828/79daa4d5/attachment-0001.htm>
More information about the core-libs-dev
mailing list