RFR: 8376403: Avoid loading ArrayDeque in java.util.zip.ZipFile [v3]
Eirik Bjørsnøs
eirbjo at openjdk.org
Fri Feb 20 12:51:17 UTC 2026
On Fri, 20 Feb 2026 12:28:14 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:
>> Eirik Bjørsnøs has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Remove null check of final field istreams which is never null
>
> src/java.base/share/classes/java/util/zip/ZipFile.java line 736:
>
>> 734: inf.reset();
>> 735: inflaters.addLast(inf);
>> 736: return;
>
> Hello Eirik, nit - my preference here would be to use `add(...)`, which is the same as `addLast(...)` but feels a bit more natural when using a `List`. When reading the code, the presence of `addLast(...)` makes me stop for a while to see if there's anything more going on here other than just adding to a `List`.
I probably did this to balance with the `removeLast` in `getInflater`. And to express the List being used as a FIFO stack.
But I agree this probably raises more questions than answers, so reverted to add.
> src/java.base/share/classes/java/util/zip/ZipFile.java line 755:
>
>> 753: Inflater inf;
>> 754: while (!inflaters.isEmpty()) {
>> 755: inflaters.removeLast().end();
>
> I think we should replace this with a trivial:
>
>
> for (Inflater inf : inflaters) {
> inf.end();
> }
>
> for the same reasons as in my previous comment - it makes it trivial to understand without raising the questions about last vs first.
Good point, yes the iteration order should not matter.
The list will not be empty after this loop, but it doesn't really matter since it is immediately set to `null` anyhow.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/29430#discussion_r2833008589
PR Review Comment: https://git.openjdk.org/jdk/pull/29430#discussion_r2833023224
More information about the core-libs-dev
mailing list