RFR 8146458 Improve exception reporting for Objects.checkIndex/checkFromToIndex/checkFromIndexSize
paul.sandoz at oracle.com
Mon Jan 18 11:22:50 UTC 2016
Thanks for the feedback. Updated:
- refer to “exception factory function”
- renamed rangeCheckExceptionMapper to outOfBoundsExceptionFactory. Tweaked the documentation and added an api note for clarity.
- tests for String/ArrayIOOBE, and uniform checking of expected exception message in all cases
> On 16 Jan 2016, at 08:36, John Rose <john.r.rose at oracle.com> wrote:
> On Jan 15, 2016, at 2:43 PM, Stuart Marks <stuart.marks at oracle.com> wrote:
>> I think, in order to avoid introducing new functional interfaces, this information is funneled into a single varargs call, which has to use Integer boxes, because it then wraps its args into a List (thanks for using the new List factory!), which then has to unpack the list and check its length against the number of arguments for the particular message kind.
> And that's before String.format does a bunch more steps of the same kind, including boxing and varargs. And then the exception creation itself walks the entire thread stack. Micro-optimization is hopeless.
> But it doesn't matter because it's all on the slow path.
Yes. FWIW the implementation also avoids any varargs call for the main check methods to keep the method size small (we could use @ForceInline instead, but i would prefer to use that for cases where it is really needed).
>> I understand that this is the "slow" exception-throwing path, but my sense is still that this ought to be made simpler. Perhaps the scope of what it's trying to accomplished should be reduced, if that helps things.
> Simpler would be good but I don't see it any simpler, yet.
Me neither, any simpler and the generality is lost. Arguably there is actually quite a simple rule underlying this: the arguments to the factory function take on a form that is very similar to those required for a reflective look up and invocation:
Objects.class.getMethod(checkKind, List.nCopies(args.size(), int.class).toArray()).
> Do give it some thought!
More information about the core-libs-dev