On Wed, 2 Jun 2021 01:53:51 GMT, Anthony Scarpino <ascarp...@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

Reply via email to