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

[Gerard Ziemski]

On Mon, 5 May 2025 13:07:58 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> Gerard Ziemski has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   use permit_forbidden_function for realloc
>
> src/hotspot/share/nmt/memLogRecorder.hpp line 151:
> 
>> 149:     SPLIT_RESERVED,
>> 150:     TAG
>> 151:   };
> 
> Better name than `Type`, like `MemoryOperation`? No need for `ALL_CAPS` names if you don't want to, you can use `ThisTypeOfName` instead. That's a style choice you get to make, though.

I like your suggestion, done.

I renamed Type -> MemoryOperation, but I kind of like the constants CAPITALIZED.

> src/hotspot/share/runtime/os.cpp line 739:
> 
>> 737:     // After a successful realloc(3), we account the resized block with its new size
>> 738:     // to NMT.
>> 739:     void* const new_inner_ptr = MemTracker::record_malloc(new_outer_ptr, size, mem_tag, stack, memblock);
> 
> Unused extra argument

We do use it:

 ```
 static inline void* record_malloc(void* mem_base, size_t size, MemTag mem_tag,
    const NativeCallStack& stack, void* old_base = nullptr) {
    assert(mem_base != nullptr, "caller should handle null");
    void* ptr = mem_base;
    if (enabled()) {
      ptr = MallocTracker::record_malloc(mem_base, size, mem_tag, stack, old_base);
    }
    NMT_MemoryLogRecorder::record_alloc(mem_tag, size, mem_base, &stack, old_base);
    return ptr;
  }

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23786#discussion_r2077958015
PR Review Comment: https://git.openjdk.org/jdk/pull/23786#discussion_r2077966039