RFR (M): 8235741: Inappropriate uses of os::javaTimeMillis()

[David Holmes]

Thanks Robbin!

David

On 22/01/2020 6:54 pm, Robbin Ehn wrote:
> Hi David, yes, thanks!
> 
> /Robbin
> 
> On 1/22/20 7:56 AM, David Holmes wrote:
>> Hi Robbin,
>>
>> Can you confirm you are okay with latest changes please.
>>
>> http://cr.openjdk.java.net/~dholmes/8235741/webrev.v3/
>>
>> Thanks,
>> David
>> -----
>>
>> On 13/01/2020 11:08 pm, Robbin Ehn wrote:
>>> Hi David, looks good, thanks!
>>>
>>> /Robbin
>>>
>>> On 1/13/20 8:13 AM, David Holmes wrote:
>>>> webrev: http://cr.openjdk.java.net/~dholmes/8235741/webrev/
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8235741
>>>>
>>>> Full details in the bug report about the existing uses of 
>>>> javaTimeMillis(), many of which just want an elapsed time in ms and 
>>>> so should be using javaTimeNanos() and convert to ms. This covers 
>>>> areas all across the VM.
>>>>
>>>> Only non-simple change is in os_perf_linux.cpp (and the same code 
>>>> will be in os_perf_aix.cpp once it has been validated). There we are 
>>>> tracking an elapsed time in ms but relative to the boot time, which 
>>>> is seconds since the epoch. Consequently the first interval has to 
>>>> be calculated using javaTimeMillis, but after that we can use 
>>>> javaTimeNanos (using a new 'first time' captured at the same time we 
>>>> used javaTimeMillis). I think I have the logic right but other than 
>>>> through JFR this code seems unused and I have limited means of 
>>>> testing it. The JFR test 
>>>> jdk/jfr/event/os/TestThreadContextSwitches.java exercises the code 
>>>> but the results of running that test seems to exhibit arbitrary 
>>>> randomness in the rates reported - e.g. 0 to 16000Hz - both with and 
>>>> without my change, so not really that useful. Stefan K. suggested a 
>>>> gtest which I may look into - though it is frustrating to have to 
>>>> expend such effort to validate this.
>>>>
>>>> Other testing tiers 1-3.
>>>>
>>>> Thanks,
>>>> David