RFR:8143413:add toEpochSecond methods for efficient access
Stephen Colebourne
scolebourne at joda.org
Tue Dec 22 14:53:29 UTC 2015
Fine by me, thanks
Stephen
On 22 December 2015 at 07:59, nadeesh tv <nadeesh.tv at oracle.com> wrote:
> Hi all,
>
> Please see the updated webrev
> http://cr.openjdk.java.net/~ntv/8143413/webrev.04/
>
> Changes : Included the changes suggested by Stephen
>
> Thanks and Regards,
> Nadeesh
>
>
> On 12/22/2015 12:30 AM, Stephen Colebourne wrote:
>>
>> The comment for the new LocalDate.EPOCH field should use 1970-01-01,
>> not 1970-1-1. However, the code should omit the zeroes:
>> Replace:
>> LocalDate.of(1970, 01, 01)
>> with
>> LocalDate.of(1970, 1, 1)
>>
>> The new method should follow the documentation of the similar method
>> on ChronoLocalDateTime:
>>
>> * This combines this local date with the specified time and
>> offset to calculate the
>> * epoch-second value, which is the number of elapsed seconds from
>> 1970-01-01T00:00:00Z.
>> * Instants on the time-line after the epoch are positive, earlier
>> are negative.
>>
>> The same applies to the new method on LocalTime:
>>
>> * This combines this local time with the specified date and
>> offset to calculate the
>> * epoch-second value, which is the number of elapsed seconds from
>> 1970-01-01T00:00:00Z.
>> * Instants on the time-line after the epoch are positive, earlier
>> are negative.
>>
>> The same applies to the new method on OffsetTime:
>>
>> * This combines this offset time with the specified date to
>> calculate the
>> * epoch-second value, which is the number of elapsed seconds from
>> 1970-01-01T00:00:00Z.
>> * Instants on the time-line after the epoch are positive, earlier
>> are negative.
>>
>> The test cases look good.
>>
>> thanks
>> Stephen
>>
>>
>> On 17 December 2015 at 18:13, nadeesh tv <nadeesh.tv at oracle.com> wrote:
>>>
>>> Hi all,
>>>
>>> Please see the updated webrev
>>> http://cr.openjdk.java.net/~ntv/8143413/webrev.03/
>>>
>>> Thanks and Regards,
>>> Nadeesh
>>>
>>> On 12/4/2015 3:56 AM, Stephen Colebourne wrote:
>>>>
>>>> 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>
>>>
>>>
>>> --
>>> Thanks and Regards,
>>> Nadeesh TV
>>>
>
> --
> Thanks and Regards,
> Nadeesh TV
>
More information about the core-libs-dev
mailing list