RFR:8143413:add toEpochSecond methods for efficient access
nadeesh tv
nadeesh.tv at oracle.com
Thu Dec 17 18:13:54 UTC 2015
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
More information about the core-libs-dev
mailing list