On 12/18/18 5:44 AM, Langer, Christoph wrote:
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.
My bad. Adding the logic back looks fine. In latest implementation the
EntryOutputStreamDef
is only created/used in sync() to write out any updated/compressed
entry, which means there
is actually only one deflater is being used at a time during the sync(),
so you can just have one
deflater and then reset it before passing on to the next entry write.
And in fact even the EOSDef
is really not necessary. I was working on that ... but somehow I dropped
the ball during copy/
paste :-( ended up using the deflater from the cache but failed to
return it back.
Anyway. It's fine to keep current deflater cache mechanism, but it might
be worth considering
to "inline" the EntryOutputStreamDef logic and/or remove the deflater
cache at all in the future.
-Sherman
