On Sat, 28 Oct 2023 13:04:05 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 one additional > commit since the last revision: > > fix various builds Just a couple of issues/questions. This is a very nice feature we're adding. Thank you! src/hotspot/os/linux/memMapPrinter_linux.cpp line 59: > 57: void print_OS_specific_details(outputStream* st) const override { > 58: st->print("%s %s ", _info.prot, _info.offset); > 59: } If that's all this is doing, do we really need it? src/hotspot/os/linux/memMapPrinter_linux.cpp line 83: > 81: char line[linesize]; > 82: while(fgets(line, sizeof(line), f) == line) { > 83: line[sizeof(line) - 1] = '\0'; What would happen if the read line is empty, i.e. sizeof(line)==0, can that happen? src/hotspot/share/nmt/memMapPrinter.cpp line 79: > 77: const void* const min = MAX2(from1, from2); > 78: const void* const max = MIN2(to1, to2); > 79: return min < max; I had to rewrite it as: `return MAX2(from1, from2) < MIN2(to1, to2);` to make sure I understand it, or better yet, why not have it as a macros? `#define RANGE_INTERSECT(min, max) (return MAX2(from1, from2) < MIN2(to1, to2))` MAX2 and MIN2 are macros already and we're not doing anything fancy here. src/hotspot/share/nmt/memMapPrinter.cpp line 89: > 87: // NMT deadlocks (ThreadCritical). > 88: Range* _ranges; > 89: MEMFLAGS* _flags; Why did you decide to have two separate memory chunks? I think I'd have used a struct to hold Range* and MEMFLAGS* and use that struct in a single array. ------------- Changes requested by gziemski (Committer). PR Review: https://git.openjdk.org/jdk/pull/16301#pullrequestreview-1704722251 PR Review Comment: https://git.openjdk.org/jdk/pull/16301#discussion_r1377905955 PR Review Comment: https://git.openjdk.org/jdk/pull/16301#discussion_r1376585794 PR Review Comment: https://git.openjdk.org/jdk/pull/16301#discussion_r1377917390 PR Review Comment: https://git.openjdk.org/jdk/pull/16301#discussion_r1377926176
