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(臧琳) <linz...@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.hol...@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.bate...@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(臧琳)" <linz...@tencent.com> >>>>> Date: Thursday, March 26, 2020 at 3:13 PM >>>>> To: "core-libs-dev@openjdk.java.net" <core-libs-dev@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 >>> > > >
