RFR: JDK-8189611: JarFile versioned stream and real name support

[Xueming Shen]

On 11/17/17, 3:35 AM, Alan Bateman wrote:
> On 17/11/2017 02:18, Xueming Shen wrote:
>> Hi,
>>
>> Please help review the change for JDK-8189611.
>>
>> issue: https://bugs.openjdk.java.net/browse/JDK-8189611
>> webrev: http://cr.openjdk.java.net/~sherman/8189611/webrev
> Just a few initial comments/questions on the API additions:
>
> 1. getRealName is very welcome but I think it should be a no-arg 
> method on JarEntry, not JarFile. That would make it easier to use and 
> also avoids the temptation to call JarFile.getRealName with an entry 
> in a different JAR file.
>

I was considering to put it into JarEntry. The only concern is that all 
mr jarfile related stuff
are in JarFile, it's kinda easy/convenient to refer to the concept "if 
multi-release is..." in
JarFile than JarEntry, especially considering mr jarfile normally should 
only be interested/
used by limited group of developer, it might be better to simply put it 
near those mr related
methods. But I don't have a strong opinion about it. I can move it into 
JarEntry, if it's desired.


> 2. JarFile.versionedStream() is very welcome too but the javadoc says 
> "latest versioned entry" when it really means the highest version that 
> is less than or equal to the runtime version that the JarFile was 
> opened with. I suspect this one might take a few iterations to get the 
> wording smooth.

sure. will see if I can find better wording.
>
> 3. Is ZipFile.entryNameStream really needed? Just asking because 
> zf.stream().map(ZipEntry::getName) is possible today.
>
It's not a "must" for sure. The motivation behind this is that my 
observation of most normal use scenario
inside JDK is that only the "name" info is interested/collect/used. The 
use pattern usually is

zf.stream().map(ZipEntry::getName)

same for the old Enumeration case, only the "name" is used and the 
"entry" object is thrown away mostly.

Other than the memory consumption (showed in those two snapshots), it's 
also relatively costly to create
the ZipEntry especially its "esxdostime" calculation. The jmh numbers 
from a simple benchmark test
suggests the entryNameStream is about 15-20% faster. So, the only reason 
it's there is for better
performance. And hope it might be useful/convenient for the end user as 
well. If it's a concern, I can
hide it into the SharedSecrets, or simply use the 
zf.stream().map(ZipEntry::getName) instead.

--------------------------------------
JarFileBenchmark.jf_entries          avgt   20  0.871 ± 0.079  ms/op
JarFileBenchmark.jf_entryNameStream  avgt   20  0.786 ± 0.165  ms/op
JarFileBenchmark.jf_getEntry         avgt   20  6.805 ± 0.615  ms/op
JarFileBenchmark.jf_stream           avgt   20  0.915 ± 0.096  ms/op
JarFileBenchmark.jf_versionedStream  avgt   20  9.412 ± 0.642  ms/op
JarFileBenchmark.zf_entries          avgt   20  0.877 ± 0.092  ms/op
JarFileBenchmark.zf_entryNameStream  avgt   20  0.730 ± 0.123  ms/op
JarFileBenchmark.zf_getEntry         avgt   20  1.805 ± 0.177  ms/op
JarFileBenchmark.zf_stream           avgt   20  0.926 ± 0.167  ms/op
---------------------------------------

Thanks!
-Sherman