On Fri, 20 Feb 2026 12:48:17 GMT, Eirik Bjørsnøs <[email protected]> wrote:
>> Hot on the heals of #29288, this PR replaces `ArrayDeque` with `ArrayList`
>> for the Inflater cache implementation in `ZipFile.CleanableResource`.
>>
>> With this PR, we change the order of the cache from a LIFO queue to a FIFO
>> stack backed by ArrayList. The order seems unimportant, and has indeed been
>> FIFO in the past when using `java.util.Vector`. Intuitively, it should be
>> better to return the most recently used Inflater.
>>
>> No tests updated, strict refactoring, `noreg-cleanup`.
>
> Eirik Bjørsnøs has updated the pull request incrementally with three
> additional commits since the last revision:
>
> - Add code comment about returning the most recently used Inflater from the
> cache
> - Replace polling-style iteration with simple for loop
> - Using add instead of addLast seems more natural for a List
Overall the changes look good to me. I have added some inline review comments.
Please give me a few more days to approve this PR. I remember there was some
discussion around the Inflater cache usage and how it may impact the
performance. In this PR we have changed the policy on which Inflater we hand
out from the cache, I would like to make sure that it doesn't have any
unforeseen impact.
src/java.base/share/classes/java/util/zip/ZipFile.java line 719:
> 717: synchronized (inflaterCache) {
> 718: if (!inflaterCache.isEmpty()) {
> 719: return inflaterCache.removeLast();
The use `removeLast()` I think is fine. Can you add a one liner comment here to
say something like:
// return back the most recently used Inflater from the cache of not-in-use
Inflaters
-------------
PR Review: https://git.openjdk.org/jdk/pull/29430#pullrequestreview-3831843273
PR Review Comment: https://git.openjdk.org/jdk/pull/29430#discussion_r2833012466
