JDK 9 RFR of JDK-8146668: Replace custom check/range functionality with check index/range methods in java.util.Objects

[Amy Lu]

No, no change to ArrayList, sorry for the confusion. I just thought 
ArrayList/RangeCheckMicroBenchmark.java can also give some data for 
reference to the Arrays change.

So I tried a modified ArraysEquals JMH test, the test is originally for
equals(char[] a, char[] a2) and I changed it to perform
equals(char[] a, int aFromIndex, int aToIndex,
        char[] b, int bFromIndex, int bToIndex)
which is changed in this patch.

No performance regression.

without 8146668

Benchmark                                              Mode Cnt          
Score         Error  Units
ArraysEqualsFromTo.testCharArrayEqualsFalseBeginning  thrpt 200  
117773032.914 ?  837866.545  ops/s
ArraysEqualsFromTo.testCharArrayEqualsFalseEnd        thrpt 200   
68162413.018 ?  252693.599  ops/s
ArraysEqualsFromTo.testCharArrayEqualsFalseMid        thrpt 200   
87845305.867 ?  434145.358  ops/s
ArraysEqualsFromTo.testCharArrayEqualsTrue            thrpt 200   
57665790.396 ? 1144777.344  ops/s

With 8146668

Benchmark                                              Mode Cnt          
Score         Error  Units
ArraysEqualsFromTo.testCharArrayEqualsFalseBeginning  thrpt 200  
118754451.736 ?  395812.217  ops/s
ArraysEqualsFromTo.testCharArrayEqualsFalseEnd        thrpt 200   
68263418.139 ?  161477.719  ops/s
ArraysEqualsFromTo.testCharArrayEqualsFalseMid        thrpt 200   
87429210.156 ?  483026.299  ops/s
ArraysEqualsFromTo.testCharArrayEqualsTrue            thrpt 200   
57694918.966 ? 1163825.417  ops/s


Thanks,
Amy

On 1/18/17 9:36 AM, Paul Sandoz wrote:
> Hi Amy,
>
> RangeCheckMicroBenchmark.java measures access for ArrayList. Did you update ArrayList? (i don’t see it in your current patch).
>
> Paul.
>
>> On 16 Jan 2017, at 01:41, Amy Lu <amy.lu at oracle.com> wrote:
>>
>> Thank you Paul and Martin for your review.
>>
>> On 1/13/17 12:52 AM, Paul Sandoz wrote:
>>> HI Amy,
>>>
>>> Overall this looks very good, well done.
>>>
>>>
>>> At this point we are down to two things:
>>>
>>> 1) should we preserve exception messages?
>>>
>>> 2) due diligence on the performance.
>>>
>>>
>>> On 1) my preference is that uniform (and informative) messages are better for IndexOutOfBounds and subtypes, and defining that in addition to the checks in one place is very valuable. In some sense that does change some behavioural compatibility and i think because of that pushing in 10 (when the repos) open with a CCC would be more preferable.
>> Got it. Will wait for 10.
>>> On 2) this is a valuable exercise to perform (either using an existing test and/or writing JMH benchmarks). I don’t expect any major problems. Care was taken to ensure the uncommon exception processing path will optimize away for the common case. The BiFunction parameter passed as the last argument is always a constant, so should fold away from the hot path.
>> Tested with java/util/ArrayList/RangeCheckMicroBenchmark.java and java/nio/Buffer/SwapMicroBenchmark.java, no performance regression observed.
>>
>> java/util/ArrayList/RangeCheckMicroBenchmark.java
>>
>> without 8146668
>> --------------------
>> get                             26          1
>> set                             40.6        1.5707
>> get/set                         85.7        3.3002
>> add/remove at end               337.5       12.9197
>> subList get                     25          0.9747
>> subList set                     59.9        2.3
>> subList get/set                 92.2        3.5414
>> subList add/remove at end       429.8       16.45
>>
>> with 8146668
>> --------------------
>> get                             26          1
>> set                             39.1        1.5188
>> get/set                         86.8        3.3363
>> add/remove at end               335.5       12.8549
>> subList get                     24.8        0.9715
>> subList set                     59.2        2.2728
>> subList get/set                 92.2        3.5473
>> subList add/remove at end       427.7       16.3789
>>
>> java/nio/Buffer/SwapMicroBenchmark.java
>>
>> without 8146668
>> --------------------
>> swap char LITTLE_ENDIAN         11          1
>> swap short LITTLE_ENDIAN        10.2        0.9895
>> swap int LITTLE_ENDIAN          5           0.4961
>> swap long LITTLE_ENDIAN         3           0.3345
>>
>> with 8146668
>> --------------------
>> swap char LITTLE_ENDIAN         10.1        1
>> swap short LITTLE_ENDIAN        10.1        0.9995
>> swap int LITTLE_ENDIAN          5           0.5005
>> swap long LITTLE_ENDIAN         3           0.3327
>>
>> Thanks,
>> Amy
>>> Paul.
>>>
>>>
>>>> On 11 Jan 2017, at 18:10, Amy Lu <amy.lu at oracle.com> wrote:
>>>>
>>>> 8135248 and 8155794 introduced utility methods for checking indexes and ranges. Existing code with custom checkIndex/checkRange can be updated to use these methods. Please review the patch for this purpose:
>>>>
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8146668
>>>> webrev: http://cr.openjdk.java.net/~amlu/8146668/webrev.01
>>>>
>>>> The type of exception thrown are preserved. Custom checkIndex/checkRange functions that throw IOOBE are now using ‘check’ utility methods provided by java.lang.Objects (which also throws IOOBE), functions that throw other exceptions use jdk.internal.util.Preconditions to preserve exception types, with the help of new BiFunction vars.
>>>>
>>>> I'd like to get this in JDK 9 if it's not too late, otherwise, JDK 10.
>>>>
>>>> Thanks,
>>>> Amy