RFR: 8209862:CipherCore performance improvement

[Adam Petcher]

Looks like a nice improvement, but is it possible to do this without 
duplicating code? For example, code almost identical to this also 
appears starting on line 860:

971 } else { // encrypting
972 try {
973 outLen = finalNoPadding(finalBuf, finalOffset, output,
974 outputOffset, finalBufLen);
975 } finally {
976 // reset after doFinal() for GCM encryption
977 requireReinit = (cipherMode == GCM_MODE);
978 if (finalBuf != input) {
979 // done with internal finalBuf array. Copied to output
980 Arrays.fill(finalBuf, (byte) 0x00);
981 }
982 }
983 }

It may be possible to factor out the entire if/else statement and some 
of the code around it into a separate method and do the short buffer 
check and save/restore around it. Ideally, each doFinal method would 
have only a small amount of code to either allocate the array or check 
array lengths, and then they would call the same private method to do 
most of the work.

I would suggest a noreg-sqe label on the ticket to document that 
existing regression tests are used to ensure correctness of the modified 
code.

Minor code style comments: check for long lines (e.g. line 856) and 
missing spaces after commas in actual parameter lists (also e.g. line 
856). Also, line 1076 is missing space around the '?' and ':' 
characters. I can check the code again and give you the complete list 
after we sort out the code duplication.


On 10/1/2018 9:11 AM, Seán Coffey wrote:
> JDK-8207775 introduced some performance regressions in the ciphercore 
> area. Sergey Kuksenko has been looking at this and has contributed the 
> following patch:
>
> http://cr.openjdk.java.net/~skuksenko/crypto/8209862/
> bug report : https://bugs.openjdk.java.net/browse/JDK-8209862
>
> I've been reviewing it and ran functionality and TCK testing. Didn't 
> see any issues. Sergey has also confirmed that the patch helps to 
> alleviate the performance issues introduced. More details regards 
> approach for fix are in the bug description.
>
> Thanks Sergey! I'm looking for another review from security team.
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20181001/ec899b44/attachment.htm>