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

[Jaikiran Pai]

On Wed, 30 Oct 2024 16:17:20 GMT, Claes Redestad <redestad at openjdk.org> wrote:

>> src/java.base/share/classes/java/util/zip/ZipFile.java line 1798:
>> 
>>> 1796:                                 metaVersions.computeIfAbsent(hashCode, _ -> new BitSet()).set(version);
>>> 1797:                             } catch (Exception e) {
>>> 1798:                                 throw new IllegalArgumentException(e);
>> 
>> Hello Eirik, I'm late to this to PR. Was throwing an unspecified `IllegalArgumentException` from here intentional? Should it have been `IOException` instead?
>
> @eirbjo will know for sure but I think it's intentional since if there's an encoding error we'd have thrown earlier when hashing over the full name.

Hello Claes, 

> if there's an encoding error we'd have thrown earlier when hashing over the full name.

If I understand correctly, then I think you are referring to the `ZipCoder.checkedHash()` which throws that `Exception`.

Before the change in this PR, there was (and continues to exist) one such call in the `checkAndAddEntry()` private method of this `Source` class. In that place we catch the `Exception` and throw a `ZipException` (which is an `IOException`) https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/zip/ZipFile.java#L1264.

I think we should be doing the same here and throw a `ZipException` instead of an `IllegalArgumentException`

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

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