On Mon, 14 Oct 2024 11:01:21 GMT, Eirik Bjørsnøs <eir...@openjdk.org> wrote:
> Please review this PR which speeds up `JarFile::getEntry` lookup
> significantly for multi-release JAR files.
>
> The changes in this PR are motivated by the following insights:
>
> * `META-INF/versions/` is sparsely populated.
> * Most entries are not versioned
> * The number of unique versions for each versioned entry is small
> * Many JAR files are 'accidentally' multi-release; they use the feature to
> hide `module-info.class` from Java 8.
>
> Instead of performing one lookup for every version identified in the JAR,
> this PR narrows the version search down to only the number of versions found
> for the entry being looked up, which will most often be zero. This speeds up
> lookup for non-versioned entries, and provides a more targeted search for
> versioned entries.
>
> An alternative approach could be to normalize the hash code to use the
> none-versioned name such that versioned and non-versioned names would be
> resolved in the same lookup. This was quickly abandoned since the code
> changes were intrusive and mixed too many JAR specific concerns into
> `ZipFile`.
>
> Testing: The existing `JarFileGetEntry` benchmark is updated to optionally
> test a multi-release JAR file with one versioned entry for
> `module-info.class` plus two other versioned class files for two distinct
> versions. Performance results in [first comment](#issuecomment-2410901754).
>
> Running `ZipFileOpen` on a multi-release JAR did not show a significat
> difference between this PR and mainline.
>
> The JAR and ZIP tests are run locally. GHA results green. The `noreg-perf`
> label is added in JBS.
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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21489#discussion_r1799921281
