RFR: 8255557: Decouple GCM from CipherCore [v4]

[Valerie Peng]

On Wed, 2 Jun 2021 01:53:51 GMT, Anthony Scarpino <ascarpino at openjdk.org> wrote:

>> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java line 741:
>> 
>>> 739:                         } else {
>>> 740:                             // If the remaining in buffer + data does not fill a
>>> 741:                             // block, complete the gctr operation
>> 
>> This comment seems more suited for the else block below at line 753. In addition, the criteria for completing the gctr operation should be whether src still has remaining bytes left. It's possible that the (src.remaining() == blockSize - over) and happen to fill up the block, right? The current flow for this particular case would be an op.update(...) then continue down to line 770-778, maybe you can adjust the if-check on line748 so this would go through line 754-759 and return, i.e. the else block?
>
> I changed the comment, but I also changed the code which will be in the next update

Ok, will wait for the update then.

>> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java line 752:
>> 
>>> 750:                             if (dst != null) {
>>> 751:                                 dst.put(block, 0, blockSize);
>>> 752:                             }
>> 
>> not counting this blockSize value into "processed"?
>
> code is now changed

Ok, also in the next update?

>> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java line 805:
>> 
>>> 803:         /**
>>> 804:          * This segments large data into smaller chunks so hotspot will start
>>> 805:          * using CTR and GHASH intrinsics sooner.  This is a problem for app
>> 
>> nit: typo: CTR->GCTR? This is to improve performance rather than a problem, no? Same comment applies to the other throttleData() method above.
>
> Yes, this is for apps or perf tests that send large input data and cause the hotspot compiler to be slower to start using the intrinsics.. This is just a new version of what was in the old code in doLastBlock() around line 400.

Sure.

-------------

PR: https://git.openjdk.java.net/jdk/pull/4072