On Mon, 13 Sep 2021 10:54:18 GMT, Markus Grönlund <mgron...@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

Reply via email to