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

Reply via email to