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

Kumar Srinivasan kumar.x.srinivasan at oracle.com
Wed Jan 22 17:54:39 UTC 2014


Alex,

I am satisfied.

Thanks
Kumar



> 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