RFR:8143413:add toEpochSecond methods for efficient access

nadeesh tv nadeesh.tv at oracle.com
Tue Dec 22 07:59:27 UTC 2015


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