RFR: 8337217: Port VirtualMemoryTracker to use VMATree [v32]

Afshin Zafari azafari at openjdk.org
Thu Feb 27 21:13:09 UTC 2025


On Thu, 27 Feb 2025 13:41:09 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   removed UseFlagInPlace test.
>
> src/hotspot/share/nmt/virtualMemoryTracker.cpp line 61:
> 
>> 59:     return _tracker->tree() != nullptr;
>> 60:   }
>> 61:   return true;
> 
> ```c++
> void* tracker = os::malloc(sizeof(VirtualMemoryTracker), mtNMT),
> if (tracker == nullptr) return false;
> _tracker = new (tracker) VirtualMemoryTracker(level == NMT_detail);

Done.

> src/hotspot/share/nmt/virtualMemoryTracker.cpp line 319:
> 
>> 317:         }
>> 318:         VirtualMemoryTracker::Instance::add_committed_region(committed_start, committed_size, ncs);
>> 319:         //log_warning(cds)("st start: " INTPTR_FORMAT " size: " SIZE_FORMAT, p2i(committed_start), committed_size);
> 
> Outdated log

Done.

> src/hotspot/share/nmt/virtualMemoryTracker.hpp line 300:
> 
>> 298: 
>> 299:  public:
>> 300:   CommittedMemoryRegion() :
> 
> Style:
> ```c==
> CommittedMemoryRegion()
>   : VirtualMemoryRegion((address)1, 1), _stack(NativeCallStack::empty_stack()) { }

Done.

> src/hotspot/share/nmt/virtualMemoryTracker.hpp line 322:
> 
>> 320:   bool is_valid() { return base() != (address)1 && size() != 1;}
>> 321:   ReservedMemoryRegion() :
>> 322:     VirtualMemoryRegion((address)1, 1), _stack(NativeCallStack::empty_stack()), _mem_tag(mtNone) { }
> 
> Style: Space between `is_valid` and constructor, fix initializer list as in `CommittedMemoryRegion`.

Done.

> src/hotspot/share/nmt/virtualMemoryTracker.hpp line 372:
> 
>> 370: class VirtualMemoryTracker {
>> 371:  private:
>> 372:   RegionsTree *_tree;
> 
> `class RegionsTree;` shouldn't be needed if you fix the circular include from above. There is no need to have the `private:` specifier. The `_tree` doesn't need to be a pointer after the forward declaration is removed, which in turn simplifies the initialization code.

new `regionsTree.inline.hpp` is introduced to resolve the dependencies.

> test/hotspot/gtest/nmt/test_nmt_treap.cpp line 30:
> 
>> 28: #include "runtime/os.hpp"
>> 29: #include "unittest.hpp"
>> 30: #include "utilities/linkedlist.hpp"
> 
> Outdated header inclusion

Removed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1974339508
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1974339232
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1974338173
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1974338397
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1974340867
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1974341165


More information about the hotspot-dev mailing list