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/ > On Apr 5, 2020, at 9:21 PM, David Holmes <david.hol...@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.hol...@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.bate...@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.
