RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (serviceability)

Wed May 6 17:58:01 UTC 2020


On 5/6/20 1:04 PM, serguei.spitsyn at oracle.com wrote:
> On 5/6/20 03:40, coleen.phillimore at oracle.com wrote:
>>
>>
>> On 5/6/20 2:09 AM, serguei.spitsyn at oracle.com wrote:
>>> On 5/5/20 17:04, Mikael Vidstedt wrote:
>>>>> On May 5, 2020, at 4:48 PM,serguei.spitsyn at 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.
>>>
>>> The file || make/hotspot/src/native/dtrace/generateJvmOffsets.cpp is 
>>> for Solaris only, and so, can be removed.
>>> It is for libjvm_db.so (provider for Solaris Pstack) and jhelper.d 
>>> (provider for Solaris DTrace jstack action).
>>> The jstack action (prints mixed java+native stack traces) was never 
>>> implemented other than for Solaris.
>>
>> I wonder if this can be used to implement the same thing on linux and 
>> if we can keep this?  Thoughts?
>
> I was also thinking about it. And DTrace kind of exists on Mac OS as well.
> Yes, it can be used. But It will require the jstack action 
> implementation and the jhelper use on the DTrace side.

> The same is true for the libjvm_db.so. It could be used in the Linux 
> Pstack utility to print mixed stack traces.
> (I see some pstack projects like this: https://github.com/peadar/pstack )
Yes, I was thinking specifically of the pstack utility and not DTrace, 
just to get Java frames in ptrace.  Could libjvm_db.so be used for that?

Coleen

>
> But as we already discussed the GDB has a different approach to 
> register code:
> https://sourceware.org/gdb/onlinedocs/gdb/JIT-Interface.html#JIT-Interface
>
> Thanks,
> Serguei
>
>>
>> thanks,
>> Coleen
>>>
>>> I do not see other problems but looked only at the Serviceability 
>>> related files.
>>>
>>> Thanks,
>>> Serguei
>>>
>>>>> 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*?
>>>>
>>>> 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.spitsyn at 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/
>>>>>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20200506/2d9861ac/attachment-0001.htm>