RFR: 8066644: Fix deprecation warnings in jdk.zipfs module

Xueming Shen xueming.shen at oracle.com
Wed Oct 28 22:17:29 UTC 2015


On 10/28/2015 03:00 PM, Claes Redestad wrote:
>
> On 2015-10-27 16:37, Claes Redestad wrote:
>> On 2015-10-27 15:09, Claes Redestad wrote:
>>>
>>>
>>> On 2015-10-27 14:40, Aleksey Shipilev wrote:
>>>> On 10/24/2015 11:54 PM, Claes Redestad wrote:
>>>>> webrev: <redacted>
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8066644
>>>> Looks good.
>>>>
>>>> Is there really no way to use the new DateTime API, and ditch the
>>>> multiplication by 1000L here?
>>>>
>>>>   116  return ldt.toEpochSecond(
>>>>   117    ZoneId.systemDefault().getRules().getOffset(ldt)) * 1000L;
>>>
>>> I guess using java.util.concurrent.TimeUnit might suffice:
>>>
>>> return TimeUnit.MILLISECONDS.convert(ldt.toEpochSecond(...), TimeUnit.SECONDS);
>>>
>>> The idiomatic DateTime way I guess would be to do:
>>>
>>> return Duration.ofSeconds(ldt.toEpochSecond(), 0).toMillis();
>>>
>>> I'll take these for a spin; properly inlined either variant might be performance neutral.
>>
>> It seems any difference between the second-to-millisecond conversion approaches is negligible under repeated testing[1], thus I think the preferred approach is to use be Duration.ofSeconds(..).toMillis():
>>
>> http://cr.openjdk.java.net/~redestad/8066644/webrev.03
>>
>
> Or we go with TimeUnit, since that's already used in the same utilities for straightforward conversions:
>
> http://cr.openjdk.java.net/~redestad/8066644/webrev.04
>
Thanks Claes! It looks good to me. I will push it for you.

-Sherman



More information about the core-libs-dev mailing list