RFR (S): 8215472: Cleanups in implementation classes of jdk.zipfs and tests
Langer, Christoph
christoph.langer at sap.com
Tue Dec 18 13:44:56 UTC 2018
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
More information about the core-libs-dev
mailing list