On Tue, 15 Oct 2024 08:05:44 GMT, Eirik Bjørsnøs <eir...@openjdk.org> wrote:
>> 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.
>
> We could also just use `Map<String, int[]>`. This would allow us to skip the
> whole `Set<Integer>` to `int[]` transition step, and the temporary `HashMap`
> is no longer needed.
>
> Deduplication could occur during registration:
>
>
> metaVersions.merge(name, new int[] {version}, (current, val) -> {
> // Check duplicates
> for (int v : current) {
> if (v == version) {
> return current;
> }
> }
> // Add version
> int[] merged = Arrays.copyOf(current, current.length + 1);
> merged[merged.length - 1] = version;
> return merged;
> });
>
>
> while the postprocessing step would simply sort each array:
>
>
> for (int[] versions : metaVersions.values()) {
> // JarFile::getVersionedEntry expects sorted versions
> Arrays.sort(versions);
> }
>
>
> This performs ~10% better on a version-heavy `ZipFileOpen`, and I think the
> code is reasonably simple compared to the current state of the PR. Thoughts?
I guess performance leans on there being only one or a small number of versions
per entry. Which I think is a fair assumption. Still would be good to have a
stress test with many entries having many versions and make sure we don't
regress such cases too much, no?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21489#discussion_r1801509202
