RFR:8143413:add toEpochSecond methods for efficient access
Stephen Colebourne
scolebourne at joda.org
Thu Dec 3 22:26:56 UTC 2015
In the interests of harmony and getting something in, I'll accept that.
I think LocalDate.EPOCH is probably better than LocalDate.EPOCHDAY
Stephen
On 3 December 2015 at 22:09, Roger Riggs <Roger.Riggs at oracle.com> wrote:
> Hi Sherman,
>
> It makes sense to me to provide the missing time/date as an explicit
> parameter
> for toEpochSeconds instead of assuming some constant.
>
> localDate.toEpochSeconds(LocalTime.MIDNIGHT, ZoneOffset.UTC);
> localTime.toEpochSeconds(LocalDate.EPOCHDAY, ZoneOffset.UTC);
> offsetTime.toEpochSeconds(LocalDate.EPOCHDAY);
>
> We'll have to add the LocalDate.EPOCHDAY constant.
>
> It makes it a bit heavier weight to use but can still be optimized and won't
> create garbage.
>
> Roger
>
>
>
> On 12/01/2015 12:33 PM, Xueming Shen wrote:
>>
>> 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