On Fri, 27 Oct 2023 12:28:51 GMT, Thomas Stuefe <stu...@openjdk.org> wrote:
>> Analysts and supporters often use /proc/xx/maps to make sense of the memory >> footprint of a process. >> >> Interpreting the memory map correctly can help when used as a complement to >> other tools (e.g. NMT). There even exist tools out there that attempt to >> annotate the process memory map with JVM information. >> >> That, however, can be more accurately done from within the JVM, at least for >> mappings originating from hotspot. After all, we can correlate the mapping >> information in NMT with the VMA map. >> >> Example output (from a spring petstore run): >> >> [example_system_map.txt](https://github.com/openjdk/jdk/files/13179054/example_system_map.txt) >> >> This patch adds the VM annotations for VMAs that are *mmaped*. I also have >> an experimental patch that works with malloc'ed memory, but it is not ready >> yet for consumption and I leave that part of for now. > > Thomas Stuefe has updated the pull request incrementally with three > additional commits since the last revision: > > - Fix spelling > - timeout fuse > - Feedback Johan Changes requested by gziemski (Committer). src/hotspot/share/services/memMapPrinter.cpp line 49: > 47: // Short, clear, descriptive names for all possible markers. Note that we > only expect to see > 48: // those that have been used with mmap. Others I leave at nullptr. > 49: #define NMTFLAGS_DO(f) \ NMTFLAGS_DO --> NMT_FLAGS_DO to follow the pattern of IK_FLAGS_DO, CM_FLAGS_DO src/hotspot/share/services/memMapPrinter.cpp line 95: > 93: }; > 94: > 95: /// NMT virtual memory Should we try to put all this NMT flags code into allocation.hpp? src/hotspot/share/services/memMapPrinter.cpp line 181: > 179: static uintx safely_get_thread_id(const Thread* t) { > 180: const OSThread* osth = t->osthread(); > 181: uintx tid = 0; Leftover? src/hotspot/share/services/memMapPrinter.cpp line 188: > 186: } > 187: > 188: // Given a region [from, to) that is supposed to represent a thread > stack, Need to finish the comment? src/hotspot/share/services/memMapPrinter.cpp line 192: > 190: > 191: for (JavaThreadIteratorWithHandle jtiwh; JavaThread* t = jtiwh.next(); > ) { > 192: const size_t len = pointer_delta(to, from, 1); Leftover? src/hotspot/share/services/memMapPrinter.cpp line 196: > 194: st->print("(" UINTX_FORMAT " \"%s\")", safely_get_thread_id(t), > t->name()); > 195: return; > 196: } Can we use `HANDLE_THREAD` here instead? src/hotspot/share/services/memMapPrinter.cpp line 294: > 292: // the absolute runtime of printing to blocking other VM operations > too long. > 293: const jlong timeout_at = os::javaTimeNanos() + > 294: ((jlong)(SafepointTimeoutDelay * > NANOSECS_PER_MILLISEC) / 2); Is AbortVMOnSafepointTimeoutDelay/2 timeout useful for real world cases? I assume the client who would need this feature, would need it for a reason and that means memory hungry processes? Instead of blocking VM and having to use the timeout, can we: A. take a snapshot of NMT virtual memory region and process it without blocking the VM B. divide this process into smaller chunks, each of which will be guaranteed to finish before AbortVMOnSafepointTimeoutDelay triggers C. temporarily increase AbortVMOnSafepointTimeoutDelay and revert it back afterwords? In case A,B we would get back a very close, but not exact view of memory, but wouldn't this be better than getting back precise, but cut off view? Case C would give us what the users wants assuming they are OK with waiting a bit longer? And adding malloc'ed memory here would be make this issue much worse. ------------- PR Review: https://git.openjdk.org/jdk/pull/16301#pullrequestreview-1702475105 PR Review Comment: https://git.openjdk.org/jdk/pull/16301#discussion_r1375002985 PR Review Comment: https://git.openjdk.org/jdk/pull/16301#discussion_r1375011085 PR Review Comment: https://git.openjdk.org/jdk/pull/16301#discussion_r1375017943 PR Review Comment: https://git.openjdk.org/jdk/pull/16301#discussion_r1375020309 PR Review Comment: https://git.openjdk.org/jdk/pull/16301#discussion_r1375020664 PR Review Comment: https://git.openjdk.org/jdk/pull/16301#discussion_r1375021914 PR Review Comment: https://git.openjdk.org/jdk/pull/16301#discussion_r1375080669
