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