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(臧琳) <linz...@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....@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(臧琳) <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 >>>> >> >> >> > > > >
