RFR: 8314199: Initial size PBEKeyFactory#validTypes is not up-to-date

Kevin Driver kdriver at openjdk.org
Mon Oct 9 19:22:56 UTC 2023


On Mon, 9 Oct 2023 18:59:43 GMT, Kevin Driver <kdriver at openjdk.org> wrote:

>> src/java.base/share/classes/com/sun/crypto/provider/PBEKeyFactory.java line 59:
>> 
>>> 57: 
>>> 58:     static {
>>> 59:         validTypes = HashSet.newHashSet(21);
>> 
>> Rather than having to change the initial size and keep it synchronized with all the subsequent adds, could we just change the type of `validTypes` to `Set<String>` and initialize it using `Set.of(E...)`?  It seems like that would allow you to set all the values directly in the declaration up on line 49, would remove the need for the static block and I think you could even declare validTypes final, couldn't you?  And any new additions to the set could simply be added and not worry about setting an accurate count in the constructor.
>
> I wondered about an approach like this. I'll push another commit with these changes.

Do you think we'll lose performance in a meaningful way? One of the guarantees of HashSet is constant-time operations. 

There is no such guarantee for Set. The number of members is probably small enough, relatively speaking, to not affect things much at creation time (and we only do this once); however, lookup time (in `engineGetKeySpec` and `engineTranslateKey`) might take a small hit if these methods are called repeatedly. 

Let me know whether you think this is a valid concern. Thanks!

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16103#discussion_r1350717600



More information about the security-dev mailing list