Re: RFR: JDK-8068803:Performance of LocalDate.plusDays could be better

[Roger Riggs]

+1

I'll integrate this.

Thanks, Roger

On 1/9/2016 6:47 PM, Stephen Colebourne wrote:

I'm happy with this now, thanks
Stephen


On 9 January 2016 at 12:44, nadeesh tv <nadeesh...@oracle.com> wrote:

Hi Stephen,

It's already covered  in

  public void test_plusDays_invalidTooLarge() and public void
test_minusDays_maximum().

Thanks and Regards,
Nadeesh


On 1/9/2016 4:58 PM, Stephen Colebourne wrote:

Thanks for the update. You should have a test case for the exception
thrown by the checkValidValue()
Stephen

On 9 January 2016 at 09:33, nadeesh tv <nadeesh...@oracle.com> wrote:

Hi Stephen/Roger,

   Please see the updated the webrev
http://cr.openjdk.java.net/~ntv/8068803/webrev.03/

   Explicit "YEAR.checkValidValue(year + 1);' added in 3rd case to handle
the

invalidTooLarge year case (LocalDate.of(Year.MAX_VALUE, 12,
31).plusDays(1))

Thanks and Regards,
Nadeesh

On 1/9/2016 3:39 AM, Roger Riggs wrote:

+1

(With Stephen's update below).

Roger


On 1/8/2016 6:56 AM, Stephen Colebourne wrote:

As I mentioned in my email:

Rather than doing:
     return withDayOfMonth((int) dom);
or
     return LocalDate.of(year, month, (int) dom);
you can do
     return new LocalDate(year, month, (int) dom);

(there are two occurrences)

Stephen



On 8 January 2016 at 10:56, nadeesh tv <nadeesh...@oracle.com> wrote:

Hi all,

Thanks Stephen for the comments
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8068803/webrev.02/
Regards,
Nadeesh

On 1/7/2016 6:15 PM, Stephen Colebourne wrote:

I updated the benchmark with this change and another one:


https://github.com/ThreeTen/threeten-bench/blob/master/src/main/java/org/threeten/LocalDateBenchmark.java

Marginally fastest is this pattern as it avoids one if statement:

      if (dom > 0) {
        if (dom <= 28) { // same month
          return ...
        }
        if (dom <= 59) { // 59th Jan is 28th Feb
          return ...
        }
      }

Rather than doing:
      return LocalDate.of(year, month, (int) dom);
we can also do
      return new LocalDate(year, month, (int) dom);

This is safe, because we know that the year, month and day are valid.
(I can't easily test the performance of this change, but it should be
noticeable as it avoids a lot of unnecessary checks).

I'd like a few more test cases. Addition around 27/28/29/30 from the
first of Jan/Feb, and of 13/14/15/16 from the 15th of Jan and 15th of
Feb.

Stephen



On 7 January 2016 at 09:20, nadeesh tv <nadeesh...@oracle.com> wrote:

Hi ,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8068803/webrev.01/
Thanks and regards,
Nadeesh

On 1/6/2016 12:11 AM, Roger Riggs wrote:

Hi Nadeesh,

LocalDate.java: +1374:
      For the most common case of dom > 0 && <= 28, I would have
explicitly
and
immediately returned the new LocalDate.

         if (dom > 0 && dom <= 28) {
              return LocalDate.of(year, month, (int) dom);
         }
         ...


TCKLocalDate.java:
     - Since the test_plusDays_normal is being replaced, its test case
should be
included in the DataProvider

          {LocalDate.of(2007, 7, 15), 1, LocalDate.of(2007, 7, 16)}

Thanks, Roger

On 1/5/2016 1:05 PM, nadeesh tv wrote:

Hi all,

Please review  a  fix for
https://bugs.openjdk.java.net/browse/JDK-8068803
     web rev :   http://cr.openjdk.java.net/~ntv/8068803/webrev.00/

Special thanks for Stephen for  providing the source code patch



--
Thanks and Regards,
Nadeesh TV

--
Thanks and Regards,
Nadeesh TV



--
Thanks and Regards,
Nadeesh TV

--
Thanks and Regards,
Nadeesh TV