RFR: 8222532: (zipfs) Performance regression when writing ZipFileSystem entries in parallel

Xueming Shen xueming.shen at gmail.com
Wed Apr 24 07:28:18 UTC 2019


Alan, Claes,

If my memory was correct, ExChannelClose was not being used even in 
previous implementation.

Actually it should never have been used in any of the released JDK. I 
might have been missing some discussions ... so I might be wrong here. 
But the original reason of having a exchannelcloser was

that I was implementing the sync() the way that it would be invoked 
during its lifetime. Means you

can actually fresh the in-memory entries back to underlying zipfile to 
reduce memory pressure ...

because the fs is not really being closed in this situation, we can't 
just close those streams. Have to

keep them alive till they get closed by the caller. But then the plan 
was changed and the sync() is

being invoked only in its close(). In this situation, we should simply 
close those pending streams.

The filesystem is getting closed, any inputstream open on its entry 
should be closed as well. No

need for this closer any more in current sync() use scenario.

-Sherman



ExistingChannelCloser

On 4/23/19 4:42 AM, Claes Redestad wrote:
> Hi Alan, Christoph,
>
> thanks for reviewing this!
>
> On 2019-04-23 12:42, Alan Bateman wrote:
>> On 18/04/2019 14:10, Claes Redestad wrote:
>>> Webrev: http://cr.openjdk.java.net/~redestad/8222532/open.00/
>> I think the approach looks good.
>>
>> One thing that I think could be improved is ExChannelCloser. I think 
>> it needs a better name. Also I think it should encapsulate its fields 
>> a lot better so that sync doesn't need to access its interanls -- 
>> e.g. maybe it can define a closeAndDelete method that closes the 
>> channel and deletes the path.
>>
>
> Based on what it does, ExChannelCloser (which I restored from its pre-
> existing state) should probably be named ExistingChannelCloser.
>
> I've collected the feedback from you, Christoph and Lance into this
> new webrev: http://cr.openjdk.java.net/~redestad/8222532/open.01/
>
> Incremental: http://cr.openjdk.java.net/~redestad/8222532/open.00_01/
>
> Thanks!
>
> /Claes


More information about the core-libs-dev mailing list