On Mon, 14 Oct 2024 20:57:45 GMT, Claes Redestad <redes...@openjdk.org> wrote:
>> If we trust that versioned entries are rare, this should not matter much
>> either way. But I pushed a commit which uses HashSet and Arrays::sort on
>> freezing instead. WDYT?
>>
>> Given that most versioned entries will only have a single version, we may
>> save some memory and speed by special-casing for sets with one or two
>> versions:
>>
>>
>> switch (metaVersionsMap.get(name)) {
>> case null -> {
>> // Special-case for set of size 1
>> metaVersionsMap.put(name, Set.of(version));
>> }
>> case Set<Integer> versions when versions.size() == 1 -> {
>> // Special-case for set of size 2
>> metaVersionsMap.put(name, Set.of(version,
>> versions.iterator().next()));
>> }
>> case Set<Integer> versions when versions.size() == 2 -> {
>> // Now we convert to HashSet
>> HashSet<Integer> set = new HashSet<>(versions);
>> set.add(version);
>> metaVersionsMap.put(name, set);
>> }
>> case HashSet<Integer> versions -> {
>> // Just add
>> versions.add(version);
>> }
>> default -> throw new AssertionError("Illegal state");
>> }
>>
>>
>>
>> But again, versioned entries are rare, so perhaps better to keep it simple.
>> Does @cl4es have thoughts about this?
>
> I like simple.
>
> Mixing Set.of and HashSet means a lot of semantic fu-fu (throwing/not
> throwing on nulls) and risks being suboptimal due making some call sites
> megamorphic.
Yep, looks good. And I agree with Claes' evaluation that we should not
complicate this construction further, especially that the result is quickly
garbage collected when we compile into the string to int array map.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21489#discussion_r1800467908
