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