On Tue, 15 Oct 2024 05:13:14 GMT, Chen Liang <li...@openjdk.org> wrote:
>> 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.
>
> 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?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21489#discussion_r1800657937
