RFR (S): 8215472: Cleanups in implementation classes of jdk.zipfs and tests

[Langer, Christoph]

Hi Sherman,

thanks for the confirmation. Further refactoring regarding Deflator caching can be done with a separate issue.

So, hearing no objections I'll push the patch once my testing is done.

Best regards
Christoph

> -----Original Message-----
> From: core-libs-dev <core-libs-dev-bounces at openjdk.java.net> On Behalf
> Of Xueming Shen
> Sent: Mittwoch, 19. Dezember 2018 08:15
> To: core-libs-dev at openjdk.java.net
> Subject: Re: RFR (S): 8215472: Cleanups in implementation classes of jdk.zipfs
> and tests
> 
> 
> 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