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

[Thomas Schatzl]

On Thu, 7 Dec 2023 11:12:13 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Hi all,
>> 
>>   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 filed JDK-8320067. There should already be a significant speedup with this change, 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_not_into_young` which removes elements from the code root sets that do no longer need to be remembered (due to object movement) with removing dead/unlinked nmethods. However, this would mean to put that class unloading code to the end of gc 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 unlinking nmethods and class/code unloading them together in the code, but I could be convinced to move it.
>>     
>> Testing: tier1-4
>> 
>> Depends on #16759, please review first.
>> 
>> Thanks,
>>   Thomas
>
> Thomas Schatzl has updated the pull request incrementally with one additional commit since the last revision:
> 
>   ayang review; merge fixes :(

> Here are two before/after graphs for this change on ccstress (also added to the CR):
> 
> The next one shows Remark Pause time before and after this change on the ccstress benchmark (from JDK-8315503).
> 
> ![CCStress Remark Pause time before after 20231207](https://private-user-images.githubusercontent.com/59967451/288774430-c96cd052-334f-4500-b065-98dcdddb3989.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTEiLCJleHAiOjE3MDE5NTY1ODksIm5iZiI6MTcwMTk1NjI4OSwicGF0aCI6Ii81OTk2NzQ1MS8yODg3NzQ0MzAtYzk2Y2QwNTItMzM0Zi00NTAwLWIwNjUtOThkY2RkZGIzOTg5LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFJV05KWUFYNENTVkVINTNBJTJGMjAyMzEyMDclMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjMxMjA3VDEzMzgwOVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWEwYjNkZGZjY2Y0ZTYwY2M2MmY3OGM3MmExMGFjN2I4NDM3N2I1MzkwYmJmNTBlMDIxYzg5YTkwYzkyMDM1NzMmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.TBJKqEqoPntBrGpXyQy3InenRmgHmKJ3OnM0tlU6_38)
> 

The large peaks at second 180 and 225 in the graph are caused by class loader/metaspace purging unrelated to this change.

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

PR Comment: https://git.openjdk.org/jdk/pull/16767#issuecomment-1845365557