On Fri, 27 Oct 2023 09:42:19 GMT, Johan Sjölen <jsjo...@openjdk.org> wrote:
>> Thomas Stuefe has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains four commits: >> >> - Merge master and solve merge conflicts >> - small fixes >> - start from VM op; show more thread details >> - start > > Hi, > > Thank you for this PR. These comments are just a first pass, I haven't > finished going through the code. @jdksjolen Many thanks for the review. I fixed most of your requests, and also added a simple timeout fuse to prevent the printing from taking overly long in the rare case of insanely large or fragmented process memory maps. > src/hotspot/os/linux/memMapPrinter_linux.cpp line 32: > >> 30: #include "utilities/globalDefinitions.hpp" >> 31: >> 32: struct proc_maps_info_t { > > `struct ProcMapsInfo` please. The `_t` is apparently reserved for POSIX, and > I prefer our structs and classes to not look like they're coming from a C > library anyway. ok ------------- PR Comment: https://git.openjdk.org/jdk/pull/16301#issuecomment-1782829434 PR Review Comment: https://git.openjdk.org/jdk/pull/16301#discussion_r1374499761