RFR: JDK-8150496,(zipfs) Fix performance issues in zip-fs

[Xueming Shen]

On 5/5/16 5:18 AM, Alan Bateman wrote:
> On 04/05/2016 20:54, Xueming Shen wrote:
>> Thanks Claes! fixed and webrev has been updated.
>>
>> http://cr.openjdk.java.net/~sherman/8150496/webrev
>>
>> sherman
> I skimmed through this and it looks quite. I just wonder if we should 
> check in the micro benchmarks as it is likely we'll need those again 
> in the future.

Will need to find an appropriate place to place it. Any idea? I can file 
an issue for this one.

>
> One thing that isn't clear (to me anyway) is the change to detecting 
> \u0000 in the normalization code. Do we have any cases where 
> IllegalPathException is thrown?
>

The change I did here is just to delay the IPE throwing later into 
normalize(byte[], int) (where we do the real
normalization). The motivation here is to combine two checks into one 
"if" (with the hope it can speed thing
up a little), given it is rare to have such Path in real world use scenario.

> In checkAccess there is a comment about needing to read the CEN. It 
> this method needed? I would check that ZipFileSystemProvider should 
> just need to invoke exists (+ checks when w or x modes).

This method is the real implementation for "exists/path.checkAccess". 
The only thing changed here
is to not create an entry object (read the cen table for that particular 
entry and load in all the fields)
on top of an inode (which is read from the cen already, only with the 
"name" and "offset" info), as we
really are not using the entry (now as an "attributes" as well), the 
only thing we need to know here
is if such Entry/Inode exists. The only benefit of creating such Entry 
is to verify that all the fields of
this Entry are valid. The current implementation will create such entry 
and validate it later when go
read the bytes of the entry(get an in/output stream), or try to obtain 
an "attributes".

>
> In cen then the versionMade-attrsEx is commented out, should they be 
> removed?
>
They can be removed. I keep them there is just for convenience that we 
might need them back,
in the future.

Thanks,
-Sherman