On 2015-02-23 16:41, Claes Redestad wrote:
On 02/22/2015 10:16 PM, Xueming Shen wrote:
On 2/21/15 6:11 PM, Claes Redestad wrote:
Hi Sherman,
On 2015-02-21 19:49, Xueming Shen wrote:
Hi Claes,
This change basically undo the "fix" for 4759491 [1], for better
performance ...
my intent was never to break current behavior, but that mistake can
be rectified
without missing out on the startup benefit of laziness:
http://cr.openjdk.java.net/~redestad/jdk9/8073497/webrev.1/
The time/mtime getters now use the mtime field if it exists, while
the setters will
update both fields. Since getLastModified already fell back to
converting the
time field rather than null if mtime wasn't set, setting mtime to a
FileTime when
calling setTime seems consistent and a cheap way to preserve the
time precision.
I guess a range check to skip the FileTime creation in setTime if
the time is within
the DOS time bounds would be a valid optimization, since that will
typically
always be the case.
It's a reasonable solution. I would assume we probably need that
"range check" optimization
to avoid setting the "mtime" field in normal use scenario. ZOS now
outputs the more accurate
"mtime" to the zip file using "extend timestamp" if it's not null
(the entry gets bigger). The
assumption now is that we only output the extended timestamp if the
time stamps set
via the setXYZTime() explicitly.
ZOS.java
...
432 int elenEXTT = 0; // info-zip extended
timestamp
433 int flagEXTT = 0;
434 if (e.mtime != null) {
435 elenEXTT += 4;
436 flagEXTT |= EXTT_FLAG_LMT;
437 }
438 if (e.atime != null) {
439 elenEXTT += 4;
440 flagEXTT |= EXTT_FLAG_LAT;
441 }
442 if (e.ctime != null) {
443 elenEXTT += 4;
444 flagEXTT |= EXTT_FLAT_CT;
445 }
446 if (flagEXTT != 0)
447 elen += (elenEXTT + 5); // headid(2) + size(2) +
flag(1) + data
448 writeShort(elen);
...
Here's my attempt to resolve this:
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
