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. Cheers, Henry

On Mar 30, 2020, at 1:37 AM, Alan Bateman <Alan.Bateman@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@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

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@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@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@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

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@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@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@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@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@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 >>> > > >

On 01/04/2020 18:47, Henry Jen wrote:

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

I looked at the webrev01 but I don't think Lin has published the updated version yet. Happy to Review. -Alan

Hi Lin, On 31/03/2020 12:39 pm, linzang(臧琳) 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.

Leaving it as-is seems okay. Though the likelihood of anyone taking advantage of an existing gethrtime() function anywhere other than Solaris seems near zero. But if the Solaris port deprecation goes ahead the point will be moot as only the "linux" version will remain. This comment block needs fixing up: 37 /* 38 * * Add gethrtime() implementation for launch time debug on Linux. 39 * */ Also a nit but the new function can be called whatever you like as you are defining CounterGet, so calling it gethrtime() is a bit misleading - I suggest getTimeMicros(). Thanks, David

BRs, Lin

>On 2020/3/31, 8:05 AM, "David Holmes" <david.holmes@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@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@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

On 2/04/2020 6:48 pm, linzang(臧琳) wrote:

Hi David, Thanks to point it out, I have uploaded a new patch at http://cr.openjdk.java.net/~lzang/8241638/webrev03/

As Alan pointed out this definition is not just for Linux. I would suggest this comment block: ! * Add CounterGet() implementation for launch time debug on Linux. ! * Use gettimeofday() here since the gethrtime() or clock_gettime() ! * may not be available for all Linux platforms. ! * The potential risk of using gettimeofday() is it can be affected ! * by settimeofday() or adjtime(). ! * Choose gettimeofday() here because it is more common on linux and ! * it is better than just return magic numbers for launch time debug. becomes simply: * Provide a CounterGet() implementation based on gettimeofday() which * is universally available, even though it may not be 'high resolution' * compared to platforms that provide gethrtime() (like Solaris). It is * also subject to time-of-day changes, but alternatives may not be * known to be available at either build time or run time. No need for updated webrev. Thanks, David

BRs, Lin

On 2020/4/2, 2:54 PM, "David Holmes" <david.holmes@oracle.com> wrote:

Hi Lin,

On 31/03/2020 12:39 pm, linzang(臧琳) 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.

Leaving it as-is seems okay. Though the likelihood of anyone taking advantage of an existing gethrtime() function anywhere other than Solaris seems near zero. But if the Solaris port deprecation goes ahead the point will be moot as only the "linux" version will remain.

This comment block needs fixing up:

37 /* 38 * * Add gethrtime() implementation for launch time debug on Linux. 39 * */

Also a nit but the new function can be called whatever you like as you are defining CounterGet, so calling it gethrtime() is a bit misleading - I suggest getTimeMicros().

Thanks, David

> BRs, > Lin > > >On 2020/3/31, 8:05 AM, "David Holmes" <david.holmes@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@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@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 > > > > > >

...

No need for updated webrev.

P.S. I mode the webrev update so it is easy for me to record the changes 😝 Thanks! Lin On 2020/4/2, 6:21 PM, "linzang(臧琳)" <linzang@tencent.com> wrote: Dear David, Here is the updated webrev : http://cr.openjdk.java.net/~lzang/8241638/webrev04/ Thanks for your help! BRs, Lin On 2020/4/2, 6:06 PM, "David Holmes" <david.holmes@oracle.com> wrote: On 2/04/2020 6:48 pm, linzang(臧琳) wrote: > Hi David, > Thanks to point it out, I have uploaded a new patch at http://cr.openjdk.java.net/~lzang/8241638/webrev03/ As Alan pointed out this definition is not just for Linux. I would suggest this comment block: ! * Add CounterGet() implementation for launch time debug on Linux. ! * Use gettimeofday() here since the gethrtime() or clock_gettime() ! * may not be available for all Linux platforms. ! * The potential risk of using gettimeofday() is it can be affected ! * by settimeofday() or adjtime(). ! * Choose gettimeofday() here because it is more common on linux and ! * it is better than just return magic numbers for launch time debug. becomes simply: * Provide a CounterGet() implementation based on gettimeofday() which * is universally available, even though it may not be 'high resolution' * compared to platforms that provide gethrtime() (like Solaris). It is * also subject to time-of-day changes, but alternatives may not be * known to be available at either build time or run time. No need for updated webrev. Thanks, David > > BRs, > Lin > > On 2020/4/2, 2:54 PM, "David Holmes" <david.holmes@oracle.com> wrote: > > Hi Lin, > > On 31/03/2020 12:39 pm, linzang(臧琳) 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. > > Leaving it as-is seems okay. Though the likelihood of anyone taking > advantage of an existing gethrtime() function anywhere other than > Solaris seems near zero. But if the Solaris port deprecation goes ahead > the point will be moot as only the "linux" version will remain. > > This comment block needs fixing up: > > 37 /* > 38 * * Add gethrtime() implementation for launch time debug on Linux. > 39 * */ > > Also a nit but the new function can be called whatever you like as you > are defining CounterGet, so calling it gethrtime() is a bit misleading - > I suggest getTimeMicros(). > > Thanks, > David > > > BRs, > > Lin > > > > >On 2020/3/31, 8:05 AM, "David Holmes" <david.holmes@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@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@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 > > > > > > > > > > > > >

Internal test shows that inline implementation is not working for some Solaris artifacts, because the -DHAVE_GETHRTIME is not consistently defined, so it is actually broken. :)

[2020-04-03T15:59:26,981Z] Creating support/test/hotspot/jtreg/native/bin/jvm-test-launcher from 1 file(s) [2020-04-03T16:02:10,984Z] [2020-04-03T16:02:10,984Z] ERROR: Build failed for target 'default (product-bundles test-bundles static-libs-bundles)' in configuration 'solaris-sparcv9-open' (exit code 2) [2020-04-03T16:02:11,051Z] [2020-04-03T16:02:11,051Z] === Output from failing command(s) repeated here === [2020-04-03T16:02:11,055Z] * For target support_native_java.base_libjli_BUILD_LIBJLI_link: [2020-04-03T16:02:11,061Z] Undefined first referenced [2020-04-03T16:02:11,061Z] symbol in file [2020-04-03T16:02:11,061Z] getTimeMicros /export/home/opt/mach5/mesos/work_dir/e101deec-9613-4d46-a64d-559e689496aa/workspace/build/solaris-sparcv9-open/support/native/java.base/libjli/java.o [2020-04-03T16:02:11,061Z] ld: fatal: symbol referencing errors [2020-04-03T16:02:11,082Z] [2020-04-03T16:02:11,082Z] * All command lines available in /export/home/opt/mach5/mesos/work_dir/e101deec-9613-4d46-a64d-559e689496aa/workspace/build/solaris-sparcv9-open/make-support/failure-logs. [2020-04-03T16:02:11,086Z] === End of repeated output === [2020-04-03T16:02:11,094Z] [2020-04-03T16:02:11,094Z] === Make failed targets repeated here === [2020-04-03T16:02:11,736Z] CoreLibraries.gmk:206: recipe for target '/export/home/opt/mach5/mesos/work_dir/e101deec-9613-4d46-a64d-559e689496aa/workspace/build/solaris-sparcv9-open/support/modules_libs/java.base/libjli.so' failed [2020-04-03T16:02:11,737Z] make/Main.gmk:195: recipe for target 'java.base-libs' failed [2020-04-03T16:02:11,739Z] === End of repeated output === [2020-04-03T16:02:11,741Z]

I verified that either move implementation into .c as a function body[1] or change to #ifdef __solaris__[2] will fix that. So I think we will change to detect __solaris__ as webrev[2] rather than have an extra #define. If some other build want to have that, they can be modify that #ifdef easily. As I look into it, I found Mac have similar implementation with minor mistake, so I fixed that as well. Please review following based on zhanglin’s patch. I’ll push [2] once I got a +1. [1] http://cr.openjdk.java.net/~henryjen/jdk/8241638.0/webrev/ [2] http://cr.openjdk.java.net/~henryjen/jdk/8241638.1/webrev/ Cheers, Henry

On Apr 2, 2020, at 6:17 AM, Alan Bateman <Alan.Bateman@oracle.com> wrote:

On 02/04/2020 11:26, linzang(臧琳) wrote:

...

: Here is the updated webrev : http://cr.openjdk.java.net/~lzang/8241638/webrev04/ Thanks for your help!

webrev04 looks good. My preference was to was to replace #ifdef HAVE_GETHRTIME with #ifdef __solaris__ but there doesn't seem to be appetite to do this now. I think Henry has offered to help sponsor.

-Alan.

On 4/04/2020 3:13 pm, Henry Jen wrote:

Internal test shows that inline implementation is not working for some Solaris artifacts, because the -DHAVE_GETHRTIME is not consistently defined, so it is actually broken. :)

The problem is in defining that function as inline rather than the -DHAVE_GETHRTIME.

...

[2020-04-03T15:59:26,981Z] Creating support/test/hotspot/jtreg/native/bin/jvm-test-launcher from 1 file(s)

The build rules for this special test launcher need to be examined as something seems wrong to me. David -----

...

[2020-04-03T16:02:10,984Z] [2020-04-03T16:02:10,984Z] ERROR: Build failed for target 'default (product-bundles test-bundles static-libs-bundles)' in configuration 'solaris-sparcv9-open' (exit code 2) [2020-04-03T16:02:11,051Z] [2020-04-03T16:02:11,051Z] === Output from failing command(s) repeated here === [2020-04-03T16:02:11,055Z] * For target support_native_java.base_libjli_BUILD_LIBJLI_link: [2020-04-03T16:02:11,061Z] Undefined first referenced [2020-04-03T16:02:11,061Z] symbol in file [2020-04-03T16:02:11,061Z] getTimeMicros /export/home/opt/mach5/mesos/work_dir/e101deec-9613-4d46-a64d-559e689496aa/workspace/build/solaris-sparcv9-open/support/native/java.base/libjli/java.o [2020-04-03T16:02:11,061Z] ld: fatal: symbol referencing errors [2020-04-03T16:02:11,082Z] [2020-04-03T16:02:11,082Z] * All command lines available in /export/home/opt/mach5/mesos/work_dir/e101deec-9613-4d46-a64d-559e689496aa/workspace/build/solaris-sparcv9-open/make-support/failure-logs. [2020-04-03T16:02:11,086Z] === End of repeated output === [2020-04-03T16:02:11,094Z] [2020-04-03T16:02:11,094Z] === Make failed targets repeated here === [2020-04-03T16:02:11,736Z] CoreLibraries.gmk:206: recipe for target '/export/home/opt/mach5/mesos/work_dir/e101deec-9613-4d46-a64d-559e689496aa/workspace/build/solaris-sparcv9-open/support/modules_libs/java.base/libjli.so' failed [2020-04-03T16:02:11,737Z] make/Main.gmk:195: recipe for target 'java.base-libs' failed [2020-04-03T16:02:11,739Z] === End of repeated output === [2020-04-03T16:02:11,741Z]

I verified that either move implementation into .c as a function body[1] or change to #ifdef __solaris__[2] will fix that. So I think we will change to detect __solaris__ as webrev[2] rather than have an extra #define. If some other build want to have that, they can be modify that #ifdef easily.

As I look into it, I found Mac have similar implementation with minor mistake, so I fixed that as well. Please review following based on zhanglin’s patch.

I’ll push [2] once I got a +1.

[1] http://cr.openjdk.java.net/~henryjen/jdk/8241638.0/webrev/ [2] http://cr.openjdk.java.net/~henryjen/jdk/8241638.1/webrev/

Cheers, Henry

...

On Apr 2, 2020, at 6:17 AM, Alan Bateman <Alan.Bateman@oracle.com> wrote:

On 02/04/2020 11:26, linzang(臧琳) wrote:

...

: Here is the updated webrev : http://cr.openjdk.java.net/~lzang/8241638/webrev04/ Thanks for your help!

webrev04 looks good. My preference was to was to replace #ifdef HAVE_GETHRTIME with #ifdef __solaris__ but there doesn't seem to be appetite to do this now. I think Henry has offered to help sponsor.

-Alan.

On Apr 5, 2020, at 6:52 AM, Alan Bateman <Alan.Bateman@oracle.com> wrote:

On 05/04/2020 14:17, David Holmes wrote:

...

On 4/04/2020 3:13 pm, Henry Jen wrote:

...

Internal test shows that inline implementation is not working for some Solaris artifacts, because the -DHAVE_GETHRTIME is not consistently defined, so it is actually broken. :)

The problem is in defining that function as inline rather than the -DHAVE_GETHRTIME.

...

...

[2020-04-03T15:59:26,981Z] Creating support/test/hotspot/jtreg/native/bin/jvm-test-launcher from 1 file(s)

The build rules for this special test launcher need to be examined as something seems wrong to me. I assume it's just that it's just compiled differently to the regular launchers and just not noticed that it was missing -DHAVE_GETHRTIME (unless for anyone to use it with _JAVA_LAUNCHER_DEBUG set).

This is my understanding as well, we built something without define -DHAVA_GETHRTIME but linked with the library that was built with that defined and use inline function. We won’t notice this before because CounterGet is no-op before.

If we go with Henry's second webrev then I assume -DHAVE_GETHRTIME can be dropped from LauncherCommon.gmk to avoid confusing any further maintainers in this area.

Right, I can drop that as well.

Also is there a strong need for the non-Solaris getTimeMicros to have be inline?

Certainly we can change to not inline by combining both fixes. I don’t think inline is necessary causing problem but that means header and the binary does need to match. Cheers, Henry

On 6/04/2020 10:53 am, linzang(臧琳) wrote:

...

Certainly we can change to not inline by combining both fixes. I don’t think inline is necessary causing problem but that means header and the binary does need to match.

How about use macro instead of function?

No that's not appropriate use of a macro. This is a full function, if inlining may be problematic (and I agree the inlining may not be a problem afterall) then it should declared in the header file and defined in java_md_solinux.c David

+#define getTimeMicros() ({ \ + uint64_t result = 0; \ + struct timeval tv; \ + if (gettimeofday(&tv, NULL) != -1) { \ + result = 1000000LL * (uint64_t)tv.tv_sec; \ + result += (uint64_t)tv.tv_usec; \ + } \ + result; \ +})

I have tested it works on linux, not sure whether it is ok for solaris.

BRs, Lin

On 2020/4/6, 3:37 AM, "Henry Jen" <henry.jen@oracle.com> wrote:

> On Apr 5, 2020, at 6:52 AM, Alan Bateman <Alan.Bateman@oracle.com> wrote: > > > On 05/04/2020 14:17, David Holmes wrote: >> On 4/04/2020 3:13 pm, Henry Jen wrote: >>> Internal test shows that inline implementation is not working for some Solaris artifacts, because the -DHAVE_GETHRTIME is not consistently defined, so it is actually broken. :) >> >> The problem is in defining that function as inline rather than the -DHAVE_GETHRTIME. >> >>>> [2020-04-03T15:59:26,981Z] Creating support/test/hotspot/jtreg/native/bin/jvm-test-launcher from 1 file(s) >> >> The build rules for this special test launcher need to be examined as something seems wrong to me. > I assume it's just that it's just compiled differently to the regular launchers and just not noticed that it was missing -DHAVE_GETHRTIME (unless for anyone to use it with _JAVA_LAUNCHER_DEBUG set).

This is my understanding as well, we built something without define -DHAVA_GETHRTIME but linked with the library that was built with that defined and use inline function. We won’t notice this before because CounterGet is no-op before.

> If we go with Henry's second webrev then I assume -DHAVE_GETHRTIME can be dropped from LauncherCommon.gmk to avoid confusing any further maintainers in this area.

Right, I can drop that as well.

> Also is there a strong need for the non-Solaris getTimeMicros to have be inline? >

Certainly we can change to not inline by combining both fixes. I don’t think inline is necessary causing problem but that means header and the binary does need to match.

Cheers, Henry

There is something not right here ... On 4/04/2020 3:13 pm, Henry Jen wrote:

Internal test shows that inline implementation is not working for some Solaris artifacts, because the -DHAVE_GETHRTIME is not consistently defined, so it is actually broken. :)

...

[2020-04-03T15:59:26,981Z] Creating support/test/hotspot/jtreg/native/bin/jvm-test-launcher from 1 file(s)

Are you sure that line actually pertains to any error? The test defines a custom launcher which doesn't use libjli so should never be including the header file we are discussing.

...

[2020-04-03T16:02:10,984Z] [2020-04-03T16:02:10,984Z] ERROR: Build failed for target 'default (product-bundles test-bundles static-libs-bundles)' in configuration 'solaris-sparcv9-open' (exit code 2) [2020-04-03T16:02:11,051Z] [2020-04-03T16:02:11,051Z] === Output from failing command(s) repeated here === [2020-04-03T16:02:11,055Z] * For target support_native_java.base_libjli_BUILD_LIBJLI_link: [2020-04-03T16:02:11,061Z] Undefined first referenced [2020-04-03T16:02:11,061Z] symbol in file [2020-04-03T16:02:11,061Z] getTimeMicros /export/home/opt/mach5/mesos/work_dir/e101deec-9613-4d46-a64d-559e689496aa/workspace/build/solaris-sparcv9-open/support/native/java.base/libjli/java.o [2020-04-03T16:02:11,061Z] ld: fatal: symbol referencing errors

This looks like a direct linkage error. AFAICS ./launcher/LauncherCommon.gmk only defines -DHAVE_GETHRTIME for launcher executables. But if we are building libjli it is not an executable. I'm suspecting there is actually a long standing build bug here from when libjli was introduced. Possibly only evident on an incremental build. David -----

...

[2020-04-03T16:02:11,082Z] [2020-04-03T16:02:11,082Z] * All command lines available in /export/home/opt/mach5/mesos/work_dir/e101deec-9613-4d46-a64d-559e689496aa/workspace/build/solaris-sparcv9-open/make-support/failure-logs. [2020-04-03T16:02:11,086Z] === End of repeated output === [2020-04-03T16:02:11,094Z] [2020-04-03T16:02:11,094Z] === Make failed targets repeated here === [2020-04-03T16:02:11,736Z] CoreLibraries.gmk:206: recipe for target '/export/home/opt/mach5/mesos/work_dir/e101deec-9613-4d46-a64d-559e689496aa/workspace/build/solaris-sparcv9-open/support/modules_libs/java.base/libjli.so' failed [2020-04-03T16:02:11,737Z] make/Main.gmk:195: recipe for target 'java.base-libs' failed [2020-04-03T16:02:11,739Z] === End of repeated output === [2020-04-03T16:02:11,741Z]

I verified that either move implementation into .c as a function body[1] or change to #ifdef __solaris__[2] will fix that. So I think we will change to detect __solaris__ as webrev[2] rather than have an extra #define. If some other build want to have that, they can be modify that #ifdef easily.

As I look into it, I found Mac have similar implementation with minor mistake, so I fixed that as well. Please review following based on zhanglin’s patch.

I’ll push [2] once I got a +1.

[1] http://cr.openjdk.java.net/~henryjen/jdk/8241638.0/webrev/ [2] http://cr.openjdk.java.net/~henryjen/jdk/8241638.1/webrev/

Cheers, Henry

...

On Apr 2, 2020, at 6:17 AM, Alan Bateman <Alan.Bateman@oracle.com> wrote:

On 02/04/2020 11:26, linzang(臧琳) wrote:

...

: Here is the updated webrev : http://cr.openjdk.java.net/~lzang/8241638/webrev04/ Thanks for your help!

webrev04 looks good. My preference was to was to replace #ifdef HAVE_GETHRTIME with #ifdef __solaris__ but there doesn't seem to be appetite to do this now. I think Henry has offered to help sponsor.

-Alan.

On Apr 5, 2020, at 7:15 PM, David Holmes <david.holmes@oracle.com> wrote:

On 6/04/2020 11:50 am, David Holmes wrote:

...

There is something not right here ... On 4/04/2020 3:13 pm, Henry Jen wrote:

...

Internal test shows that inline implementation is not working for some Solaris artifacts, because the -DHAVE_GETHRTIME is not consistently defined, so it is actually broken. :)

...

[2020-04-03T15:59:26,981Z] Creating support/test/hotspot/jtreg/native/bin/jvm-test-launcher from 1 file(s) Are you sure that line actually pertains to any error? The test defines a custom launcher which doesn't use libjli so should never be including the header file we are discussing. [2020-04-03T16:02:10,984Z] [2020-04-03T16:02:10,984Z] ERROR: Build failed for target 'default (product-bundles test-bundles static-libs-bundles)' in configuration 'solaris-sparcv9-open' (exit code 2) [2020-04-03T16:02:11,051Z] [2020-04-03T16:02:11,051Z] === Output from failing command(s) repeated here === [2020-04-03T16:02:11,055Z] * For target support_native_java.base_libjli_BUILD_LIBJLI_link: [2020-04-03T16:02:11,061Z] Undefined first referenced [2020-04-03T16:02:11,061Z] symbol in file [2020-04-03T16:02:11,061Z] getTimeMicros /export/home/opt/mach5/mesos/work_dir/e101deec-9613-4d46-a64d-559e689496aa/workspace/build/solaris-sparcv9-open/support/native/java.base/libjli/java.o [2020-04-03T16:02:11,061Z] ld: fatal: symbol referencing errors This looks like a direct linkage error. AFAICS ./launcher/LauncherCommon.gmk only defines -DHAVE_GETHRTIME for launcher executables. But if we are building libjli it is not an executable. I'm suspecting there is actually a long standing build bug here from when libjli was introduced. Possibly only evident on an incremental build.

I can confirm that the flags set in LauncherCommon.gmk are not passed to the compilation of java_md_solinux.c (I added a custom -DDAVIDH to the linux flags and checked the build). So I have no idea how this has been working, if indeed it actually has.

I should say it’s the inconsistency for building java.c for launcher executable and libjli.so, thus cause libjli.so failed to build. It wasn’t detected before because CounterGet is defined as no-op, so nothing to link with. Cheers, Henry

David

...

David -----

...

...

[2020-04-03T16:02:11,082Z] [2020-04-03T16:02:11,082Z] * All command lines available in /export/home/opt/mach5/mesos/work_dir/e101deec-9613-4d46-a64d-559e689496aa/workspace/build/solaris-sparcv9-open/make-support/failure-logs. [2020-04-03T16:02:11,086Z] === End of repeated output === [2020-04-03T16:02:11,094Z] [2020-04-03T16:02:11,094Z] === Make failed targets repeated here === [2020-04-03T16:02:11,736Z] CoreLibraries.gmk:206: recipe for target '/export/home/opt/mach5/mesos/work_dir/e101deec-9613-4d46-a64d-559e689496aa/workspace/build/solaris-sparcv9-open/support/modules_libs/java.base/libjli.so' failed [2020-04-03T16:02:11,737Z] make/Main.gmk:195: recipe for target 'java.base-libs' failed [2020-04-03T16:02:11,739Z] === End of repeated output === [2020-04-03T16:02:11,741Z]

I verified that either move implementation into .c as a function body[1] or change to #ifdef __solaris__[2] will fix that. So I think we will change to detect __solaris__ as webrev[2] rather than have an extra #define. If some other build want to have that, they can be modify that #ifdef easily.

As I look into it, I found Mac have similar implementation with minor mistake, so I fixed that as well. Please review following based on zhanglin’s patch.

I’ll push [2] once I got a +1.

[1] http://cr.openjdk.java.net/~henryjen/jdk/8241638.0/webrev/ [2] http://cr.openjdk.java.net/~henryjen/jdk/8241638.1/webrev/

Cheers, Henry

...

On Apr 2, 2020, at 6:17 AM, Alan Bateman <Alan.Bateman@oracle.com> wrote:

On 02/04/2020 11:26, linzang(臧琳) wrote:

...

: Here is the updated webrev : http://cr.openjdk.java.net/~lzang/8241638/webrev04/ Thanks for your help!

webrev04 looks good. My preference was to was to replace #ifdef HAVE_GETHRTIME with #ifdef __solaris__ but there doesn't seem to be appetite to do this now. I think Henry has offered to help sponsor.

-Alan.

On 6/04/2020 12:37 pm, David Holmes wrote:

On 6/04/2020 12:20 pm, Henry Jen wrote:

...

On Apr 5, 2020, at 7:15 PM, David Holmes <david.holmes@oracle.com> wrote:

...

On 6/04/2020 11:50 am, David Holmes wrote:

...

There is something not right here ... On 4/04/2020 3:13 pm, Henry Jen wrote:

...

Internal test shows that inline implementation is not working for some Solaris artifacts, because the -DHAVE_GETHRTIME is not consistently defined, so it is actually broken. :)

...

[2020-04-03T15:59:26,981Z] Creating support/test/hotspot/jtreg/native/bin/jvm-test-launcher from 1 file(s) Are you sure that line actually pertains to any error? The test defines a custom launcher which doesn't use libjli so should never be including the header file we are discussing. [2020-04-03T16:02:10,984Z] [2020-04-03T16:02:10,984Z] ERROR: Build failed for target 'default (product-bundles test-bundles static-libs-bundles)' in configuration 'solaris-sparcv9-open' (exit code 2) [2020-04-03T16:02:11,051Z] [2020-04-03T16:02:11,051Z] === Output from failing command(s) repeated here === [2020-04-03T16:02:11,055Z] * For target support_native_java.base_libjli_BUILD_LIBJLI_link: [2020-04-03T16:02:11,061Z] Undefined            first referenced [2020-04-03T16:02:11,061Z]  symbol                  in file [2020-04-03T16:02:11,061Z] getTimeMicros /export/home/opt/mach5/mesos/work_dir/e101deec-9613-4d46-a64d-559e689496aa/workspace/build/solaris-sparcv9-open/support/native/java.base/libjli/java.o

[2020-04-03T16:02:11,061Z] ld: fatal: symbol referencing errors This looks like a direct linkage error. AFAICS ./launcher/LauncherCommon.gmk only defines -DHAVE_GETHRTIME for launcher executables. But if we are building libjli it is not an executable. I'm suspecting there is actually a long standing build bug here from when libjli was introduced. Possibly only evident on an incremental build.

I can confirm that the flags set in LauncherCommon.gmk are not passed to the compilation of java_md_solinux.c (I added a custom -DDAVIDH to the linux flags and checked the build). So I have no idea how this has been working, if indeed it actually has.

I should say it’s the inconsistency for building java.c for launcher executable and libjli.so, thus cause libjli.so failed to build. It wasn’t detected before because CounterGet is defined as no-op, so nothing to link with.

My point is that this seems completely broken. HAVE_GETHRTIME is never defined when building libjli, only when building the launcher executables, and the launcher executable code never calls CounterGet(). This suggests that the launcher debug code on Solaris has been using the same code as linux and thus should be seen to be reporting the same thing ... which it is ...

_JAVA_LAUNCHER_DEBUG=true /java/re/jdk/9/promoted/latest/binaries/solaris-x64/bin/java -version ----_JAVA_LAUNCHER_DEBUG---- Launcher state:         First application arg index: -1         debug:on         javargs:off         program name:java         launcher name:java         javaw:off         fullversion:9+181 Command line args: argv[0] = /java/re/jdk/9/promoted/latest/binaries/solaris-x64/bin/java argv[1] = -version JRE path is .../java_re/jdk/9/fcs/181/binaries/solaris-x64 jvm.cfg[0] = ->-server<- jvm.cfg[1] = ->-client<- 1 micro seconds to parse jvm.cfg Default VM: server Does `.../export/java_re/jdk/9/fcs/181/binaries/solaris-x64/lib/server/libjvm.so' exist ... yes. mustsetenv: FALSE JVM path is .../export/java_re/jdk/9/fcs/181/binaries/solaris-x64/lib/server/libjvm.so 1 micro seconds to LoadJavaVM

Always reports 1 microsecond just like Linux.

This whole setup is completely broken at the build level.

This worked correctly in JDK 6 (6u181 is latest I could test) but is broken in JDK 7. David -----

Cheers, David

...

Cheers, Henry

...

David

...

David -----

...

...

[2020-04-03T16:02:11,082Z] [2020-04-03T16:02:11,082Z] * All command lines available in /export/home/opt/mach5/mesos/work_dir/e101deec-9613-4d46-a64d-559e689496aa/workspace/build/solaris-sparcv9-open/make-support/failure-logs.

[2020-04-03T16:02:11,086Z] === End of repeated output === [2020-04-03T16:02:11,094Z] [2020-04-03T16:02:11,094Z] === Make failed targets repeated here === [2020-04-03T16:02:11,736Z] CoreLibraries.gmk:206: recipe for target '/export/home/opt/mach5/mesos/work_dir/e101deec-9613-4d46-a64d-559e689496aa/workspace/build/solaris-sparcv9-open/support/modules_libs/java.base/libjli.so' failed [2020-04-03T16:02:11,737Z] make/Main.gmk:195: recipe for target 'java.base-libs' failed [2020-04-03T16:02:11,739Z] === End of repeated output === [2020-04-03T16:02:11,741Z]

I verified that either move implementation into .c as a function body[1] or change to #ifdef __solaris__[2] will fix that. So I think we will change to detect __solaris__ as webrev[2] rather than have an extra #define. If some other build want to have that, they can be modify that #ifdef easily.

As I look into it, I found Mac have similar implementation with minor mistake, so I fixed that as well. Please review following based on zhanglin’s patch.

I’ll push [2] once I got a +1.

[1] http://cr.openjdk.java.net/~henryjen/jdk/8241638.0/webrev/ [2] http://cr.openjdk.java.net/~henryjen/jdk/8241638.1/webrev/

Cheers, Henry

...

On Apr 2, 2020, at 6:17 AM, Alan Bateman <Alan.Bateman@oracle.com> wrote:

On 02/04/2020 11:26, linzang(臧琳) wrote: > : >              Here is the updated webrev : > http://cr.openjdk.java.net/~lzang/8241638/webrev04/ >              Thanks for your help! > webrev04 looks good. My preference was to was to replace #ifdef HAVE_GETHRTIME with #ifdef __solaris__ but there doesn't seem to be appetite to do this now. I think Henry has offered to help sponsor.

-Alan.

Hi Henry, On 7/04/2020 3:36 am, Henry Jen wrote:

Here is the combined webrev[1] which I think is what we should have. By using __solaris__, we make sure no extra define from build and assuming that all solaris build will have gethrtime() and use that.

This looks good to me and means the Solaris code should start working correctly again, as well as providing an implementation on all other platforms. Only minor suggestion is to move 33 #include <sys/time.h> outside of the ifdef in the header file as all platforms will include it anyway. You can then also remove the include in the .c file 819 #include <sys/time.h> Thanks, David -----

The current build only use that for static build launcher, not custom launcher use libjli.

Cheers, Henry

[1] http://cr.openjdk.java.net/~henryjen/jdk/8241638.2/webrev/

...

On Apr 5, 2020, at 9:21 PM, David Holmes <david.holmes@oracle.com> wrote:

On 6/04/2020 12:37 pm, David Holmes wrote:

...

On 6/04/2020 12:20 pm, Henry Jen wrote:

...

On Apr 5, 2020, at 7:15 PM, David Holmes <david.holmes@oracle.com> wrote:

...

On 6/04/2020 11:50 am, David Holmes wrote:

...

There is something not right here ... On 4/04/2020 3:13 pm, Henry Jen wrote: > Internal test shows that inline implementation is not working for some Solaris artifacts, because the -DHAVE_GETHRTIME is not consistently defined, so it is actually broken. :) > >> [2020-04-03T15:59:26,981Z] Creating support/test/hotspot/jtreg/native/bin/jvm-test-launcher from 1 file(s) Are you sure that line actually pertains to any error? The test defines a custom launcher which doesn't use libjli so should never be including the header file we are discussing. >> [2020-04-03T16:02:10,984Z] >> [2020-04-03T16:02:10,984Z] ERROR: Build failed for target 'default (product-bundles test-bundles static-libs-bundles)' in configuration 'solaris-sparcv9-open' (exit code 2) >> [2020-04-03T16:02:11,051Z] >> [2020-04-03T16:02:11,051Z] === Output from failing command(s) repeated here === >> [2020-04-03T16:02:11,055Z] * For target support_native_java.base_libjli_BUILD_LIBJLI_link: >> [2020-04-03T16:02:11,061Z] Undefined first referenced >> [2020-04-03T16:02:11,061Z] symbol in file >> [2020-04-03T16:02:11,061Z] getTimeMicros /export/home/opt/mach5/mesos/work_dir/e101deec-9613-4d46-a64d-559e689496aa/workspace/build/solaris-sparcv9-open/support/native/java.base/libjli/java.o >> [2020-04-03T16:02:11,061Z] ld: fatal: symbol referencing errors This looks like a direct linkage error. AFAICS ./launcher/LauncherCommon.gmk only defines -DHAVE_GETHRTIME for launcher executables. But if we are building libjli it is not an executable. I'm suspecting there is actually a long standing build bug here from when libjli was introduced. Possibly only evident on an incremental build.

I can confirm that the flags set in LauncherCommon.gmk are not passed to the compilation of java_md_solinux.c (I added a custom -DDAVIDH to the linux flags and checked the build). So I have no idea how this has been working, if indeed it actually has.

I should say it’s the inconsistency for building java.c for launcher executable and libjli.so, thus cause libjli.so failed to build. It wasn’t detected before because CounterGet is defined as no-op, so nothing to link with. My point is that this seems completely broken. HAVE_GETHRTIME is never defined when building libjli, only when building the launcher executables, and the launcher executable code never calls CounterGet(). This suggests that the launcher debug code on Solaris has been using the same code as linux and thus should be seen to be reporting the same thing ... which it is ... _JAVA_LAUNCHER_DEBUG=true /java/re/jdk/9/promoted/latest/binaries/solaris-x64/bin/java -version ----_JAVA_LAUNCHER_DEBUG---- Launcher state: First application arg index: -1 debug:on javargs:off program name:java launcher name:java javaw:off fullversion:9+181 Command line args: argv[0] = /java/re/jdk/9/promoted/latest/binaries/solaris-x64/bin/java argv[1] = -version JRE path is .../java_re/jdk/9/fcs/181/binaries/solaris-x64 jvm.cfg[0] = ->-server<- jvm.cfg[1] = ->-client<- 1 micro seconds to parse jvm.cfg Default VM: server Does `.../export/java_re/jdk/9/fcs/181/binaries/solaris-x64/lib/server/libjvm.so' exist ... yes. mustsetenv: FALSE JVM path is .../export/java_re/jdk/9/fcs/181/binaries/solaris-x64/lib/server/libjvm.so 1 micro seconds to LoadJavaVM Always reports 1 microsecond just like Linux. This whole setup is completely broken at the build level.

This worked correctly in JDK 6 (6u181 is latest I could test) but is broken in JDK 7.

David -----

...

Cheers, David

...

Cheers, Henry

...

David

...

David ----- >> [2020-04-03T16:02:11,082Z] >> [2020-04-03T16:02:11,082Z] * All command lines available in /export/home/opt/mach5/mesos/work_dir/e101deec-9613-4d46-a64d-559e689496aa/workspace/build/solaris-sparcv9-open/make-support/failure-logs. >> [2020-04-03T16:02:11,086Z] === End of repeated output === >> [2020-04-03T16:02:11,094Z] >> [2020-04-03T16:02:11,094Z] === Make failed targets repeated here === >> [2020-04-03T16:02:11,736Z] CoreLibraries.gmk:206: recipe for target '/export/home/opt/mach5/mesos/work_dir/e101deec-9613-4d46-a64d-559e689496aa/workspace/build/solaris-sparcv9-open/support/modules_libs/java.base/libjli.so' failed >> [2020-04-03T16:02:11,737Z] make/Main.gmk:195: recipe for target 'java.base-libs' failed >> [2020-04-03T16:02:11,739Z] === End of repeated output === >> [2020-04-03T16:02:11,741Z] > > > I verified that either move implementation into .c as a function body[1] or change to #ifdef __solaris__[2] will fix that. So I think we will change to detect __solaris__ as webrev[2] rather than have an extra #define. If some other build want to have that, they can be modify that #ifdef easily. > > As I look into it, I found Mac have similar implementation with minor mistake, so I fixed that as well. Please review following based on zhanglin’s patch. > > I’ll push [2] once I got a +1. > > [1] http://cr.openjdk.java.net/~henryjen/jdk/8241638.0/webrev/ > [2] http://cr.openjdk.java.net/~henryjen/jdk/8241638.1/webrev/ > > Cheers, > Henry > >> On Apr 2, 2020, at 6:17 AM, Alan Bateman <Alan.Bateman@oracle.com> wrote: >> >> On 02/04/2020 11:26, linzang(臧琳) wrote: >>> : >>> Here is the updated webrev : http://cr.openjdk.java.net/~lzang/8241638/webrev04/ >>> Thanks for your help! >>> >> webrev04 looks good. My preference was to was to replace #ifdef HAVE_GETHRTIME with #ifdef __solaris__ but there doesn't seem to be appetite to do this now. I think Henry has offered to help sponsor. >> >> -Alan.

On 06/04/2020 18:36, Henry Jen wrote:

Here is the combined webrev[1] which I think is what we should have. By using __solaris__, we make sure no extra define from build and assuming that all solaris build will have gethrtime() and use that.

The current build only use that for static build launcher, not custom launcher use libjli.

Cheers, Henry

[1] http://cr.openjdk.java.net/~henryjen/jdk/8241638.2/webrev/

I think it would be simpler to drop the macros and have function prototypes in java_md_sollinux.h. That would avoid preprocessor code in the header file. The implementation in java_md_sollinux.c would then be simple #ifdef __solaris ... #else ... Minor nit is that the #include <sys/time.h> should probably be moved up to the top with the other includes. -Alan.

Dear all, As Lin mentioned, I found the copyright info is not updated, so I prepared a patch to fix this. Lin helps me to build an issue in the jdk bug system. Would you help me to review this patch? Thank you very much. Patch link: https://cr.openjdk.java.net/~lzang/BuddyLiao/launcher/webrev/ Issue link: https://bugs.openjdk.java.net/browse/JDK-8243539 BRs, Bin 在 2020/4/24 上午10:38，“linzang(臧琳)”<linzang@tencent.com> 写入: Hi Henry, Alan, David My colleague Bin(Buddy) just remind me that the copyright info of the touched files should be updated, sorry that I forgot to add it in my original patch , do you think there should be a patch for fixing that? BRs, Lin On 2020/4/7, 4:56 PM, "Alan Bateman" <Alan.Bateman@oracle.com> wrote: On 06/04/2020 18:36, Henry Jen wrote: > Here is the combined webrev[1] which I think is what we should have. By using __solaris__, we make sure no extra define from build and assuming that all solaris build will have gethrtime() and use that. > > The current build only use that for static build launcher, not custom launcher use libjli. > > Cheers, > Henry > > [1] http://cr.openjdk.java.net/~henryjen/jdk/8241638.2/webrev/ > I think it would be simpler to drop the macros and have function prototypes in java_md_sollinux.h. That would avoid preprocessor code in the header file. The implementation in java_md_sollinux.c would then be simple #ifdef __solaris ... #else ... Minor nit is that the #include <sys/time.h> should probably be moved up to the top with the other includes. -Alan.