Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8030864

Add an efficient getDateTimeMillis method to java.time

    Details

    • Type: Enhancement
    • Status: Resolved
    • Priority: P3
    • Resolution: Fixed
    • Affects Version/s: 9
    • Fix Version/s: 9
    • Component/s: core-libs
    • Subcomponent:
    • Resolved In Build:
      b114

      Description

      Add an efficient/fast/garbage free getDateTimeMillis method.

      That converts to epoch time without creating garbage. There doesn't seem to be an equivalent garbage free method on the new API, and this looks like an oversight.

      Conversion to epoch is important and doing it efficiently was a core reason many where using JodaTime. It would be a shame to lose it.

        Issue Links

          Activity

          rriggs Roger Riggs created issue -
          msheppar Mark Sheppard made changes -
          Field Original Value New Value
          Status New [ 10000 ] Open [ 1 ]
          msheppar Mark Sheppard made changes -
          Assignee Roger Riggs [ rriggs ]
          rriggs Roger Riggs made changes -
          Fix Version/s tbd_major [ 11972 ]
          rriggs Roger Riggs made changes -
          Fix Version/s 9 [ 14949 ]
          Fix Version/s tbd_major [ 11972 ]
          rriggs Roger Riggs made changes -
          Fix Version/s tbd_major [ 11972 ]
          Fix Version/s 9 [ 14949 ]
          Hide
          scolebourne Stephen Colebourne added a comment -
          This is related to JDK-8133079 which proposes to add 'ofInstant(Instant,ZoneId)' methods to 'LocalDate' and 'LocalTime'.

          To solve this issue
          - add 'toEpochSecond(ZoneOffset) ' methods to 'LocalDate' and 'LocalTime'.
          - add 'toEpochSecond() ' method to 'OffsetTime'.

          The method on 'LocalDate' would be defined to set the time part to midnight (safe as ZoneOffset, not ZoneId)
          The methods on the time classes would be defined to set the date part to 1970-01-01.
          Show
          scolebourne Stephen Colebourne added a comment - This is related to JDK-8133079 which proposes to add 'ofInstant(Instant,ZoneId)' methods to 'LocalDate' and 'LocalTime'. To solve this issue - add 'toEpochSecond(ZoneOffset) ' methods to 'LocalDate' and 'LocalTime'. - add 'toEpochSecond() ' method to 'OffsetTime'. The method on 'LocalDate' would be defined to set the time part to midnight (safe as ZoneOffset, not ZoneId) The methods on the time classes would be defined to set the date part to 1970-01-01.
          rriggs Roger Riggs made changes -
          Fix Version/s 9 [ 14949 ]
          Fix Version/s tbd_major [ 11972 ]
          ntv Nadeesh Tv made changes -
          Assignee Roger Riggs [ rriggs ] Nadeesh Tv [ ntv ]
          Hide
          rriggs Roger Riggs added a comment -
          The original request came from a comment in JSR 310:
             https://github.com/ThreeTen/threeten/issues/344
          Show
          rriggs Roger Riggs added a comment - The original request came from a comment in JSR 310:     https://github.com/ThreeTen/threeten/issues/344
          Hide
          rriggs Roger Riggs added a comment -
          Adding methods to LocalDate, LocalTime, and OffsetTime would not address the concern about
          creating garbage since in actual use it would require creating instances and then discarding them.
          The desired method(s) would return long and have y/m/d h,m,s,n as the arguments.
          They are not exactly factory methods since they don't produce an object and so might not
          be appropriate for LocalDate, etc.
          Show
          rriggs Roger Riggs added a comment - Adding methods to LocalDate, LocalTime, and OffsetTime would not address the concern about creating garbage since in actual use it would require creating instances and then discarding them. The desired method(s) would return long and have y/m/d h,m,s,n as the arguments. They are not exactly factory methods since they don't produce an object and so might not be appropriate for LocalDate, etc.
          Hide
          scolebourne Stephen Colebourne added a comment -
          To solve the "efficient epoch millis" problem of this issue, the following new methods are needed on `Chronology:

          // using era
          public long epochSecond(Era, int, int, int, int, int, int, ZoneId);
          // using proleptic year
          public long epochSecond(int, int, int, int, int, int, ZoneId);

          These can be implemented on the interface using default methods by calling `date(Era, int, int, int)` and `date(int, int, int)` and manipulating the result to add the time and offset. This could then be overridden in IsoChronology to add a garbage-free fast path.

          While methods could also be provided for epochMilli(), that seems overkill (java.time focusses on epoch seconds)

          Do you want to create a separate issue for the `toEpochSecond` changes that need to go in to match JDK-8133079?

          Show
          scolebourne Stephen Colebourne added a comment - To solve the "efficient epoch millis" problem of this issue, the following new methods are needed on `Chronology: // using era public long epochSecond(Era, int, int, int, int, int, int, ZoneId); // using proleptic year public long epochSecond(int, int, int, int, int, int, ZoneId); These can be implemented on the interface using default methods by calling `date(Era, int, int, int)` and `date(int, int, int)` and manipulating the result to add the time and offset. This could then be overridden in IsoChronology to add a garbage-free fast path. While methods could also be provided for epochMilli(), that seems overkill (java.time focusses on epoch seconds) Do you want to create a separate issue for the `toEpochSecond` changes that need to go in to match JDK-8133079 ?
          ntv Nadeesh Tv made changes -
          Assignee Nadeesh Tv [ ntv ]
          rriggs Roger Riggs made changes -
          Link This issue relates to JDK-8143413 [ JDK-8143413 ]
          rriggs Roger Riggs made changes -
          Assignee Roger Riggs [ rriggs ]
          ntv Nadeesh Tv made changes -
          Assignee Roger Riggs [ rriggs ] Nadeesh Tv [ ntv ]
          Hide
          ntv Nadeesh Tv added a comment - - edited

          Stephen/Roger -

          According to ChronoLocalDate.toEpochDay() 1970-01-01 is the epochday for all chronologies.

          From Era interface
          "The era in use at the epoch 1970-01-01 (ISO) has the value 1".

          Does it mean that in all chronology, Era will have the value 1 for EpochDay?

          Then do we really need epochSecond(Era, int, int, int, int, int, int, ZoneOffset);?

          Is not public long epochSecond(int yaer, int, int, int, int, int, ZoneOffset); sufficient?
          Show
          ntv Nadeesh Tv added a comment - - edited Stephen/Roger - According to ChronoLocalDate.toEpochDay() 1970-01-01 is the epochday for all chronologies. From Era interface "The era in use at the epoch 1970-01-01 (ISO) has the value 1". Does it mean that in all chronology, Era will have the value 1 for EpochDay? Then do we really need epochSecond(Era, int, int, int, int, int, int, ZoneOffset);? Is not public long epochSecond(int yaer, int, int, int, int, int, ZoneOffset); sufficient?
          Hide
          rriggs Roger Riggs added a comment -
          The Era.getValue() must be 1 for 1970-01-01.
          The year in epochSecond(Era, int, int, int, int, int, int, ZoneOffset) is offset from the base of the Era.
          The Japanese calendar makes the most significant use of Era.

          See java.time.chrono.Chronology.date to see the implementation differernces between the two methods.
          Show
          rriggs Roger Riggs added a comment - The Era.getValue() must be 1 for 1970-01-01. The year in epochSecond(Era, int, int, int, int, int, int, ZoneOffset) is offset from the base of the Era. The Japanese calendar makes the most significant use of Era. See java.time.chrono.Chronology.date to see the implementation differernces between the two methods.
          ntv Nadeesh Tv made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Understanding Fix Understood [ 10001 ]
          Hide
          ntv Nadeesh Tv added a comment -
          Stephen/Roger
          Some methods in IsoChronlogy like
          date(Era era, int yearOfEra, int month, int dayOfMonth)
          dateYearDay(Era era, int yearOfEra, int dayOfYear)
          prolepticYear(Era era, int yearOfEra) accepting Era as a parameter . Can we change it to more concrete IsoEra?

          Even though it may not give much improvement in performance atleast we could avoid the unnecessary instance check in prolepticYear()


          Can I use IsoEra as parameter in Isochronology.epochSecond(IsoEra, int, int, int, int, int, int, ZoneOffset) instead of Era?
          Show
          ntv Nadeesh Tv added a comment - Stephen/Roger Some methods in IsoChronlogy like date(Era era, int yearOfEra, int month, int dayOfMonth) dateYearDay(Era era, int yearOfEra, int dayOfYear) prolepticYear(Era era, int yearOfEra) accepting Era as a parameter . Can we change it to more concrete IsoEra? Even though it may not give much improvement in performance atleast we could avoid the unnecessary instance check in prolepticYear() Can I use IsoEra as parameter in Isochronology.epochSecond(IsoEra, int, int, int, int, int, int, ZoneOffset) instead of Era?
          Hide
          scolebourne Stephen Colebourne added a comment -
          Two new default methods are on `Chronology`, so `Era` is the correct input:

          // untested, but probably close to correct, may need to use Math.addExact/multiplyExact
          public default long epochSecond(Era era, int year, int month, int dayOfMonth, int hour, int minute, int second, ZoneOffset offset) {
            long epochDay = IsoChronology.INSTANCE.date(era, year, month, dayOfMonth).toEpochDay();
            long time = (((hour * 60) + minute) * 60) + second;
            return epochDay * 86400 + time + offset.getTotalSeconds();
          }
          public default long epochSecond(int, int, int, int, int, int, ZoneOffset) {
            // similar to above
          }

          This default implementation will do for all calendar systems except IsoChronology, where the default method should be overridden to avoid creating a LocalDate on the first line.
          Show
          scolebourne Stephen Colebourne added a comment - Two new default methods are on `Chronology`, so `Era` is the correct input: // untested, but probably close to correct, may need to use Math.addExact/multiplyExact public default long epochSecond(Era era, int year, int month, int dayOfMonth, int hour, int minute, int second, ZoneOffset offset) {   long epochDay = IsoChronology.INSTANCE.date(era, year, month, dayOfMonth).toEpochDay();   long time = (((hour * 60) + minute) * 60) + second;   return epochDay * 86400 + time + offset.getTotalSeconds(); } public default long epochSecond(int, int, int, int, int, int, ZoneOffset) {   // similar to above } This default implementation will do for all calendar systems except IsoChronology, where the default method should be overridden to avoid creating a LocalDate on the first line.
          Show
          ntv Nadeesh Tv added a comment - http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-March/039218.html
          dbessono Dmitry Bessonov made changes -
          Link This issue relates to JCK-7302970 [ JCK-7302970 ]
          Hide
          hgupdate HG Updates added a comment -
          URL: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/a738394080a3
          User: ntv
          Date: 2016-04-06 07:42:14 +0000
          Show
          hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/a738394080a3 User: ntv Date: 2016-04-06 07:42:14 +0000
          hgupdate HG Updates made changes -
          Status In Progress [ 3 ] Resolved [ 5 ]
          Resolved In Build team [ 17324 ]
          Understanding Fix Understood [ 10001 ]
          Resolution Fixed [ 1 ]
          Hide
          hgupdate HG Updates added a comment -
          URL: http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/a738394080a3
          User: lana
          Date: 2016-04-13 17:56:05 +0000
          Show
          hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/a738394080a3 User: lana Date: 2016-04-13 17:56:05 +0000
          hgupdate HG Updates made changes -
          Resolved In Build team [ 17324 ] master [ 18256 ]
          hgupdate HG Updates made changes -
          Resolved In Build master [ 18256 ] b114 [ 17713 ]
          iris Iris Clark made changes -
          Labels jsr310 jsr310 jsr379-annex2-tbd
          iris Iris Clark made changes -
          Labels jsr310 jsr379-annex2-tbd jsr310 jsr379-annex1

            People

            • Assignee:
              ntv Nadeesh Tv
              Reporter:
              rriggs Roger Riggs
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: