Ah, I suspect the getTime() for xdostime performance issue relates to this: https://bugs.openjdk.java.net/browse/JDK-4981560 which is not relevant any more since dosToJavaTime() is now implemented differently....
On Tue, Nov 30, 2021 at 2:44 PM Andrew Leonard <anleo...@redhat.com> wrote: > Thanks Alan, > Having tried implementing a Zoned option, I agree it does seem to bloat > ZipEntry a bit. > As JohnN's suggestion pointed out we could at some point move to always > setting the extended > mtime(FileTime), but we have 78years until we really need to worry about > that! (xdostime covers to 2099) > It seems a conscious decision to only set xdostime was made for VM startup > optimization (see https://bugs.openjdk.java.net/browse/JDK-8073497), at > least until 2099. > > I will update the PR based on option (1), which I have a commit which is > nearly at that point anyway. > I like your suggestion of restricting the --date option to 1980->2099 > > regards > Andrew > > > On Tue, Nov 30, 2021 at 1:13 PM Alan Bateman <alan.bate...@oracle.com> > wrote: > >> On 29/11/2021 19:25, Andrew Leonard wrote: >> >> *Problem:* >> PR https://github.com/openjdk/jdk/pull/6481 >> addresses the main problems with deterministic timestamping of Zip entries, >> with >> the introduction of a new --date <ISO-8601-date-time> option for jar & jmod. >> However, the ZipEntry timestamp is constructed from a combination of an >> MS-DOS timestamp >> and a FileTime(seconds since epoch). MS-DOS timestamp is used between >> 1980->2106, FileTime is >> used outside that range. >> The problem arises in deterministically setting the ZipEntry times within >> JVMs >> with different default system time-zones. This occurs because >> ZipEntry.setTime(long millisSinceEpoch) or >> setTimeLocal(LocalDateTime) are unaware of time-zones. So when setTime has >> to calculate the msdostime from >> millisSinceEpoch, it has to query the system time-zone to work out what the >> "date-time" is. Similarly, for >> setTimeLocal(LocalDateTime), if the LocalDateTime is outside 1980-2106 >> range, then it has to create a >> FileTime, hence again it has to use the system time-zone to work out how >> many seconds since epoch >> the LocalDateTime is. >> >> *Solutions proposed so far:* >> 1. Take the --date <ISO-8601-datetime> value and create a LocalDateTime >> eg.1982-12-09T14:02:06, and call >> ZipEntry.setTimeLocal(with LocalDateTime"1982-12-09T14:02:06"). For the >> same input and across time-zones >> the ZipEntry is deterministic when within the valid MS-DOS time range >> (1980->2106), because no extended >> FileTime is created. >> Pros: >> - No API changes, uses existing ZipEntry.setTimeLoca(LocalDateTime) >> Cons: >> - Is only determinstic between years 1980->2106, outside this range a >> FileTime is generated using the system >> time-zone. >> >> 2. Add new "Zoned" set/get methods to ZipEntry, so that ZipEntry does not >> have to defer to the system time-zone. >> By adding set/getTimeZoned(ZonedDateTime), >> set/getLastModifiedTimeZoned(FileTime, ZoneId) methods to ZipEntry >> the ZipEntry MS-DOS time and extended FileTime can be set precisely and >> deterministically to the user supplied >> date-time. >> Pros: >> - ZipEntry is always deterministically created for the same input --date >> <ISO-8601-datetime>, across different JVM >> time-zones, for all time ranges. >> Cons: >> - An API addition is required to ZipEntry to add new "Zoned" set/get time >> methods. >> >> Thoughts, comments, or alternatives most welcome please? >> >> ZipEntry already defines 3 methods to set the last modification time so I >> think we have to be cautious about adding additional methods. If we >> eventually do add a method to set the date-time with time zone then I think >> it will require broader work in the javadoc to help developers choose the >> right method to use. >> >> I don't think we should get hung up being able to set (via a CLI option) >> the time stamps to the 1970-01-01 epoch, it's just not interesting. For >> reproducible builds it should sufficient to set the timestamp to a fixed >> DOS date/time. >> >> So for the above options then I think #1 isn't too bad. The tools could >> limit the range supported by the --date option to avoid needing to use the >> modification time in the extended field data. >> >> -Alan. >> >> >> >> >> >> >> >> >> >> >> >>
