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 <[email protected]> On Behalf > Of Xueming Shen > Sent: Mittwoch, 19. Dezember 2018 08:15 > To: [email protected] > 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
