RFR:8143413:add toEpochSecond methods for efficient access

Xueming Shen xueming.shen at oracle.com
Tue Dec 1 17:33:56 UTC 2015


On 12/1/15 6:36 AM, Stephen Colebourne wrote:
> As Roger says, these new methods are about performance as well as conversion.
>
> While I fully acknowledge the time methods make an assumption, it is
> not a crazy one, after all 1970-01-01 is just zero.
>
> Key I think is it allows:
>   long epochSecs = date.toEpochSeconds(offset) + time.toEpochSeconds(offset);
> to efficiently merge two objects without garbage.

So it's not about j.t.LD/LT <-> j.u.Date, but instead of the clean approach

LocalDate date = ...
LocalDate time = ...
ZoneOffset offset = ...

==> long spochSecs = LocalDateTime.of(date, time).toEpochSeconds(offset);

we are adding APIs to provide a "fastpath" with the special assumption 
that the LocalDate "date"
here is actually a "LocalDateTime" object ("date" + LocalTime.MIDNIGHT) 
and the LocalTime "time"
again actually means a "LocalDateTime" (the "time" of 1970-01-01), to  
let the third party "libraries"
to fool the basic date/time abstract in java.time package,  simply to 
avoid creating the garbage
middle man, localDateTime? it really does not sound right to me.

First, if someone needs to mix/link LocalDate, LocalTime and Offset to 
epoch seconds in their
libraries, they really SHOULD think hard about the conversion and make 
it right (it should not
be encouraged to use these classes LocalDate, LocalTime and Offset 
without  even understand
what these classes are). But if we really have to provide such fastpath, 
personally I think it might
be better either to provide these "utility/convenient" methods in a 
"utilities" class, or with an
explicit date/time parameters (instead of the false assumption) for the 
missing date/time piece,
such as

localDate.toEpochSeconds(LocalTime.MIDNIGHT, offset);
localTime.toEpochSeconds(LocalDate.EPOCHDAY, offset);

Sherman


>
> It also means that no-one has to think hard about the conversion, as
> it is just there. It tends to be when people try to work this stuff
> out for themselves that they get it wrong.
>
> Stephen
>
>
> On 1 December 2015 at 14:21, Roger Riggs <Roger.Riggs at oracle.com> wrote:
>> Hi Sherman,
>>
>> On 11/30/2015 6:09 PM, Xueming Shen wrote:
>>> On 11/30/2015 01:26 PM, Stephen Colebourne wrote:
>>>> Converting LocalDate<-> java.util.Date is the question with the
>>>> largest number of votes on SO:
>>>>
>>>> http://stackoverflow.com/questions/21242110/convert-java-util-date-to-java-time-localdate/21242111
>>>> The proposed method is designed to make the conversion easier. It also
>>>> plugs an obvious gap in the API.
>>>>
>>>> While the LocalTime/OffsetTime methods are of lower importance, not
>>>> having them would result in inconsistency between the various classes.
>>>> We've already added factory methods to LocalTime for Java 9, these are
>>>> just the other half of the picture.
>>>>
>>> I'm not sure I understand the idea of "the proposed method is designed to
>>> make the conversion easier", as the SO is mainly about
>>> j.u.Date->LocalDate,
>>> not the other way around, from LocalDate -> j.u.Date.
>>
>> I think the issue is about *other* libraries that manipulate time via
>> epochSeconds
>> not about j.u.Date conversions.  The concern was about performance and
>> creating garbage along the way.
>>
>> Roger
>>
>>
>>
>>> As I said in the previous email, it might be "common" to use the j.u.Date
>>> to
>>> abstract a "local date" and/or a "local time" (no other choice) before
>>> java.time,
>>> and now there is the need to provide a migration path from those "local
>>> date/
>>> time" to the j.t.LocalDate/Time. But convert backward from the new
>>> date/time
>>> type to the "old" j.u.Date should not be encouraged (guess this is also
>>> the
>>> consensus we agreed on back to jsr203).
>>>
>>> What are the "factory methods" you are referring to here? JDK-8133079, The
>>> LocalDate/LocalTime.ofInstant()?
>>> (btw, these two methods see missing the "since 1.9/9" tag)
>>>
>>> It seems to me that the ofInstant(Instant, ZondId) is from a "super-set"
>>> of
>>> date/time to a "sub-set", without any assumption of "default value", it is
>>> similar to the conversion from zdt->ldt->ld/lt, and I can see the "small"
>>> improvement
>>>
>>> from|
>>> Date input = new Date();
>>> LocalDatedate
>>> =input.toInstant().atZone(ZoneId.systemDefault()).toLocalDate();|
>>>
>>> to
>>>
>>> |LocalDatedate
>>> =LocalDate.ofInstant(input.toInstant(),ZoneId.systemDefault()));|
>>>
>>> The proposed pair toEpochSecond() however is doing the other way around
>>> and
>>> with an unusual assumption of the missing date/time piece to a "default
>>> value"
>>> (midnight, the epoch day).
>>>
>>> personally I think
>>>
>>> localDate.atTime(LocalTime.MIDNIGHT).toEpochSecond(ZoneOffset);
>>> localTime.atDate(LocalDate.EPOCHDATE).toEpochSecond(ZoneOffset);
>>>
>>> is clean and good enough to help this backward conversion (with the
>>> addition
>>> of LocalDate.EPOCHDATE/DAY constant). Maybe, the vm can even help to
>>> remove that LocalDateTime middle man, with some arrangement.
>>>
>>> -Sherman
>>>
>>>> Note that these methods are specifically not referencing
>>>> java.util.Date itself. Epoch seconds is the appropriate intermediate
>>>> form here, and still widely used.
>>>>
>>>> Stephen
>>>>
>>>>
>>>>
>>>> On 30 November 2015 at 19:36, Xueming Shen<xueming.shen at oracle.com>
>>>> wrote:
>>>>> On 11/30/2015 10:37 AM, Stephen Colebourne wrote:
>>>>>> This is based on user difficulties picked up via Stack Overflow. These
>>>>>> methods aim to provide a simpler and faster approach, particularly for
>>>>>> cases converting to/from java.util.Date.
>>>>> Can you be a little more specific on this one? We now have Instance<=>
>>>>> Date,
>>>>> and considerably we might add LocalDateTime<=>  Date with an offset, if
>>>>> really
>>>>> really desired (for performance? to save a Instant object as the
>>>>> bridge?).
>>>>> But I'm
>>>>> a little confused about the connection among LocalDate/LocalTime, epoch
>>>>> seconds
>>>>> and j.u.Date here. Are you saying someone wants to convert
>>>>>
>>>>> j.t.LocalDate ->  epoch seconds ->  j.u.Date
>>>>> j.t.LocalTime ->  epoch seconds ->  j.u.Date
>>>>>
>>>>> and uses the converted j.u.Date to represent a local date (date with
>>>>> time
>>>>> part to
>>>>> be 0) and/or the local time (with year/month/day to be epoch time) in
>>>>> the
>>>>> "old"
>>>>> system which only has j.u.Date, and has to use the j.u.Date to abstract
>>>>> the
>>>>> "local
>>>>> date" and "local time"?
>>>>>
>>>>> I think we agreed back to JSR310 that we don't try to add such kind of
>>>>> "bridge/
>>>>> convenient/utility" methods into the new java.time package, but only in
>>>>> the
>>>>> old date/calendar classes, if really needed. So if these methods are
>>>>> only to
>>>>> help
>>>>> migrate/bridge between the "old" and "new" calendar systems, the
>>>>> java.time
>>>>> might not be the best place for them?
>>>>>
>>>>>> For the time cases, the convention of 1970-01-01 is natural and
>>>>>> commonly used in many codebases.
>>>>>>
>>>>> I'm not sure about that, especially the "natural" part. It might be
>>>>> "common"
>>>>> in
>>>>> the old days if you only have j.u.Date", and might be forced to use
>>>>> 1970-01-01
>>>>> to fill in the "date" part even when you are really only interested in
>>>>> "time" part
>>>>> of it in your app. One of the advantage of java.time.LDT/LD/LT is now we
>>>>> have
>>>>> separate abstract for these different need, I don't see the common need
>>>>> of
>>>>> having a LocalTime only meas the "local time" of 1970-01-01, or I
>>>>> misunderstood
>>>>> something here.
>>>>>
>>>>> -Sherman
>>>>>
>>>>>
>>>>>
>>>>>> Stephen
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 30 November 2015 at 18:15, Xueming Shen<xueming.shen at oracle.com>
>>>>>> wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> While it is kinda understandable to have LocalDate.toEpochSecond(...)
>>>>>>> to get the epoch seconds since 1970.1.1, with the assumption of the
>>>>>>> time is at the midnight of that day. It is strange to have the
>>>>>>> Local/OffsetTime
>>>>>>> to have two public methods with the assumption of the "date" is at
>>>>>>> epoch
>>>>>>> year/month/day. What's the use case/scenario for these two proposed
>>>>>>> methods?
>>>>>>>
>>>>>>> -Sherman
>>>>>>>
>>>>>>>
>>>>>>> On 11/30/2015 06:36 AM, Stephen Colebourne wrote:
>>>>>>>> The method Javadoc (specs) for each of the three new methods needs to
>>>>>>>> be enhanced.
>>>>>>>>
>>>>>>>> For the date ones it needs to say
>>>>>>>> "The resulting date will have a time component of midnight at the
>>>>>>>> start of the day."
>>>>>>>>
>>>>>>>> For the time ones it needs to say
>>>>>>>> "The resulting time will be on 1970-01-01."
>>>>>>>>
>>>>>>>> Some of the line wrapping in the tests looks like it is not indented
>>>>>>>> correctly.
>>>>>>>>
>>>>>>>> Otherwise looks fine,
>>>>>>>> thanks
>>>>>>>> Stephen
>>>>>>>>
>>>>>>>>
>>>>>>>> On 30 November 2015 at 11:50, nadeesh tv<nadeesh.tv at oracle.com>
>>>>>>>> wrote:
>>>>>>>>> Hi all,
>>>>>>>>>
>>>>>>>>> Please review a fix for
>>>>>>>>>
>>>>>>>>> Bug Id -https://bugs.openjdk.java.net/browse/JDK-8143413
>>>>>>>>>
>>>>>>>>> Issue - add toEpochSecond methods for efficient access
>>>>>>>>>
>>>>>>>>> webrev - http://cr.openjdk.java.net/~ntv/8143413/webrev.01/
>>>>>>>>>
>>>>>>>>> -- Thanks and Regards,
>>>>>>>>> Nadeesh TV
>>>>>>>
>>>>>> <div id="DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2"><table
>>>>>> style="border-top: 1px solid #aaabb6; margin-top: 10px;">
>>>>>>           <tr>
>>>>>>                   <td style="width: 105px; padding-top: 15px;">
>>>>>>                           <a
>>>>>>
>>>>>> href="https://www.avast.com/?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail"
>>>>>> target="_blank"><img
>>>>>> src="https://ipmcdn.avast.com/images/logo-avast-v1.png" style="width:
>>>>>> 90px; height:33px;"/></a>
>>>>>>                   </td>
>>>>>>                   <td style="width: 470px; padding-top: 20px; color:
>>>>>> #41424e;
>>>>>> font-size: 13px; font-family: Arial, Helvetica, sans-serif;
>>>>>> line-height: 18px;">This email has been sent from a virus-free
>>>>>> computer protected by Avast.<br /><a
>>>>>>
>>>>>>
>>>>>> href="https://www.avast.com/?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail"
>>>>>> target="_blank" style="color: #4453ea;">www.avast.com</a>
>>>>>>                   </td>
>>>>>>           </tr>
>>>>>> </table><a href="#DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2" width="1"
>>>>>> height="1"></a></div>
>>>>>




More information about the core-libs-dev mailing list