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

[Stephen Colebourne]

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.tv at 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.tv at 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.tv at 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
>