RFR JDK-8075526: Need a way to read and write time in UTC
Xueming Shen
xueming.shen at oracle.com
Mon Jul 6 17:07:07 UTC 2015
On 07/06/2015 07:15 AM, Claes Redestad wrote:
> Hi,
>
> On 2015-07-01 19:05, Xueming Shen wrote:
>> On 07/01/2015 05:02 AM, Claes Redestad wrote:
>>> Hi,
>>>
>>> On 2015-06-30 23:11, Xueming Shen wrote:
>>>> Hi,
>>>>
>>>> Please help review and comment on this rfe.
>>>>
>>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8075526
>>>> webrev: http://cr.openjdk.java.net/~sherman/8075526
>>>
>>> I think this looks reasonable.
>>>
>>> I think we could consolidate the LocalDateTime<->xdostime
>>> conversions into ZipUtils along with and aligned with the code
>>> to convert UTC instants (ZipUtils::dosToJavaTime/javaToDosTime),
>>> which I've suggested should be converted into similar code
>>> anyhow:
>>
>> I would prefer to deal with JDK-8066644 issue separately. One of the concerns
>> of replacing the j.u.Date with the java.time classes was the possible startup
>> time regression that may be triggered by forcing the vm to load lots of java.time
>> classes given all the jvm starts with loading bunch of jar files. As a matter of
>> fact, we spent the time and did the prototype when doing JSR310, it did show
>> startup regression back then... This might no longer be an issue if moving to
>> module system, but it'd be better to have some data first. On the other hand,
>> the proposed change here is for two new methods, no impact to any existing
>> apps.
>>
>
> I fired up the patch proposed to address 8066644 on latest jdk9/dev (the work was paused
> while Peter Levart addressed some related inefficiencies, e.g., caching result in
> ZoneId.getDefault()) and have verified that replacing the current j.u.Date implementation
> with java.time in ZipUtils have no measurable effect on our internal startup and footprint
> benchmarks (with/without modules) since neither is no longer loaded on application startup.
>
> I don't care if we move forward with 8066644 before or after this patch, justnoticedthat
> the parts of the code from this and that patch looks very much the same.
>
Thanks for taking time on this. But let's do it separately. Just in case 8066644 might need
to be backport.
> Also spotted this nit:
>
> + this.mtime = FileTime.from(
> + ZonedDateTime.of(time, ZoneId.systemDefault()).toInstant());
>
> could be written
>
> + this.mtime = FileTime.from(
> + time.atZone(ZoneId.systemDefault()).toInstant());
@Override
public ZonedDateTime atZone(ZoneId zone) {
return ZonedDateTime.of(this, zone);
}
Just tried to save one layer of invocation :-) though probably does not matter. The
chained/short code may look more decent.
-sherman
More information about the core-libs-dev
mailing list