Hi Claes, Hi Lance, thanks for reviewing my patch.
> I believe the convention in the OpenJDK sources is to sort static > imports after non-static, so changing to the other way around in a few > places adds inconsistencies. I was unaware of this but it seems that's the way most files are structured (though you'll find exceptions from that rule). I adapted my change. > In ZipFileSystem you remove the unused method releaseDeflater - to me > this indicates the resource pooling is actually broken? I.e., shouldn't > EntryOutputStreamDef return its Deflater to the cache when it's closed, > similar to how the anonymous InflaterInputStream in getInputStream does > it? As it's currently implemented the deflaters list will always be > empty and no Deflater returned, so could go the other way and remove > that cache if caching Deflaters isn't useful. You are right. I think this is a flaw of the change for 8034802 [1] [2]. I added the call to releaseDeflater() in the close method of EntryOutputStreamDef. > Also in ZipFileSystem, I would instead of removing line 2115, I would either > keep it and remove the other references to version() in writeLOC, or make > the change in writeCEN so that the usage of version is consistent. Good catch, I've updated this for consistency. This is the new webrev: http://cr.openjdk.java.net/~clanger/webrevs/8215472.1/ Are you good with it? I'll run jtreg tests before pushing... Thanks Christoph [1] http://hg.openjdk.java.net/jdk/jdk/rev/ba51515b64e5 [2] https://bugs.openjdk.java.net/browse/JDK-8034802
