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 >
