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

[David Holmes]

On Wed, 29 May 2024 05:02:05 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:

>> src/hotspot/share/runtime/java.cpp line 165:
>> 
>>> 163:     ClassLoader::print_counters();
>>> 164:   }
>>> 165: }
>> 
>> This method seems unnecessary. Inside `print_counters` it checks if the log is enabled and whether `ProfileClassLinkage` is set, so no need to check the log is enabled here. Wherever this is called you should just call `ClassLoader::print_counters` directly. (Further the "init" part of the name is only meaningful for the call site at the end of VM initialization.)
>
> This function will cover other sets of counters in the future. Maybe changing its name to `log_vm_stats`?

Regardless there seems to be confusion about which method should be responsible for checking if the requisite logging is enabled. They should not both do it.

>> src/hotspot/share/runtime/java.cpp line 367:
>> 
>>> 365:   ThreadsSMRSupport::log_statistics();
>>> 366: 
>>> 367:   log_vm_init_stats();
>> 
>> Do we really want to call `ClassLoader::print_counters` here? IIUC most everything else here is printing to tty, but `ClassLoader::print_counters` will "print" to whereever the logging has been configured. (` ThreadsSMRSupport::log_statistics` seems similarly misplaced as it too uses logging).
>
> If using `tty`,  the output would lose the logging tag. The output would look as follows:
> 
> ClassLoader:
>   clinit:               11ms / 285 events
>   link methods:         13ms / 7493 events
>   method adapters:      12ms / 571 events
> 
> versus with logging tag:
> 
> [0.094s][info][init] ClassLoader:
> [0.094s][info][init]   clinit:               11ms / 278 events
> [0.094s][info][init]   link methods:         13ms / 7336 events
> [0.094s][info][init]   method adapters:      12ms / 571 events

Yes I understand that, but this method is generally printing a ton of stuff to the tty - that is what it is for. If we want to add such stuff to the output then it too should just go to the tty - else it doesn't belong in this method IMO.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18790#discussion_r1618341748
PR Review Comment: https://git.openjdk.org/jdk/pull/18790#discussion_r1618343557