RFR: 8328944: NMT reports "unknown" memory [v2]

Johan Sjölen jsjolen at openjdk.org
Tue Nov 19 22:01:14 UTC 2024


On Tue, 19 Nov 2024 21:57:52 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

>> We use `mtNone` value in several functions default parameters, which may show up in NMT reports.
>> 
>> We address this, by avoiding using `mtNone`.
>> 
>> Eventually the goal would be to eliminate using `mtNone` whenever possible.
>> TODO: update copyrights.
>> 
>> Testing: undergoing MARCH5 tier1-5 ...
>
> Gerard Ziemski has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains six additional commits since the last revision:
> 
>  - avoid using mtNone
>  - Merge remote-tracking branch 'upstream/master' into JDK-8328944
>  - revert, we will re-do with a smaller change
>  - remove more mtNone
>  - remove API that allows to change the mem_tag for virtual memory, feedback
>  - do not allow default parameter for mtNone

Why should we supplement `mtNone` with `mtAllocated`? Is one indicative of a failure mode?

src/hotspot/share/nmt/virtualMemoryTracker.hpp line 389:

> 387:   static bool remove_released_region    (address base_addr, size_t size);
> 388:   static bool remove_released_region    (ReservedMemoryRegion* rgn);
> 389:   static void set_reserved_region_type  (address addr, MemTag mem_tag);

I don't think that we should remove this API. It may very well need to be used, even if its usage should be minimized.

src/hotspot/share/runtime/os.hpp line 460:

> 458:   // Attempts to reserve the virtual memory at [addr, addr + bytes).
> 459:   // Does not overwrite existing mappings.
> 460:   static char*  attempt_reserve_memory_at(char* addr, size_t bytes, bool executable, MemTag mem_tag);

Is it worth adding an overload: `reserve_memory(bytes, mem_tag)` where `executable` is implicitly `false`?

test/hotspot/gtest/memory/test_virtualspace.cpp line 374:

> 372:                      alignment,       // alignment
> 373:                      page_size,       // page size
> 374:                      (char *)nullptr, // requested_address

Style fix: Remove space between `char` and `*`

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

PR Review: https://git.openjdk.org/jdk/pull/21843#pullrequestreview-2442231730
PR Review Comment: https://git.openjdk.org/jdk/pull/21843#discussion_r1846345086
PR Review Comment: https://git.openjdk.org/jdk/pull/21843#discussion_r1846346929
PR Review Comment: https://git.openjdk.org/jdk/pull/21843#discussion_r1846350091


More information about the shenandoah-dev mailing list