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.

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.

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