RFR: 8317453: NMT: Performance benchmarks are needed to measure speed and memory [v15]

[Johan Sjölen]

On Wed, 7 May 2025 17:34:09 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

>> Please review this addition of an internal benchmark, mostly of interest to those working with NMT.
>> 
>> This benchmark allows us to record a pattern of memory allocation operations (i.e. `malloc`, `realloc` and `free`) as well as the virtual memory allocations (i.e. `VirtualMemoryTracker::add_reserved_region`, etc.) and record those into files.
>> 
>> Later we can use that recording to _play back_ the pattern with different code or settings to compare the performance (i.e. memory usage as well as time).
>> 
>> The goal of this benchmark is for anyone working on NMT to be able to measure and prove whether their improvement helps or regresses the performance.
>> 
>> ### To use it:
>> 
>> To record pattern of allocations of memory calls:
>> 
>> `NMTRecordMemoryAllocations=0x7FFFFFFF ./build/macosx-aarch64-server-release/xcode/build/jdk/bin/java -XX:+UnlockDiagnosticVMOptions -XX:NativeMemoryTracking=summary -jar build/macosx-aarch64-server-release/images/jdk/demo/jfc/J2Ddemo/J2Ddemo.jar`
>> 
>> OR to record pattern of allocations of virtual memory calls:
>> 
>> `NMTRecordVirtualMemoryAllocations=0x7FFFFFFF ./build/macosx-aarch64-server-release/xcode/build/jdk/bin/java -XX:+UnlockDiagnosticVMOptions -XX:NativeMemoryTracking=summary -jar build/macosx-aarch64-server-release/images/jdk/demo/jfc/J2Ddemo/J2Ddemo.jar`
>> 
>> This will result in the file:
>> -  hs_nmt_pid22770_allocs_record.log (is the chronological record of the the desired operations)
>> OR
>> - hs_nmt_pid22770_virtual_allocs_record.log (is the chronological record of the desired operations)
>> 
>> And 2 additional files:
>> - hs_nmt_pid22770_info_record.log (is the record of default NMT memory overhead and the NMT state)
>> - hs_nmt_pid22770_threads_record.log (is the record of thread names that can be retrieved later when processing)
>> 
>> 
>> then to actually run the benchmark:
>> 
>> NMTBenchmarkRecordedPID=22770 ./build/macosx-aarch64-server-release/xcode/build/jdk/bin/java -XX:+UnlockDiagnosticVMOptions -XX:NativeMemoryTracking=summary
>> 
>> ### Usage:
>> 
>> See the issue for more details and the design document.
>
> Gerard Ziemski has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Johan feedback, remove unused parameter

src/hotspot/share/nmt/memLogRecorder.cpp line 93:

> 91: 
> 92: 
> 93: #define NMT_HEADER_SIZE 16

Do not use `#define` for the constant, use a static const, and don't put it in as a magic number but find it out from a `sizeof` on the appropriate type.

src/hotspot/share/nmt/memLogRecorder.hpp line 67:

> 65: protected:
> 66:   volatile size_t _threads_names_size = 0;
> 67:   typedef struct thread_name_info {

Remove the `typedef`, not necessary in C++.

src/hotspot/share/nmt/memLogRecorder.hpp line 71:

> 69:     long int thread;
> 70:   } thread_name_info;
> 71:   thread_name_info *_threads_names = nullptr;

Let the * hug the typename, not the variable.

src/hotspot/share/nmt/memLogRecorder.hpp line 94:

> 92: 
> 93: private:
> 94:     struct Entry {

wrong indentation, and also pull out the common stuff from the two `Entry` structs we have into a superclass.

src/hotspot/share/runtime/arguments.cpp line 3936:

> 3934: // 2. The passed in "buflen" should be large enough to hold the null terminator.
> 3935: bool Arguments::copy_expand_pid(const char* src, size_t srclen,
> 3936:                                 char* buf, size_t buflen, int pid) {

What does this change do? Why is it needed?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23786#discussion_r2079041839
PR Review Comment: https://git.openjdk.org/jdk/pull/23786#discussion_r2079028654
PR Review Comment: https://git.openjdk.org/jdk/pull/23786#discussion_r2079029095
PR Review Comment: https://git.openjdk.org/jdk/pull/23786#discussion_r2079030731
PR Review Comment: https://git.openjdk.org/jdk/pull/23786#discussion_r2079032694