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