On Tue, 11 Mar 2025 15:32:53 GMT, Eirik Bjørsnøs <eir...@openjdk.org> wrote:
>> Jaikiran Pai has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> tiny typo fix in newly introduced documentation
>
> src/java.base/share/classes/java/util/zip/ZipFile.java line 1222:
>
>> 1220: table[hsh] = index;
>> 1221: // Record the CEN offset and the name hash in our hash
>> cell.
>> 1222: entries[index] = hash;
>
> Seems unrelated to the issue at hand. Perhaps better left for a separate PR,
> backed by a benchmark.
This isn't a performance related change. I merely marked the method parameters
as `final` and the `index` method parameter was being updated here.
> src/java.base/share/classes/java/util/zip/ZipFile.java line 1426:
>
>> 1424: private final Charset entryNameCommentCharset;
>> 1425:
>> 1426: public Key(File file, BasicFileAttributes attrs, Charset
>> entryNameCommentCharset) {
>
> I feel this parameter and the field it is assigned to could also just be
> called `charset`. The comment has the information about what it's used for.
I've updated the PR to rename this to `charset`.
> src/java.base/share/classes/java/util/zip/ZipFile.java line 1434:
>
>> 1432: @Override
>> 1433: public int hashCode() {
>> 1434: long t = entryNameCommentCharset.hashCode();
>
> This represents a behavioral change, right? Is a CSR warranted?
>
> EDIT: Scratch that, I guess the caching mechanism here is unspecified and and
> more of implementation detail.
This is an internal implementation detail, so doesn't require a CSR.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23986#discussion_r1990706483
PR Review Comment: https://git.openjdk.org/jdk/pull/23986#discussion_r1990704378
PR Review Comment: https://git.openjdk.org/jdk/pull/23986#discussion_r1990704708
