RFR: 8376403: Avoid loading ArrayDeque in java.util.zip.ZipFile [v4]
Eirik Bjørsnøs
eirbjo at openjdk.org
Sat Feb 21 17:01:10 UTC 2026
On Sat, 21 Feb 2026 16:45:44 GMT, Eirik Bjørsnøs <eirbjo at openjdk.org> wrote:
> Setting a synchronized field to null is a bit of a code smell, so ideally I think that `inflaterCache` should be made final, the list should be cleared instead of nulled and we should introduce a separate `boolean` field to track the closed state of the cache. But that's perhaps for another PR.
Another benefit of making the closed state explicit is that it would remove the awkward `null` and double `==` checking.
Diff on top of this PR:
Index: src/java.base/share/classes/java/util/zip/ZipFile.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/java.base/share/classes/java/util/zip/ZipFile.java b/src/java.base/share/classes/java/util/zip/ZipFile.java
--- a/src/java.base/share/classes/java/util/zip/ZipFile.java (revision 02de2507929a1431a1070e0d6660c1585152cf9f)
+++ b/src/java.base/share/classes/java/util/zip/ZipFile.java (date 1771693058851)
@@ -691,7 +691,9 @@
final Set<InputStream> istreams;
// List of cached Inflater objects for decompression
- List<Inflater> inflaterCache;
+ final List<Inflater> inflaterCache;
+ // Closed state, access guarded by inflaterCache
+ boolean inflaterCacheClosed;
final Cleanable cleanable;
@@ -727,36 +729,28 @@
* Releases the specified inflater to the list of available inflaters.
*/
void releaseInflater(Inflater inf) {
- List<Inflater> inflaters = this.inflaterCache;
- if (inflaters != null) {
- synchronized (inflaters) {
- // double checked!
- if (inflaters == this.inflaterCache) {
- inf.reset();
- inflaters.add(inf);
- return;
- }
- }
- }
- // inflaters cache already closed - just end it.
- inf.end();
+ synchronized (inflaterCache) {
+ if (inflaterCacheClosed) {
+ // inflaters cache already closed - just end it.
+ inf.end();
+ } else {
+ inf.reset();
+ inflaterCache.add(inf);
+ }
+ }
}
public void run() {
IOException ioe = null;
// Release cached inflaters and close the cache first
- List<Inflater> inflaters = this.inflaterCache;
- if (inflaters != null) {
- synchronized (inflaters) {
- // no need to double-check as only one thread gets a
- // chance to execute run() (Cleaner guarantee)...
- for (Inflater inf : inflaters) {
- inf.end();
- }
- // close inflaters cache
- this.inflaterCache = null;
- }
+ synchronized (inflaterCache) {
+ for (Inflater inf : inflaterCache) {
+ inf.end();
+ }
+ inflaterCache.clear();
+ // close inflaters cache
+ this.inflaterCacheClosed = true;
}
// Close streams, release their inflaters
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/29430#discussion_r2836359284
More information about the core-libs-dev
mailing list