On Mon, 14 Oct 2024 19:52:50 GMT, Eirik Bjørsnøs <eir...@openjdk.org> wrote:
>> src/java.base/share/classes/java/util/zip/ZipFile.java line 1831:
>>
>>> 1829: metaVersions = new HashMap<>();
>>> 1830: for (var entry : metaVersionsMap.entrySet()) {
>>> 1831: // Convert TreeSet<Integer> to int[] for
>>> performance
>>
>> I think if you just require order upon final freezing, you can just use a
>> HashSet or ArrayList, and perform the sorting and deduplication when you
>> compile to an int array.
>
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21489#discussion_r1800131485
