On 5/5/20 17:04, Mikael Vidstedt wrote:
On May 5, 2020, at 4:48 PM, serguei.spit...@oracle.com wrote:
Hi Mikael,
The fixes in webrev look good to me.
I've just noticed a couple of more serviceability related things can be missed.
(Not sure if they are included into different chunk of fixes.)
One is libjvm_db.so which is for Solaris Pstack support:
make/hotspot/lib/CompileDtraceLibraries.gmk: # Note that libjvm_db.c has
tests for COMPILER2, but this was never set by
make/hotspot/lib/CompileDtraceLibraries.gmk: LIBJVM_DB_OUTPUTDIR :=
$(JVM_VARIANT_OUTPUTDIR)/libjvm_db
make/hotspot/lib/CompileDtraceLibraries.gmk: NAME := jvm_db, \
make/hotspot/lib/CompileDtraceLibraries.gmk: SRC :=
$(TOPDIR)/src/java.base/solaris/native/libjvm_db, \
Another is DTrace support which also includes jhelper.d (support for DTrace
jstack action which is for Solaris only):
make/hotspot/gensrc/GensrcDtrace.gmk
make/hotspot/lib/JvmDtraceObjects.gmk
It also impacts some other make files.
I believe you’ll find that these changes were included in the build system
patch:
https://mail.openjdk.java.net/pipermail/build-dev/2020-May/027342.html
http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.00/build/open/webrev/
Let me know if I missed something.
Thanks, I'll look at at.
Also, these lines can be just removed:
open/src/jdk.jdwp.agent/unix/native/libjdwp/path_md.h:#ifndef
_JAVASOFT_SOLARIS_PATH_MD_H_
open/src/jdk.jdwp.agent/unix/native/libjdwp/path_md.h:#define
_JAVASOFT_SOLARIS_PATH_MD_H_
open/src/jdk.jdwp.agent/unix/native/libjdwp/path_md.h:#endif /*
!_JAVASOFT_SOLARIS_PATH_MD_H_ */
Remove or rather rename? The corresponding Windows header also has the guard,
any particular reason why it should be *removed*?
You are right, we have to rename it: _JAVASOFT_SOLARIS_PATH_MD_H_ =>
_UNIX_PATH_MD_H_
No need in another webrev.
Thanks,
Serguei
Cheers,
Mikael
Thanks,
Serguei
On 5/5/20 14:34, Mikael Vidstedt wrote:
All good points! I deliberately chose *not* to update comments where it wasn’t
immediately 100% obvious exactly how to update them. For example, in many cases
I found that the comments are already incomplete or stale, and for each such
case we’ll want to consider how exactly to update the comment (remove/switch to
Unix/etc.). I would prefer to handle this as follow-up(s), separate from the
JEP. Does that sounds reasonable?
Apart from the comments - do the changes look good? If so, can I count you as a
reviewer?
Cheers,
Mikael
On May 4, 2020, at 12:20 AM, serguei.spit...@oracle.com wrote:
HI Mikael,
Some quick comments.
Some extra references to Solaris/solaris, SunOS or SPARC are listed below:
src/java.instrument/unix/native/libinstrument/FileSystemSupport_md.c (2 refs to
Solaris/solaris)
src/java.management/share/classes/javax/management/loading/MLet.java (refs to
Solaris, SPARC/sparc and SunOS)
src/jdk.management.agent/unix/classes/jdk/internal/agent/FileSystemImpl.java
(ref to Solaris)
src/hotspot/share/prims/forte.cpp:// Solaris SPARC Compiler1 needs an
additional check on the grandparent
src/hotspot/share/prims/forte.cpp:// on Linux X86, Solaris SPARC and Solaris
X86.
src/hotspot/share/prims/jvmti.xml: On the <tm>Solaris</tm> Operating
Environment, an agent library is a shared
src/hotspot/share/prims/jvmti.xml: <code>LD_LIBRARY_PATH</code> under the
<tm>Solaris</tm> operating
src/hotspot/share/prims/jvmti.xml: example, in the Java 2 SDK a CTRL-Break
on Win32 and a CTRL-\ on Solaris
src/hotspot/share/prims/methodHandles.cpp:#undef CS // Solaris builds complain
Thanks,
Serguei
On 5/3/20 22:12, Mikael Vidstedt wrote:
Please review this change which implements part of JEP 381:
JBS: https://bugs.openjdk.java.net/browse/JDK-8244224
webrev:
http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.00/serviceability/open/webrev/
JEP: https://bugs.openjdk.java.net/browse/JDK-8241787
Note: When reviewing this, please be aware that this exercise was *extremely*
mind-numbing, so I appreciate your help reviewing all the individual changes
carefully. You may want to get that coffee cup filled up (or whatever keeps you
awake)!
Background:
Because of the size of the total patch and wide range of areas touched, this
patch is one out of in total six partial patches which together make up the
necessary changes to remove the Solaris and SPARC ports. The other patches are
being sent out for review to mailing lists appropriate for the respective areas
the touch. An email will be sent to jdk-dev summarizing all the
patches/reviews. To be clear: this patch is *not* in itself complete and
stand-alone - all of the (six) patches are needed to form a complete patch.
Some changes in this patch may look wrong or incomplete unless also looking at
the corresponding changes in other areas.
For convenience, I’m including a link below[1] to the full webrev, but in case
you have comments on changes in other areas, outside of the files included in
this thread, please provide those comments directly in the thread on the
appropriate mailing list for that area if possible.
In case it helps, the changes were effectively produced by searching for and
updating any code mentioning “solaris", “sparc”, “solstudio”, “sunos”, etc.
More information about the areas impacted can be found in the JEP itself.
Testing:
A slightly earlier version of this change successfully passed tier1-8, as well
as client tier1-2. Additional testing will be done after the first round of
reviews has been completed.
Cheers,
Mikael
[1]
http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.00/all/open/webrev/
