On Mon, 9 Oct 2023 18:59:43 GMT, Kevin Driver <kdri...@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
