JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.

Alexander Zuev alexander.zuev at oracle.com
Wed Jan 22 13:39:05 UTC 2014


Kumar,

   see my notes inline.

On 1/21/14 20:11, Kumar Srinivasan wrote:
> Alex,
>
> Thanks for adding the test, few comments:
>
> PackTestZip64.java:
>
> 1. compareTwoFile, I would read the entire file into 
> ByteArrayInputStream, compare
> the total first ie. fast fail, but what you have is also ok.
I've tried the approach of reading the entire file into memory and it 
doesn't seem to
speed up the process so much, so i'd left it as it is.
> 2.  generateLargeJar: I would replace lines 126,127 and 128 using 
> for-each loop,
> by using Collections.list<Enumeration> e); will make it more compact.
Done.
> 3. Though the test might not take too long on your system, I am 
> concerned it may
> take too long on our Jprt system.
I tested it on JPRT - there's no system on which test takes more than a 
minute - actually
the longest test run time was on the solaris-sparc and it's 44 seconds, 
so it seems it's Ok.

The updated webrev can be found at: 
http://cr.openjdk.java.net/~kizune/8029646/webrev.03

/Alex
> Kumar
>
>
> On 1/16/2014 8:21 AM, Alexander Zuev wrote:
>> Sherman, Kumar,
>>
>>   i have fixed the glitches you have found and changed the test so it 
>> creates a new jar
>> based on the golden.jar content (to have a reasonable set of various 
>> data to start with),
>> then adding to it 65536 empty files to test how we cope with such a 
>> huge jars.
>>
>>   The testing time is less than a 30 seconds on my machine (not the 
>> top-of-the-line) so i
>> decided not to bother with conditional testing, lets test our 
>> behavior every time in full.
>>
>>   The updated webrev can be found at 
>> http://cr.openjdk.java.net/~kizune/8029646/webrev.02
>>
>> /Alex
>>
>> On 1/15/14 21:34, Xueming Shen wrote:
>>> On 1/15/14 7:01 AM, Alexander Zuev wrote:
>>>> Hello,
>>>>
>>>>   the new webrev with all the typos and comments fixed can be found 
>>>> at http://cr.openjdk.java.net/~kizune/8029646/webrev.01/
>>>>
>>>> /Alex
>>>
>>> (1)  jarmagic can be just a static constant somewhere or a stack 
>>> variable. not big deal though.
>>> (2)  the test only "tests" EOF for s. there is possibility that the 
>>> newly created has more extra
>>>       bytes at the end...though in theory this should not happen, it 
>>> might be better just add an
>>>       extra line to check the sizes of two first first?
>>> (3)  the rest of the change looks good, but I agreed with Kumar that 
>>> you may need to add a
>>>       regression test for  a jar file with > 64k entries. otherwise 
>>> the code for zip64 end  is not
>>>       being tested. the code looks fine, but I would trust a 
>>> regression test more than my eyes:-)
>>>
>>> Thanks!
>>> -Sherman
>>
>




More information about the core-libs-dev mailing list