RFR(S): 8241638: launcher time metrics alway report 1 on Linux when _JAVA_LAUNCHER_DEBUG set(Internet mail)

[Henry Jen]

Yes, we need an official Reviewer to approve. I can help to push the change after that.

Cheers,
Henry



> On Mar 31, 2020, at 9:55 PM, linzang(臧琳) <linzang at tencent.com> wrote:
> 
> Thanks Henry, 
> 
> I agree to leave Solaris as it is, it seems to me there is no strong reason to remove it.
> 
> So, Do I need more review before push this patch? 
> 
> And may I ask your help to push it if a go decision is made.
> 
> Thanks.
> 
> 
> BRs,
> Lin
> 
> On 2020/3/31, 12:30 PM, "Henry Jen" <henry.jen at oracle.com> wrote:
> 
>    OK, I agree with David gettimeofday is an improvement than 1, so we can go ahead with the patch. Not sure if the caveat should be disclosed or not, I don’t think it’s important enough, just a known fact probably worth to leave a trace(perhaps a comment in code or the bug entry). As whether to change the ifdef to simply detect Solaris, I prefer just to leave it as is for two reasons:
> 
>    1. It’s not broken, why change it?
>    2. I checked the code, it’s just a simply macro defined by the make file. Realtime Linux extension would have that function and special custom build can still use that, even though that probably is not happening.
> 
>    Cheers,
>    Henry
> 
> 
>> On Mar 30, 2020, at 7:39 PM, linzang(臧琳) <linzang at tencent.com> wrote:
>> 
>> Hi David, Henry, Alan,
>>   Thanks a lot for your review. 
>>   I have considered use clock_gettime() first, but I seached the code and found there is a marco SUPPORTS_CLOCK_MONOTONIC guard the usage of it. Which makes me think it may not be available under specific situation. So I choosed gettimeofday.  
>>  Do you think the patch need to be refined to remove HAVE_GETHRTIME as Alan suggested?  Thanks. 
>> 
>> BRs,
>> Lin
>> 
>>> On 2020/3/31, 8:05 AM, "David Holmes" <david.holmes at oracle.com> wrote:
>>> 
>>>  On 31/03/2020 4:08 am, Henry Jen wrote:
>>>> Based on my understanding to gethrtime(), the main benefit is not to be affected by settimeofday or adjtime. I think it is probably better to use
>>>> 
>>>> clock_gettime(CLOCK_MONOTONIC_RAW, ts);
>>>> 
>>>> which I checked seems to be available on both Linux and Mac. Haven’t test it though.
>>> 
>>>  Not guaranteed to be available - either clock_gettime function or that 
>>>  particular clock - at build time or runtime. We use a check in the build 
>>>  system to determine build-time availability for hotspot, and then use 
>>>  dl_lookup etc at runtime to determine if actually available. We should 
>>>  be able to get rid of this one day but we checked fairly recently and 
>>>  there were still some issues.
>> 
>>>  gettimeofday is a lot better than returning 1. Otherwise call into the 
>>>  VM and use JVM_NanoTime.
>>> 
>>>  Cheers,
>>>  David
>>>  -----
>>> 
>>>> Cheers,
>>>> Henry
>>>> 
>>>>> On Mar 30, 2020, at 1:37 AM, Alan Bateman <Alan.Bateman at oracle.com> wrote:
>>>>> 
>>>>> On 30/03/2020 03:41, linzang(臧琳) wrote:
>>>>>> Dear All,
>>>>>>     May I ask your help to reivew this tiny patch? Thanks.
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> BRs,
>>>>>> Lin
>>>>>> 
>>>>>> From: "linzang(臧琳)" <linzang at tencent.com>
>>>>>> Date: Thursday, March 26, 2020 at 3:13 PM
>>>>>> To: "core-libs-dev at openjdk.java.net" <core-libs-dev at openjdk.java.net>
>>>>>> Subject: RFR(S): 8241638: launcher time metrics alway report 1 on Linux when _JAVA_LAUNCHER_DEBUG set
>>>>>> 
>>>>>> Dear All,
>>>>>> May I ask your help to review this tiny fix?
>>>>>>    Bug: https://bugs.openjdk.java.net/browse/JDK-8241638
>>>>>>    Webrev: http://cr.openjdk.java.net/~lzang/8241638/webrev01/
>>>>>> Thanks!
>>>>>> 
>>>>> Using gettimeofday on non-Solaris platforms seems reasonable here. The comment in the patch suggests Linux but it's other Unix builds too. Also just a minor nit that the code in java.base uses 4-space indent, not 2. Looking at the patch makes me wondering if we should remove HAVE_GETHRTIME as it seems to be only used on Solaris >and the launcher is already using #ifdef __solaris__ in several places. Henry, do you have any comments on this?
>>>>> 
>>>>> -Alan
>>>> 
>> 
>> 
>> 
> 
> 
> 
>