Hi The changes look good for me.
Leonid > On Jul 2, 2020, at 7:21 AM, Chris Plummer <chris.plum...@oracle.com> wrote: > > [Note, the following changes only impact serviceability tests, but I am > adding some test library code to assist with creating and finding core files, > and I thought others on hotspot-dev might have an interest in that.] > > Hello, > > Please help review the following changes to add better support for writing SA > tests that work on core files: > > https://bugs.openjdk.java.net/browse/JDK-8248194 > http://cr.openjdk.java.net/~cjplummer/8248194/webrev.01/index.html > > As outlined in the CR, these are the 3 main goals of this CR: > > 1. Add a shared API for locating the path to the core file. This includes > parsing the output of the crashed process to locate where the core file was > saved, and returning this location to the user. This API will be placed in > the new CoreUtils class. > > 2. Add a shared API to support adding the "ulimit -c unlimited" prefix to the > command that will produce the core file, allowing the overriding of any lower > limit so we can be sure the core file will be produced. This API will also be > placed in the new CoreUtils class. > > 3. LingeredApp should include support for producing a core file. > > As proof of concept for these improvements, I'm updating the following 3 > tests to make use of them: > > ClhsdbCDSCore.java and TestJmapCore.java: Use the CoreUtils support listed > above. These tests do not use LingeredApp, so those improvements don't apply. > > ClhsdbFindPC.java: Use all the above new features, including having > LingeredApp produce a core file. This is the only test modified to start > testing on core files that didn't previously do so. It still also tests on a > live process. > > In the future more Clhsdb tests will be converted to work on core files in a > manner similar to ClhsdbFindPC. > > The new CoreUtils code is borrowed from (more like ripped out of) > ClhsdbCDSCore.java and TestJmapCore.java. They both had a lot of code > dedicated to finding the core file and also applying "ulimit -c unlimited", > but didn't do so in quite the same way. Now both these tests share code in > CoreUtils.java. One thing I did drop is TestJmapCore.java use of > ":KILLED_PID" in the output to help find the core file. It's no longer > necessary based on the smarter core locating code I pulled from > ClhsdbCDSCore.java. > > One other improvement was to enable windows support for ClhsdbCDSCore. The > only reason it was not enabled previously is because the author couldn't > figure out how to properly generate the command for the process that produces > the core. Since it is launched using "sh -c", the path has to be converted to > use forward slashes. This is now done in CoreUtils. > > One other change in ClhsdbCDSCore is that it no longer renames the core file > to a well known name in the cwd. This was unnecessary. It originated in code > from ciReplayBase.java, which does have a reason to do the rename, but > ClhsdbCDSCore does not. > > The new libLingeredApp.c relies on JDK-8248667 [1] being in place in order to > have the build system properly compile it. JDK-8248667 will be reviewed > separately on build-dev and pushed just before the changes for this CR. > > thanks, > > Chris > > [1] https://bugs.openjdk.java.net/browse/JDK-8248667
