RFR(S): 8073497: Lazy conversion of ZipEntry time

[Xueming Shen]

On 02/26/2015 12:13 PM, Claes Redestad wrote:
> On 02/26/2015 05:45 PM, Xueming Shen wrote:
>> On 2/25/15 4:17 PM, Claes Redestad wrote:
>>>
>>>
>>> We could preserve precision by shifting the remaining milliseconds into the dostime field,
>>> since the DOS time will only use the 4 lower bytes:
>>>
>>> http://cr.openjdk.java.net/~redestad/jdk9/8073497/webrev.4/
>>>
>>> After I went through more extensive testing and verification I realized that narrowing
>>> dostime to an int and breaking this remainder hack out into a separate field would be
>>> possible without affecting ZipEntry footprint; arguably cleaner, but if this version is
>>> acceptable I would prefer not to.
>>>
>>> Going this way preserves precision across the entire input range of setTime, thus all tests
>>> I could find pass with this version; I've also expanded the TestExtraTime test to test both
>>> far ancient, far future and near future times to verify precision is preserved.
>>>
>>> I backpedaled on the getLastModifiedTime optimization to set the mtime field that Peter
>>> suggested since it would have the side-effect that the extra time field would be written
>>> after a call to this getter, which would be a bit too unexpected.
>>>
>>> Thanks!
>>>
>>> /Claes
>>
>> Hi Claes,
>>
>> It's a good idea! Given the upper half now covers that 2000 million seconds, the "dostime" field
>> is no longer a real "dos time", just wonder if we can go a little further.  For example simply move
>> the bits for years/months/days/hours/minutes/seconds out (left) a little more, 11(?) bits, to leave
>> the lower bits for the 2000 million seconds. The benefit is that the upper bound is no longer needed.
>> We need a pair of package private set/getDostime()  for ZIS, ZOS and ZF to access though. Yes,
>> it costs a little left/right bit-shift when to set/get the dostime, but it does not requires the
>> expensive  j.u.Date/timezone.
>
> Not sure what the practical benefit would be, though: the entry we read from and write to is
> just 32 bits, so precision will be still be lost when writing and we won't read more exact values.
>
> So.. we'd still need to populate and write the extended time field sooner or later (although no later
> than 2107) or accept the overflow to 1980. We can chose to do that implicitly (like in this patch:
> let the dostime overflow, but output the extended timestamp for full precision) or leave it to the
> end user to handle this by explicit use of setLastModifiedTime.
>
> The current proposal has the benefit that the 4 bytes we write/read in ZIS/ZOS/ZF is the actual
> dostime, the "soft" upper bound enables us to cheaply detect risk of overflow and act accordingly
> (we could add 10 years to the bound, though), while requiring no extra manipulation when just
> reading a zip (which I believe needs to be our optimization priority here).
>
> We'd also have to update the javaToDosTime to do long arithmetic since it currently only keeps 7
> bits for the year field. Minor detail, though.
>
> /Claes

Yes, practically it does not benefit any real world zip/jar file. Just thought the upper-bound check
might not be necessary. No, I don't have a strong opinion on this one.

-Sherman