RFR: 8317007: Add bulk removal of dead nmethods during class unloading [v3]

[Thomas Schatzl]

On Wed, 6 Dec 2023 17:54:53 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> Thomas Schatzl has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains six commits:
>> 
>>  - Remove some dead code introduced by merge
>>  - Merge branch 'master' of https://git.openjdk.org/jdk into submit/8317007-nmethod-bulk-removal
>>  - 8317007
>>    
>>    please review this change that bulk-removes dead nmethods for the STW collectors instead of unregistering nmethod by nmethod. This significantly speeds up the class unloading phase.
>>    
>>    For G1, this is almost 100% the code that has been removed from the review for JDK-8315503.
>>    
>>    For Serial and Parallel GC, the code is new.
>>    
>>    This change does not try to improve the situation for concurrent collectors - this would at first glance require extending the scope of the CodeCache_lock which I did not want to do. See the CR for more details.
>>    
>>    Also, no parallelism for Parallel GC: the existing data structure for code root remembered set is a linked list. There is in almost all cases no point in trying to parallelize what is basically a traversal of the linked list (with each element not having a lot of work to do). I file JDK-8320067. There should still be a significant speedup, as each unregister_nmethod call previously traversed the entire linked list to find that element (i.e. complexity reduction of O(n^2) -> O(n)).
>>    
>>    One more comment:
>>    So it would be possible to merge the ScavengableNMethod::prune_nmethods which removes elements from the code root sets that do no longer need to be remembered (due to object movement) and removing dead nmethods.
>>    However, this would mean to put that code at the end of gc (where prune_nmethods is called now) as during phase 1 the new locations which are relevant for pruning uninteresting code root remembered set entries did not move yet. I wanted to keep determining the dead nmethods and removing them together in the code, but I could be convinced to move it.
>>    
>>    Testing: tier1-4
>>  - 8317809 Insert code blobs in a sorted fashion to exploit the finger-optimization when adding, making this procedure O(n) instead of O(n^2)
>>    
>>    Introduce a globally available ClassUnloadingContext that contains common methods pertaining to class and code unloading.
>>    GCs may use it to efficiently manage unlinked class loader datas and nmethods to allow use of common methods (unlink/merge).
>>    
>>    The steps typically are registering a new to be unlinked CLD/nmethod, and the...
>
> src/hotspot/share/code/nmethod.cpp line 1467:
> 
>> 1465:   }
>> 1466: 
>> 1467:   if (unregister_nmethods) {
> 
> Could it be named sth like "unregister_individual_nmethod" to emphasize that this operation is on a single nmethod?

Since `purge()` works on an individual nmethod, this seems implied and unnecessary. I would even say that the suggestion is confuses more than it helps (why would an operation that gets passed a single nmethod called a single nmethod do unregistering of anything else than that single nmethod?).

I will remove the "s" though which may have been the cause of the confusion.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16767#discussion_r1418506492