RFR: 8275729: Qualified method names in CodeHeap Analytics

Evgeny Astigeevich duke at openjdk.java.net
Tue Nov 2 17:08:17 UTC 2021


On Tue, 2 Nov 2021 16:34:34 GMT, Volker Simonis <simonis at openjdk.org> wrote:

>> This PR changes nmethods names in `METHOD NAMES for CodeHeap` section to be  qualified.
>> Testing:
>> - `make test TEST="gtest"`:  Passed
>> - `make run-test TEST="tier1"`: Passed
>> - `make run-test TEST="tier2"`: Passed
>> - `make run-test TEST=serviceability/dcmd/compiler/CodeHeapAnalyticsMethodNames.java`: Passed
>
> src/hotspot/share/code/codeHeapState.cpp line 2340:
> 
>> 2338: 
>> 2339:             Klass* klass = method->method_holder();
>> 2340:             assert(klass->is_loader_alive(), "must be alive");
> 
> Are you sure `klass` is always valid here and that its class loader has to be alive (i.e. the corresponding class hasn't been unloaded in the meantime)?
> 
> In [https://bugs.openjdk.java.net/browse/JDK-8275729](JDK-8275729) you say that the Top50 list already has qualified names but as far as I know, that information is already collected in the aggregation step where it is safe. You now query this information in the reporting step.
> 
> I know we had problems due to access to dead methods before (see [JDK-8219586: CodeHeap State Analytics processes dead nmethods](https://bugs.openjdk.java.net/browse/JDK-8219586) and I just want to make sure we don't re-introduce such problems.
> 
> Maybe @RealLucy or @fisk can have an additional look?

@simonis 
The code is guarded by checks:

        // access nmethod and Method fields only if we own the CodeCache_lock.
        // This fact is implicitly transported via nm != NULL.
        if (nmethod_access_is_safe(nm)) {
        ...
          bool         get_name   = (cbType == nMethod_inuse) || (cbType == nMethod_notused);
        ...
          if (get_name) {

I was thinking whether I should use `if (klass->is_loader_alive())` or `assert(klass->is_loader_alive())`. I chose the assert because if it is safe to access `Method` than its holder `Klass` must be alive.

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

PR: https://git.openjdk.java.net/jdk/pull/6200


More information about the serviceability-dev mailing list