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...@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...@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

Reply via email to