RFR: 8313306: More sensible reservation logging [v2]

Thomas Stuefe stuefe at openjdk.org
Tue Jan 30 10:39:41 UTC 2024


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

Good. I like the test. Some nits. See also David's comments.

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

> 308:     if (::disclaim(p, lastDisclaimSize, DISCLAIM_ZEROMEM) != 0) {
> 309:       ErrnoPreserver ep;
> 310:       log_trace(os,map)("Cannot disclaim %p - %p (errno %d)\n", p, p + lastDisclaimSize, errno);

Use RANGEFMT

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

> 1477:               p2i(p), p2i(p + s), p2i(addr), p2i(addr + size));
> 1478:       log_trace(os,map)("[" PTR_FORMAT " - " PTR_FORMAT "] is not a sub "
> 1479:               "range of [" PTR_FORMAT " - " PTR_FORMAT "].",

Use RANGEFMT

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

> 1485:               " aligned to pagesize (%lu)", p2i(p), p2i(p + s), (unsigned long) pagesize);
> 1486:       log_trace(os,map)("range [" PTR_FORMAT " - " PTR_FORMAT "] is not"
> 1487:               " aligned to pagesize (%lu)", p2i(p), p2i(p + s), (unsigned long) pagesize);

RANGEFMT

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

> 1554:   if (requested_addr != nullptr && is_close_to_brk((address)requested_addr)) {
> 1555:     trcVerbose("Wish address " PTR_FORMAT " is too close to the BRK segment.", p2i(requested_addr));
> 1556:     log_trace(os,map)("Wish address " PTR_FORMAT " is too close to the BRK segment.", p2i(requested_addr));

Please make this one info level, this is important.

src/hotspot/share/runtime/os.cpp line 1815:

> 1813:   if (result != nullptr) {
> 1814:     MemTracker::record_virtual_memory_reserve(result, bytes, CALLER_PC, flags);
> 1815:     log_debug(os,map)("Reserved [" INTPTR_FORMAT " - " INTPTR_FORMAT "] (%zu bytes).", p2i(result), p2i(result + bytes), bytes);

RANGEFMT

src/hotspot/share/runtime/os.cpp line 1829:

> 1827:   } else {
> 1828:     log_info(os,map)("Attempt to reserve [" INTPTR_FORMAT " - " INTPTR_FORMAT "] (%zu bytes) failed",
> 1829:       p2i(addr), p2i(addr + bytes), bytes);

RANGEFMT (will stop now; please convert all range specs to this format)

src/hotspot/share/utilities/globalDefinitions.hpp line 383:

> 381: 
> 382: #define RANGEFMT                      "[" PTR_FORMAT " - " PTR_FORMAT "], (" SIZE_FORMAT " bytes);"
> 383: #define RANGEFMTARGS(p1, size)    p2i(p1), p2i(p1 + size -1), size

Please change this to display the typical "[..)" range (e.g. "[0xC0000000 - 0xE000000)" ). Using the [) notation makes it very clear that the last address is excluded.

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

Changes requested by stuefe (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17356#pullrequestreview-1850790564
PR Review Comment: https://git.openjdk.org/jdk/pull/17356#discussion_r1470960856
PR Review Comment: https://git.openjdk.org/jdk/pull/17356#discussion_r1470961130
PR Review Comment: https://git.openjdk.org/jdk/pull/17356#discussion_r1470961403
PR Review Comment: https://git.openjdk.org/jdk/pull/17356#discussion_r1470962239
PR Review Comment: https://git.openjdk.org/jdk/pull/17356#discussion_r1470966741
PR Review Comment: https://git.openjdk.org/jdk/pull/17356#discussion_r1470967444
PR Review Comment: https://git.openjdk.org/jdk/pull/17356#discussion_r1470969695


More information about the hotspot-runtime-dev mailing list