RFR: 8330198: Add some class loading related perf counters to measure VM startup [v8]

[David Holmes]

On Tue, 11 Jun 2024 00:06:53 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:

>> src/hotspot/share/runtime/java.cpp line 164:
>> 
>>> 162:   if (log.is_enabled()) {
>>> 163:     ClassLoader::print_counters(st);
>>> 164:   }
>> 
>> Probably worth adding a comment here as to why we actually print to the passed in stream and not the log stream., given we check if the log stream is enabled. Someone could easily think this is a typo/bug.
>
> I can change `log_vm_stats` to accept a `bool` argument so that the the `st` becomes clear.
> 
> 
> void log_vm_stats(bool use_tty) {
>   LogStreamHandle(Info, perf, class, link) log;
>   if (log.is_enabled()) {
>     outputStream* st = use_tty ? tty : &log;
>     ClassLoader::print_counters(st);
>   }
> }

That makes the tty usage a lot clearer - thanks.

>> src/hotspot/share/runtime/threads.cpp line 835:
>> 
>>> 833:     log.print_cr("At VM initialization completion:");
>>> 834:     log_vm_stats(&log);
>>> 835:   }
>> 
>> If we are going to have more types of VM stats in the future, it is not clear how you will change this if-condition? Nor what stream you would pass in. ???
>
> The if-condition could be something like:
> 
> 
> if (log_is_enabled(Info, perf, class, link) ||
>      log_is_enabled(Info, perf, xxx, yyy) ||
>      ...)
> 
> 
> Regarding which stream to pass in, with my proposed change in `log_vm_stats` above, the current fix would look like when calling from threads.cpp:
> `log_vm_stats(false /* use_tty */);`
> when calling from java.cpp:
> `log_vm_stats(true /* use_tty */);`
> 
> Or do you prefer not having the `log_vm_stats` function and calling `ClassLoader::print_counters` directly?
> If so, we don't need the compound `if` conditions in the above.

The problem is that the two different logging configurations could have been given different destinations and need not write to the same "stream".

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18790#discussion_r1635735627
PR Review Comment: https://git.openjdk.org/jdk/pull/18790#discussion_r1635735421