RFR (M): 8143925: Enhancing CounterMode.crypt() for AES

[John Rose]

On Jan 4, 2016, at 3:42 AM, Paul Sandoz <paul.sandoz at oracle.com> wrote:
> 
> Hi,
> 
>> On 31 Dec 2015, at 22:33, John Rose <john.r.rose at oracle.com <mailto:john.r.rose at oracle.com>> wrote:
>> 
>> When performing explicit range checks in pre-intrinsic code,
>> let's try to use the new intrinsic functions in java.util.Objects,
>> called checkIndex, checkFromToIndex, and checkFromIndexSize.
> 
> At the moment only checkIndex is a C2 intrinsic, we could revisit making the others intrinsic as well based on use-cases.

Corrected, thanks.  They don't need to be intrinsics if they optimize well.
The point is that the library functions have code shapes which work well
with the JIT.  For example, the multi-index checks might (as in Kishor's code)
be implemented on top of the single-index check, without themselves being
intrinsics.

> 
>> These are simpler, safer, and more maintainable than our previous
>> practice of using hand-written "random logic", such as in this bug:
>>  http://hg.openjdk.java.net/jdk9/hs-comp/jdk/rev/cb31a76eecd1#l1.52 <http://hg.openjdk.java.net/jdk9/hs-comp/jdk/rev/cb31a76eecd1#l1.52>
>> 
> 
> Yes, in this case i believe the calls to cryptBlockCheck 
> 
>  176         cryptBlockCheck(in, inOff, len);
>  177         cryptBlockCheck(out, outOff, len);
>  178         return implCrypt(in, inOff, len, out, outOff);
> could be replaced with:
> 
>  Objects.checkFromIndexSize(inOff, len, in.length, <BiFunction>);
>  Objects.checkFromIndexSize(outOff, len, out.length, <BiFunction>);
>  return implCrypt(in, inOff, len, out, outOff);

Yes.  And if that doesn't produce clean code, it's a JIT bug, not a JDK bug.

One caveat:  If the BiFunction must produce a message with the index,
it is not a constant and we might have potential capture costs.
The correct trade-off here is to either simplify the message,
or ask the JIT to scalarize (EA-away) the closure node on the hot path,
or even add another entry point (with a TriFunction, perhaps).
As I pointed out below, we can just simplify the message.

— John

> Paul.
> 
> 
>> Depending on the documented API, it is usually enough that the
>> thrown exception be a RuntimeException of any sort.  By default,
>> the methods throw a generic IndexOutOfBoundsException.
>> In cases where a particular exception must be thrown, the Objects
>> methods provide an optional "hook" for building the desired exception.
>> 
>> In this case, since the code is already pushed, we should clean it
>> up as part of this bug:
>>  https://bugs.openjdk.java.net/browse/JDK-8135250 <https://bugs.openjdk.java.net/browse/JDK-8135250>
>> 
>> — John

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20160104/edaf7ed0/attachment.html>