RFR (M): 8143925: Enhancing CounterMode.crypt() for AES
Paul Sandoz
paul.sandoz at oracle.com
Mon Jan 4 11:42:15 UTC 2016
Hi,
> On 31 Dec 2015, at 22:33, John Rose <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.
> 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
>
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);
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
>
> — John
>
> On Dec 29, 2015, at 9:33 AM, Kharbas, Kishor <kishor.kharbas at intel.com> wrote:
>>
>> That's great.. Thank you!
>>
>> I will keep the jcheck tip in mind for next time :)
>>
>> - Kishor
>>
>> -----Original Message-----
>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>> Sent: Tuesday, December 29, 2015 12:47 AM
>> To: Kharbas, Kishor; hotspot-compiler-dev at openjdk.java.net
>> Cc: Anthony Scarpino
>> Subject: Re: RFR (M): 8143925: Enhancing CounterMode.crypt() for AES
>>
>> Hi Kishor,
>>
>> There were coding style problems which I fixed. Please, do cleanup in a future (use jcheck).
>>
>> src/cpu/x86/vm/stubGenerator_x86_32.cpp:2144: Trailing whitespace
>> src/cpu/x86/vm/stubGenerator_x86_64.cpp:3061: Trailing whitespace
>> src/cpu/x86/vm/stubRoutines_x86.hpp:36: Trailing whitespace
>> src/cpu/x86/vm/vm_version_x86.cpp:709: Trailing whitespace
>> src/share/vm/opto/library_call.cpp:702: Trailing whitespace
>> src/share/vm/opto/runtime.hpp:317: Trailing whitespace
>>
>> src/share/vm/opto/library_call.cpp:5789: Carriage return (^M)
>>
>> I submitted push job. Lets see how it will go.
>>
>> Regards,
>> Vladimir
>>
>> On 12/28/15 8:48 PM, Kharbas, Kishor wrote:
>>> Vladimir, sorry that file was added accidentally.
>>> Here is an updated patch -
>>> http://cr.openjdk.java.net/~vdeshpande/8143925/webrev.01/
>>>
>>> This patch includes,
>>> 1. Changes to some comments.
>>> 2. Small correction in vm_version_x86.cpp.
>>> 3. Removal of version.rc file.
>>>
>>> Thanks for reviewing the code.
>>>
>>> Kishor
>>>
>>> -----Original Message-----
>>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>>> Sent: Thursday, December 24, 2015 4:36 PM
>>> To: Kharbas, Kishor; hotspot-compiler-dev at openjdk.java.net
>>> Cc: Anthony Scarpino
>>> Subject: Re: RFR (M): 8143925: Enhancing CounterMode.crypt() for AES
>>>
>>> What are the changes in src/os/windows/vm/version.rc?
>>>
>>> Otherwise this looks good.
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 12/24/15 2:26 PM, Kharbas, Kishor wrote:
>>>> Hello all,
>>>>
>>>> Thank you Vladimir and Anthony for your inputs so far.
>>>> I have updated the hotspot based on the suggestions and also added CTR mode to jtreg test.
>>>>
>>>> During testing I also noticed that the Java code for CounterMode.crypt() uses the partially used encrypted counter from previous invocation and also saves the last encryptedCounter for next invocation.
>>>> This case was not handled by the intrinsic. I have fixed this in the latest patch.
>>>>
>>>> Summary of changes:
>>>> 1. Proper disabling of UseAESCTRIntrinsic flag based on hardware
>>>> support 2. Adding the missing support explained above.
>>>> 3. Added CTR mode in jtreg test 7184394 4. Added and changed some
>>>> encodings (pextr and pinsr) in assembler_x86.cpp
>>>>
>>>> The updated hotspot webrev is at :
>>>> http://cr.openjdk.java.net/~vdeshpande/8143925/webrev.00/
>>>> There is no update to jdk webrev posted earlier which is
>>>> http://cr.openjdk.java.net/~mcberg/8143925/jdk/webrev.02/
>>>> Bug id : https://bugs.openjdk.java.net/browse/JDK-8143925
>>>>
>>>> Much appreciated!
>>>>
>>>> Happy holidays!
>>>> Kishor
>>>>
>>>>
>>>> -----Original Message-----
>>>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>>>> Sent: Friday, December 04, 2015 3:59 PM
>>>> To: Kharbas, Kishor; hotspot-compiler-dev at openjdk.java.net
>>>> Cc: Anthony Scarpino
>>>> Subject: Re: RFR (M): 8143925: Enhancing CounterMode.crypt() for AES
>>>>
>>>> jdk: http://cr.openjdk.java.net/~mcberg/8143925/jdk/webrev.02/
>>>>
>>>> JDK changes looks good to me.
>>>>
>>>> hotspot:
>>>> http://cr.openjdk.java.net/~mcberg/8143925/hotspot/webrev.04/
>>>>
>>>> Please, set flag to 'false' on platforms which does not support this
>>>> intrinsic:
>>>>
>>>> if (UseAESCTRIntrinsics) {
>>>> warning("AES/CTR intrinsics are not available on this CPU");
>>>> FLAG_SET_DEFAULT(UseAESCTRIntrinsics, false);
>>>> }
>>>>
>>>> Also Anthony asked to add test for this intrinsic. Please do it:
>>>>
>>>> "2) It would be good to add CTR to the TestAES tests. It's in hotspot/test/compiler/codegen/7184394/. The test currently has CBC, ECB, and GCM in it, so it should be easy. It's also the only test I know of that tests the intrinsic. None of the tests in the jdk repo that I know of loop enough to trigger the intrinsic."
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 12/4/15 1:40 PM, Kharbas, Kishor wrote:
>>>>> Thanks Vladimir for the feedback!
>>>>>
>>>>> I have updated the jbs entry with the new patch.
>>>>>
>>>>> JDK changes : added range checks in the JDK using additional methods.
>>>>> Hotspot changes : renamed the UseCTRAESIntrinsics flag to
>>>>> UseAESCTRIntrinsics
>>>>>
>>>>> Further review and feedback is appreciated!
>>>>>
>>>>> - Kishor
>>>>>
>>>>> -----Original Message-----
>>>>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>>>>> Sent: Tuesday, December 01, 2015 5:32 PM
>>>>> To: Kharbas, Kishor; hotspot-compiler-dev at openjdk.java.net
>>>>> Subject: Re: RFR (M): 8143925: Enhancing CounterMode.crypt() for AES
>>>>>
>>>>> Hotspot changes seems fine. But JDK changes should have additional method for range checks - this is new requirement for intrinsics which access arrays. See, for example, cryptBlockCheck() in AESCrypt.java.
>>>>>
>>>>> Thanks,
>>>>> Vladimir
>>>>>
>>>>> On 11/24/15 2:33 PM, Kharbas, Kishor wrote:
>>>>>> Hello all,
>>>>>>
>>>>>> I request the community to review a patch for enhancing
>>>>>> CounterMode.crypt() for AES. This patch defines intrinsic for
>>>>>> CounterMode.crypt() to leverage the parallel nature of AES in
>>>>>> Counter
>>>>>> (CTR) Mode.
>>>>>>
>>>>>> This is achieved by operating on 6 blocks in parallel to issue
>>>>>> independent x86 AES-NI instructions and keep the CPU pipeline full.
>>>>>>
>>>>>> Testing on micro-benchmark has shown a speedup of 4x-6x.
>>>>>>
>>>>>> Bug id:
>>>>>>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8143925
>>>>>>
>>>>>> Webrev:
>>>>>>
>>>>>> hotspot:
>>>>>> http://cr.openjdk.java.net/~mcberg/8143925/hotspot/webrev.02/
>>>>>>
>>>>>> jdk:
>>>>>> http://cr.openjdk.java.net/~mcberg/8143925/jdk/webrev.01/
>>>>>>
>>>>>> Much appreciated!
>>>>>>
>>>>>> Kishor Kharbas
>>>>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20160104/ac8bee0c/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 841 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20160104/ac8bee0c/signature-0001.asc>
More information about the hotspot-compiler-dev
mailing list