RFR: 8371450: AES performance improvements for key schedule generation [v2]

Jamil Nimeh jnimeh at openjdk.org
Sat Nov 8 17:11:08 UTC 2025


On Fri, 7 Nov 2025 23:34:10 GMT, Shawn M Emery <duke at openjdk.org> wrote:

>> src/java.base/share/classes/com/sun/crypto/provider/AES_Crypt.java line 986:
>> 
>>> 984:             int idx = kLen - widx;
>>> 985: 
>>> 986:             dw[idx] = TMI0[w[widx] >>> 24] ^ TMI1[(w[widx] >> 16) & 0xFF]
>> 
>> Do you think there would be any benefit to putting w[widx] through w[widx+3] on local int variables?  In some cases I've seen where that increases register pressure and can lead to some perf benefits.  I'm not sure if this is one of those cases but it seems like you'd only need to reference memory once instead of 4 times per assignment.
>
> I believe my original changes here utilize a "MergeStore" technique that the compiler optimizes.  I've asked @minborg to see if I got this right.  To verify the optimization here, I used the separate local int variable technique and saw a 0.7% decrease in benchmark performance.

Well, your experiment answers my question.  Unless @minborg says otherwise I'm fine with what you have.  You're already seeing improvements so that's great.

>> src/java.base/share/classes/com/sun/crypto/provider/AES_Crypt.java line 1011:
>> 
>>> 1009:      */
>>> 1010:     private static int subWord(int word) {
>>> 1011:         byte b0 = (byte) (word >>> 24);
>> 
>> Nits (unrelated to this block of code, but I can't put comments on the lines directly since they're not part of the modified code):
>> 
>> - Lines 37-43: I think you could do an `@link` annotation with <a href></a> surrounding the links.
>> - Lines 1004-1005: Looks like a bit of comment rot, I think you just need an `@param` for `word`.
>
> Re: `@link` - I'm not for sure I understand the context and couldn't find an example of this in the existing code base.
> Re: `@param` - Good catch.  I've made this update and a couple of others that needed cleaning up.

When I was looking at the code in IntelliJ it was giving warnings about the links, but I think it just wanted them to be inside href HTML tags so the links would be active when rendering the comment block.  After looking at the `@link` tag in reference docs I think it's the wrong tag.  Honestly, let's just forget this - if it was a public-facing javadoc comment it would be more important but what you have here is fine.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28188#discussion_r2507020546
PR Review Comment: https://git.openjdk.org/jdk/pull/28188#discussion_r2507019920


More information about the security-dev mailing list