On 04/10/2017 10:27 AM, Paul Sandoz wrote:
On 7 Apr 2017, at 12:47, Anthony Scarpino <anthony.scarp...@oracle.com> wrote:
On 04/07/2017 06:58 AM, Chris Hegarty wrote:
On 06/04/17 21:39, Anthony Scarpino wrote:
I'd like to get a review for this performance change to use the existing
CounterMode parallelized intrinsic instead of GCTR's own version. The
two classes were nearly identical except for the doFinal() method which
doesn't belong in CounterMode.java.
I could have been more aggressive with this change, but I'm looking to
get this into 9, so I stayed away from completely merging GCTR into
CounterMode in case of incompatibilities. All tests security and
hotspot tests pass.
http://cr.openjdk.java.net/~ascarpino/8177784/webrev/
This change looks good to me. Trivially, the class-level comment in
GCTR should be updated ( it refers to removed fields ). Also,
CounterMode.counter could be protected ( rather than package-private ).
-Chris.
Thanks Chris,
I left CounterMode.counter as package-private because the package is what
becomes the SunJCE provider. I don't believe there should be any outside
package classes accessing this code.
I updated the webrev at with the comment update:
http://cr.openjdk.java.net/~ascarpino/8177784/webrev.01/
It’s the same pattern for “iv” with CounterMode and FeedbackCipher, although i
prefer protected as well if never accessed by other classes in the same
package, it makes it easier to reason about the code, since i know more about
what can access it.
Suggestion, take it or leave it: personally i would prefer to see an additional
constructor in CounterMode:
GCTR(SymmetricCipher cipher, byte[] initialCounterBlk) {
super(cipher, checkBlockSize(initialCounterBlk));
}
Where new CounterMode constructor performs the clone, assignmentm and reset,
the static method checkBlockSize checks the array length is equal to
AES_BLOCK_SIZE Thereby you have a cleaner separation of concerns.
Paul.
I contemplated putting changing the constructors, but went against it
for the moment so a jdk9 change would be as minimal as possible. A
future change would most likely include the new constructor in CounterMode.
Tony
