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

Claes Redestad redestad at openjdk.org
Mon Oct 14 21:00:13 UTC 2024


On Mon, 14 Oct 2024 19:52:50 GMT, Eirik Bjørsnøs <eirbjo at 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


More information about the core-libs-dev mailing list