RFR: 8342040: Further improve entry lookup performance for multi-release JARs [v2]

[Eirik Bjørsnøs]

On Mon, 14 Oct 2024 18:17:15 GMT, Chen Liang <liach at openjdk.org> wrote:

>> Eirik Bjørsnøs has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Use Arrays.sort instead of TreeSet
>
> 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?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21489#discussion_r1800001845