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.tv at 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.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
>>>>
>> --
>> Thanks and Regards,
>> Nadeesh TV
>>