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