RFR:8143413:add toEpochSecond methods for efficient access
Roger Riggs
Roger.Riggs at Oracle.com
Wed Dec 23 18:19:19 UTC 2015
+1
Roger
On 12/22/2015 9:53 AM, Stephen Colebourne wrote:
> 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