On Fri, 27 Oct 2023 09:42:19 GMT, Johan Sjölen <[email protected]> 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
