On Sun, 1 Aug 2021 08:17:08 GMT, Thomas Stuefe <stu...@openjdk.org> wrote:
>> Short: this patch makes NMT available in custom-launcher scenarios and >> during gtests. It simplifies NMT initialization. It adds a lot of >> NMT-specific testing, cleans them up and makes them sideeffect-free. >> >> --------- >> >> NMT continues to be an extremely useful tool for SAP to tackle memory >> problems in the JVM. >> >> However, NMT is of limited use due to the following restrictions: >> >> - NMT cannot be used if the hotspot is embedded into a custom launcher >> unless the launcher actively cooperates. Just creating and invoking the JVM >> is not enough, it needs to do some steps prior to loading the hotspot. This >> limitation is not well known (nor, do I believe, documented). Many products >> don't do this, e.g., you cannot use NMT with IntelliJ. For us at SAP this >> problem limits NMT usefulness greatly since our VMs are often embedded into >> custom launchers and modifying every launcher is impossible. >> - Worse, if that custom launcher links the libjvm *statically* there is just >> no way to activate NMT at all. This is the reason NMT cannot be used in the >> `gtestlauncher`. >> - Related to that is that we cannot pass NMT options via `JAVA_TOOL_OPTIONS` >> and `-XX:Flags=<file>`. >> - The fact that NMT cannot be used in gtests is really a pity since it would >> allow us to both test NMT itself more rigorously and check for memory leaks >> while testing other stuff. >> >> The reason for all this is that NMT initialization happens very early, on >> the first call to `os::malloc()`. And those calls happen already during >> dynamic C++ initialization - a long time before the VM gets around parsing >> arguments. So, regular VM argument parsing is too late to parse NMT >> arguments. >> >> The current solution is to pass NMT arguments via a specially prepared >> environment variable: `NMT_LEVEL_<PID>=<NMT arguments>`. That environment >> variable has to be set by the embedding launcher, before it loads the >> libjvm. Since its name contains the PID, we cannot even set that variable in >> the shell before starting the launcher. >> >> All that means that every launcher needs to especially parse and process the >> NMT arguments given at the command line (or via whatever method) and prepare >> the environment variable. `java` itself does this. This only works before >> the libjvm.so is loaded, before its dynamic C++ initialization. For that >> reason, it does not work if the launcher links statically against the >> hotspot, since in that case C++ initialization of the launcher and hotspot >> are folded into one phase with no possibility of executing code beforehand. >> >> And since it bypasses argument handling in the VM, it bypasses a number of >> argument processing ways, e.g., `JAVA_TOOL_OPTIONS`. >> >> ------ >> >> This patch fixes these shortcomings by making NMT late-initializable: it can >> now be initialized after normal VM argument parsing, like all other parts of >> the VM. This greatly simplifies NMT initialization and makes it work >> automagically for every third party launcher, as well as within our gtests. >> >> The glaring problem with late-initializing NMT is the NMT malloc headers. If >> we rule out just always having them (unacceptable in terms of memory >> overhead), there is no safe way to determine, in os::free(), if an >> allocation came from before or after NMT initialization ran, and therefore >> what to do with its malloc headers. For a more extensive explanation, please >> see the comment block `nmtPreInit.hpp` and the discussion with @kimbarrett >> and @zhengyu123 in the JBS comment section. >> >> The heart of this patch is a new way to track early, pre-NMT-init >> allocations. These are tracked via a lookup table. This was a suggestion by >> Kim and it worked out well. >> >> Changes in detail: >> >> - pre-NMT-init handling: >> - the new files `nmtPreInit.hpp/cpp` take case of NMT pre-init >> handling. They contain a small global lookup table managing C-heap blocks >> allocated in the pre-NMT-init phase. >> - `os::malloc()/os::realloc()/os::free()` defer to this code before >> doing anything else. >> - Please see the extensive comment block at the start of >> `nmtPreinit.hpp` explaining the details. >> >> - Changes to NMT: >> - Before, NMT initialization was spread over two phases, `initialize()` >> and `late_initialize()`. Those were merged into one and simplified - there >> is only one initialization now which happens after argument parsing. >> - Minor changes were needed for the `NMT_TrackingLevel` enum - to >> simplify code, I changed NMT_unknown to be numerically 0. A new comment >> block in `nmtCommon.hpp` now clearly specifies what's what, including >> allowed level state transitions. >> - New utility functions to translate tracking level from/to strings >> added to `NMTUtil` >> - NMT has never been able to handle virtual memory allocations before >> initialization, which is fine since os::reserve_memory() is not called >> before VM parses arguments. We now assert that. >> - All code outside the VM handling NMT initialization (eg. libjli) has >> been removed, as has the code testing it. >> >> - Gtests: >> - Some existing gtests had to be modified: before, they all changed >> global state (turning NMT on/off) before testing. This is not allowed >> anymore, to keep NMT simple. Also, this pattern disturbed other tests. >> - The new way to test is to passively check whether NMT has been >> switched on or off, and do tests accordingly: if on, full tests, if off, >> test just what makes sense in off-state. That does not disturb neighboring >> tests, gives us actually better coverage all around. >> - It is now possible to start the gtestlauncher with NMT on! Which >> additionally gives us good coverage. >> - To actually do gtests with NMT - since it's disabled by default - we >> now run NMT-enabled gtests as part of the hotspot jtreg NMT wrapper. This >> pattern we have done for a number of other facitilites, see all the tests in >> test/hotspot/jtreg/gtest.. . It works very well. >> - Finally, a new gtest has been written to test the NMT preinit lookup >> map in isolation, placed in `gtest/nmt/test_nmtpreinitmap.cpp`. >> >> - jtreg: >> - A new test has been added, `runtime/NMT/NMTInitializationTest.java`, >> testing NMT initialization in the face of many many VM arguments. >> >> ------------- >> >> Tests: >> - ran manually all new tests on 64-bit and 32-bit Linux >> - GHAs >> - The patch has been active in SAPs test systems for a while now. > > Thomas Stuefe has updated the pull request incrementally with six additional > commits since the last revision: > > - feedback zhengyu > - feeback Coleen/Kim (constness fix) > - Feedback David > - Add test to test for non-java launcher support of NMT > - move all test code to gtest > - actually free blocks freed in pre-init phase This looks good. Thanks for fixing the mysterious (to me) cast. src/hotspot/share/services/nmtPreInit.cpp line 207: > 205: static TestAllocations g_test_allocations; // make this an automatic > object to let ctor run during in C++ dynamic initialization. > 206: #endif // ASSERT > 207: Oh, good. I was hoping this didn't need to be an in-jvm test. ------------- Marked as reviewed by coleenp (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4874
