I put these new methods in VMSupport since this was the class containg the getVMTemporaryDirectory and the intention of this class was document as:
/* * Support class used by JVMTI and VM attach mechanism. */ class VMSupport { I could create a new PerfDataFileImpl or jdk.internal.jvmstat.VMSupport class with a Linux specific alternate implementation that contains these classes: getTemporaryDirectories getTemporaryDirectory getLocalVmId But the getNamespaceVmId method is also needed by the attach mechanism. I removed the duplicated functionality to avoid having to maintain this in 2 places. Shouldn’t the attach functionality, in the future, be decoupled from jvmstat? It looked like jcmd was using these jvmstat classes in order to avoid duplication but I don’t have all the history. Bob. > On Jan 17, 2018, at 1:16 PM, mandy chung <mandy.ch...@oracle.com> wrote: > > Hi Bob, > > I think the new methods in VMSupport and probably with > VMSupport.getVMTemporaryDirectory should belong to jdk.internal.jvmstat as > they are specific for jvmstat and attach API to use. > > W.r.t VMSupportImpl, the implementation is the same for all platforms except > Linux. One option is to have a shared implementation class and instantiate > an alternate implementation class, if present, using Class::forName. > Another minor point is that the new getVMTemporaryDirectories method can > return a List rather than an array. > > Mandy > > On 1/17/18 8:37 AM, Bob Vandette wrote: >> Here’s an updated webrev addressing the comments from David Holmes. >> >> 1. Moved new cgroup specific support from unix -> Linux and put stubs in all >> other OS's >> 2. Reduced the size of the stack arrays in perfMemory_linux.cpp >> >> >> There are still two outstanding issues: >> >> [serviceability] : Can we and should we remove the getFile methods in the >> PerfDataFile class? >> >> [core-libs]: Do you have any problems with the module info file changes >> impacting java.base? >> >> Details: >> >> Bug: >> >> >> https://bugs.openjdk.java.net/browse/JDK-8193710 >> >> >> Webrev: >> >> >> http://cr.openjdk.java.net/~bobv/8193710/webrev.01/ >> >> >> Summary: >> >> This changeset enables the ability to use jcmd and jps running on a Host to >> list the java processes that are running in docker (cgroup based) containers. >> >> I’ve tested this change by examining processes running as root on both host >> and in >> docker containers as well as under my userid using “jps and jcmd -l”. >> I’ve also tested the getFile functions with a small example program that I >> wrote. >> From what I can tell, these two getFile functions are not used in the JDK >> sources >> and in fact the PerfDataFile.getFile(int lvmid) method doesn’t appear to >> have never worked! >> I’d really like to remove these two methods. >> >> 140 candidate = new File(f.getName(), name); <<——— This >> line drops the /tmp directory off of the path. >> >> Here are some implementation details that I’ve added added to one of the >> Unix >> specific source files >> (src/java.base/unix/classes/jdk/internal/vm/VMSupportImpl.java) >> >> /* Implementation Details: >> * >> * Java processes that run in docker containers are typically running >> * under cgroups with separate pid namespaces which means that pids >> * within the container are different that the pid which is visible >> * from the host. The container pids typically start with 1 and >> * increase. The java process running in the container will use these >> * pids when creating the hsperfdata files. In order to locate java >> * processes that are running in containers, we take advantage of >> * the Linux proc file system which maps the containers tmp directory >> * to the hosts under /proc/{hostpid}/root/tmp. We use the /proc status >> * file /proc/{hostpid}/status to determine the containers pid and >> * then access the hsperfdata file. The status file contains an >> * entry "NSPid:" which shows the mapping from the hostpid to the >> * containers pid. >> * >> * Example: >> * >> * NSPid: 24345 11 >> * >> * In this example process 24345 is visible from the host, >> * is running under the PID namespace and has a container specific >> * pid of 11. >> * >> * The search for Java processes is done by first looking in the >> * traditional /tmp for host process hsperfdata files and then >> * the search will container in every /proc/*/root/tmp directory. >> * There are of course added complications to this search that >> * need to be taken into account. >> * >> * 1. duplication of tmp directories >> * >> * /proc/{hostpid}/root/tmp directories exist for many processes >> * that are running on a Linux kernel that has cgroups enabled even >> * if they are not running in a container. To avoid this duplication, >> * we compare the inode of the /proc tmp directories to /tmp and >> * skip these duplicated directories. >> * >> * 2. Containerized processes without PID namespaces being enabled. >> * >> * If a container is running a Java process without namespaces being >> * enabled, an hsperfdata file will only be located at >> * /proc/{hostpid}/root/tmp/{hostpid}. This is handled by >> * checking the last component in the path for both the hostpid >> * and potential namespacepids (if one exists). >> */ >> >> >> >> Bob. >> >