RFR: 8266936: Add a finalization JFR event [v10]

[Coleen Phillimore]

On Mon, 13 Sep 2021 10:54:18 GMT, Markus Grönlund <mgronlun at openjdk.org> wrote:

>> src/hotspot/share/services/finalizerService.cpp line 44:
>> 
>>> 42:     _ik(ik),
>>> 43:     _objects_on_heap(0),
>>> 44:     _total_finalizers_run(0) {}
>> 
>> Is this hashtable for every InstanceKlass that defines a finalizer?  How many are there generally?  Why couldn't this be a simple hashtable like ResourceHashtable (soon to be renamed) which you can write in only a few lines of code?
>
> This hashtable holds a FinalizerEntry for every InstanceKlass that provides a non-empty finalizer method and have allocated at least one object. How many there are in general depends on the application. A ResourceHashtable does not have the concurrency property required, as lookups and inserts will happen as part of object allocation.

ok.

>> src/hotspot/share/services/finalizerService.cpp line 331:
>> 
>>> 329:   }
>>> 330:   Thread* const thread = Thread::current();
>>> 331:   // We use current size
>> 
>> Since you remove entries on unloading, I don't see any reason to have any concurrent cleanup.
>> You do however need in the lookup code, some code that doesn't find the InstanceKlass if !ik->is_loader_alive()
>
> "Since you remove entries on unloading, I don't see any reason to have any concurrent cleanup."
> Thank you, that is true. The only concurrent work required will be to grow the table.
> 
> "You do however need in the lookup code, some code that doesn't find the InstanceKlass if !ik->is_loader_alive()"
> 
> A precondition is that the code doing the lookup hold the ClassLoaderDataGraph_lock or is at a safepoint. Is that still the case? Would not purge_unloaded() take out the InstanceKlass from the table, as part of unloading, before !ik->is_loader_alive() is published to the outside world?

Ok, I see - grow the table.

I'm not sure if you need to ask ik->is_loader_alive() or not, but I think you do.  The InstanceKlass is removed from your table during class unloading but before that during concurrent class unloading, the class might not be alive while you look at it and concurrent class unloading hasn't gotten around to removing it yet.  Since you save classes regardless of CLD, you have to check if it's alive.  See classLoaderDataGraph.cpp ClassLoaderDataGraphIterator for example.  The CLDG_lock only keeps the graph from not getting modified, but the classes in it might be dead.

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

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