RFR: 8222029: Optimize Math.floorMod

Claes Redestad claes.redestad at oracle.com
Wed Apr 10 10:27:42 UTC 2019

I found a few missing corner cases in the existing test (all
permutations of [MIN|MAX]/[MIN|MAX]), so I added them and verified
the test pass both with and without my patch:



On 2019-04-09 21:06, Claes Redestad wrote:
> In my cursory analysis of the test the interesting corner cases are
> covered and I have no reason to believe we'd see spurious errors
> elsewhere. I ran an adhoc semi-exhaustive test comparing results
> of the new version with the old and got an all-pass.
> /Claes
> On 2019-04-09 18:02, Joe Darcy wrote:
>> Basically I'm inquiring about whether the existing tests provide at 
>> least as good code coverage on the new implementation as the old one. 
>> As it is a relatively simple method, perhaps it there is full coverage 
>> before and after. However, at times changing the implementation 
>> requires updates to the tests to includes different cases to check and 
>> I wanted to make sure that was looked at here.
>> Thanks,
>> -Joe
>> On 4/9/2019 5:32 AM, Claes Redestad wrote:
>>> I think those tests cover all interesting corner cases, so the only way
>>> I see it can be improved is to make it more exhaustive (say generate a
>>> large random sample of tests every run). Do you feel that is needed?
>>> /Claes
>>> On 2019-04-09 01:35, Joseph D. Darcy wrote:
>>>> Should any additional cases be added to 
>>>> test/jdk/java/lang/Math/DivModTests.java to cover the new 
>>>> implementation?
>>>> Thanks,
>>>> -Joe
>>>> On 4/5/2019 10:21 AM, Claes Redestad wrote:
>>>>> On 2019-04-05 17:41, Andrew Haley wrote:
>>>>>> On 4/5/19 2:44 PM, Claes Redestad wrote:
>>>>>>> Testing: tier1-2, all Math tests run locally, -prof perfasm 
>>>>>>> verification
>>>>>>> on the provided microbenchmark.
>>>>>> Looks good.
>>>>> Thanks!
>>>>>> I've kicked the tyres on AArch64, and it looks like a useful 
>>>>>> optimization. The
>>>>>> gains when the divisor is constant (a common case) are modest but 
>>>>>> worthwhile.
>>>>> Thanks for trying it out and glad to hear it helps on AArch64 as well.
>>>>> /Claes

More information about the core-libs-dev mailing list