On Wed, 11 Sep 2024 18:22:00 GMT, Simon Tooke <sto...@openjdk.org> wrote:
> This is a port of #16301 to macOS. > > System.map and System.dump_map are implemented using the macOS API and > provide roughly the same information in the same format. Most of the heavy > lifting was implemented by @tstuefe in > https://github.com/openjdk/jdk/pull/16301 - this PR adds the macOS > implementation and enables the common code for macOS 64 bit. > > The System.map tests are also reworked to be cleaner for the three > implementations. > > [Sample output](https://github.com/user-attachments/files/17517412/sample.txt) Nice work, thank you. Small nits inside. I have no time to test this extensively, but if the tests run through, it should be fine. src/hotspot/os/bsd/memMapPrinter_macosx.cpp line 120: > 118: > 119: static const char* tagToStr(uint32_t user_tag) { > 120: switch (user_tag) { You could reduce the code and remove duplications with an [x macro](https://en.wikipedia.org/wiki/X_macro) if you wanted. src/hotspot/os/bsd/memMapPrinter_macosx.cpp line 360: > 358: break; > 359: } else if (retval < (int)sizeof(region_info)) { > 360: fatal("proc_pidinfo() returned %d", retval); I would make this an assert, and return without printing anything in release. ------------- PR Review: https://git.openjdk.org/jdk/pull/20953#pullrequestreview-2395601473 PR Review Comment: https://git.openjdk.org/jdk/pull/20953#discussion_r1816880506 PR Review Comment: https://git.openjdk.org/jdk/pull/20953#discussion_r1816894551
