RFR: 8275729: Qualified method names in CodeHeap Analytics

[Lutz Schmidt]

On Tue, 2 Nov 2021 17:03:50 GMT, Evgeny Astigeevich <duke at openjdk.java.net> wrote:

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

Hi,
the code is safe. Not because of the checks cited by @eastig but because print_names() is only called if the required locks (Compile_lock and CodeCache_lock) have been continuously held since the aggregation step. See src/hotspot/share/compiler/compileBroker.cpp. A lot of effort has been spent to be less restrictive on print_names(), with no success. 
Thanks for the enhancement.

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

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