On Thu, 14 Nov 2024 20:49:55 GMT, Anthony Scarpino <ascarp...@openjdk.org> wrote:
>> src/java.base/share/classes/com/sun/crypto/provider/CipherBlockChaining.java >> line 222: >> >>> 220: processed += >>> 221: implDecrypt(cipher, cipherOffset, cipherLen, plain, >>> plainOffset); >>> 222: return processed; >> >> The `for` loop logic is the same for encrypt and decrypt operations, only >> different positioning of the arguments. How about creating a helper method >> `chunkOperation` that would take one additional encrypt/decrypt boolean >> argument based on which it would do either encrypt or decrypt operation. > > Given this is a performance change, I'm fine with leaving it as is. Jumping > to a helper method with an encrypt/decrypt conditional check for every crypto > op will costs performance. This is a case where more efficient code is more > verbose syntax. > @ascarpino But a method call should be very cheap, we are adding a bunch of > extra implEncrypt/implDecrypt calls here already. Besides, compiler/hotspot > will optimize that call if needed. It will be just a method wrapping the > `for` loop. I have done a lot of performance testing with AES/GCM in similar situations by looking at the byte code generated, running JFR, and performance testing with hotspot debugging. You are right that the "method call should be very cheap", but I've seen code like you describe returning slower results. This loop is in the hot path for calling the intrinsic and not occasional used. I will never say absolutely one way or the other, but this looks like a situation where the more verbose syntax will be better. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22086#discussion_r1847158351
