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

[Gerard Ziemski]

On Thu, 8 May 2025 06:57:04 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:
>> 
>>   Johan feedback, remove unused parameter
>
> 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++.

Hmm, without it, I get a `must use 'struct' tag to refer to type 'thread_name_info' in this scope` error here:

`  thread_name_info* _threads_names = nullptr;
`

I could fix it by putting `struct` in front of `thread_name_info` every time I use, but I'd rather keeep:

`typedef struct thread_name_info {`

and not worry anymore about it.

> 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.

Done.

> 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?

It allows us to re-use existing hotspot code by accepting pid into the resulting string, which we need for custom log file names with pid.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23786#discussion_r2079838024
PR Review Comment: https://git.openjdk.org/jdk/pull/23786#discussion_r2079839199
PR Review Comment: https://git.openjdk.org/jdk/pull/23786#discussion_r2079841987