RFR: 8313306: More sensible reservation logging [v2]

[David Holmes]

On Fri, 26 Jan 2024 21:23:54 GMT, Sonia Zaldana Calles <szaldana at openjdk.org> wrote:

>> This PR implements more specialized logs for virtual memory APIs in the “os” namespace. It uses “os” and “map” as log tags using unified JVM logging as introduced in JEP 158 (https://openjdk.org/jeps/158). 
>> 
>> As far as testing is concerned, I have added a regression test to verify the logging is working accordingly.
>
> Sonia Zaldana Calles has updated the pull request incrementally with three additional commits since the last revision:
> 
>  - Implementation of high level debug logging and low level trace logging
>  - Implementation of high level debug logging
>  - Feedback - deleting oracle copyright and fixing INTPTR formatting

Moving the logging to the lower level seems an improvement but I have a few issues.

Thanks

src/hotspot/os/aix/os_aix.cpp line 300:

> 298:     if (::disclaim(p, maxDisclaimSize, DISCLAIM_ZEROMEM) != 0) {
> 299:       ErrnoPreserver ep;
> 300:       log_trace(os,map)("Cannot disclaim %p - %p (errno %d)\n", p, p + maxDisclaimSize, errno);

Using `os::strerror` or `os::errno_name` would be more user friendly than printing the numeric value. Don't simply replicate the crude approach of `trcVerbose`. This applies everywhere `errno` is being printed.

src/hotspot/os/aix/os_aix.cpp line 301:

> 299:       ErrnoPreserver ep;
> 300:       log_trace(os,map)("Cannot disclaim %p - %p (errno %d)\n", p, p + maxDisclaimSize, errno);
> 301:       trcVerbose("Cannot disclaim %p - %p (errno %d)\n", p, p + maxDisclaimSize, errno);

I don't think you need actual unified logging and the crude `trcVerbose`, but perhaps that would be a separate cleanup. Though the duplication is pretty awful.

src/hotspot/share/utilities/preserveErrno.hpp line 1:

> 1: #include <errno.h>

This file requires an OpenJDK copyright and licence header.

But do we really need a a file just for this? I think it could be added to os.hpp

src/hotspot/share/utilities/preserveErrno.hpp line 4:

> 2: 
> 3: 
> 4: // Preserve errno across a range of calls

You also need to expose the preserved value so it can be logged.

-------------

Changes requested by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17356#pullrequestreview-1847718984
PR Review Comment: https://git.openjdk.org/jdk/pull/17356#discussion_r1469005183
PR Review Comment: https://git.openjdk.org/jdk/pull/17356#discussion_r1469007301
PR Review Comment: https://git.openjdk.org/jdk/pull/17356#discussion_r1469004291
PR Review Comment: https://git.openjdk.org/jdk/pull/17356#discussion_r1469008684