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

[David Holmes]

On Mon, 13 May 2024 23:02:27 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:

>> Adding a few perf counters related to class loading to measure VM startup. The counters are only active if the user specifies `-Xlog:init` in the command line. A diagnostic flag `ProfileClassLinkage` is added to control the new counters. The flag is set to false by default and will be enabled if `-Xlog:init` is specified.
>> 
>> This change is already in the leyden/premain branch. There are more counters in the branch to measure other stuff. For now, just upstreaming class loader related counters.
>> 
>> Refer to the [comment](https://bugs.openjdk.org/browse/JDK-8330198?focusedId=14665311&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14665311) in the bug report for an example output.
>> 
>> Passed tiers 1 - 4 testing.
>
> Calvin Cheung has updated the pull request incrementally with one additional commit since the last revision:
> 
>   comments from Ioi

Okay my first reaction here is "I object!". I get that Leyden wants to be able to easily compare startup costs between itself and mainline, but what is this costing mainline? Even if these counters are not active there is an impact on the code execution and I want to know that impact is negligible.

The initialization logic seems a little off to me too - see comments below.

src/hotspot/share/classfile/classLoader.cpp line 1477:

> 1475: 
> 1476: jlong ClassLoader::class_init_count() {
> 1477:   return (UsePerfData) ? _perf_classes_inited->get_value() : -1;

No need to add brackets here

src/hotspot/share/classfile/classLoader.cpp line 1481:

> 1479: 
> 1480: jlong ClassLoader::class_init_time_ms() {
> 1481:   return (UsePerfData) ?

Or here

src/hotspot/share/oops/instanceKlass.cpp line 1219:

> 1217:     } else {
> 1218:       // The elapsed time is so small it's not worth counting.
> 1219:       if (UsePerfData || ProfileClassLinkage) {

You have to have UsePerfData being true for this work so you don't need the change.

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

> 3757:   if (log_is_enabled(Info, init)) {
> 3758:      FLAG_SET_ERGO_IF_DEFAULT(ProfileClassLinkage, true);
> 3759:   }

What if ProfileClassLinkage is set true on the command-line without -Xlog:init? That doesn't seem to make sense to me. So I'm not clear why it is a settable diagnostic flag.

src/hotspot/share/runtime/perfData.hpp line 834:

> 832:   public:
> 833:     inline PerfTraceTime(PerfLongCounter* timerp) : _timerp(timerp) {
> 834:       if (!UsePerfData || timerp == nullptr) return;

Okay so this is needed because the existence of some counters is gated on the ProfileClassLinkage flag.

Style nit: use a { } block please.

src/hotspot/share/runtime/perfData.hpp line 838:

> 836:     }
> 837: 
> 838:     const char* name() const { return _timerp->name(); }

Do you need a null check here?

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

PR Review: https://git.openjdk.org/jdk/pull/18790#pullrequestreview-2075233160
PR Review Comment: https://git.openjdk.org/jdk/pull/18790#discussion_r1612397674
PR Review Comment: https://git.openjdk.org/jdk/pull/18790#discussion_r1612398041
PR Review Comment: https://git.openjdk.org/jdk/pull/18790#discussion_r1612401224
PR Review Comment: https://git.openjdk.org/jdk/pull/18790#discussion_r1612410143
PR Review Comment: https://git.openjdk.org/jdk/pull/18790#discussion_r1612406745
PR Review Comment: https://git.openjdk.org/jdk/pull/18790#discussion_r1612407157