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
