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

Thomas Schatzl tschatzl at openjdk.org
Tue Dec 5 11:50:46 UTC 2023


> 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 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 then purge its memory later. STW collectors perform
   this work in one big chunk taking the CodeCache_lock, for the entire duration, while concurrent collectors lock/unlock for every
   insertion to allow for concurrent users for the lock to progress.
   
   Some care has been taken to stay consistent with an "unloading = unlinking + purge" scheme; however particularly the existing
   CLD handling API (still) mixes unlinking and purging in its CLD::unload() call. To simplify this change that is mostly geared
   towards separating nmethod unlinking from purging, to make code blob freeing O(n) instead of O(n^2).
   
   Upcoming changes will
   * separate nmethod unregistering from nmethod purging to allow doing that in bulk (for the STW collectors); that can significantly
     reduce code purging time for the STW collectors.
   * better name the second stage of unlinking (called "cleaning" throughout, e.g. the work done in `G1CollectedHeap::complete_cleaning`)
   * untangle CLD unlinking and what's called "cleaning" now to allow moving more stuff into the second unlinking stage for better
     parallelism
   * G1: move some signifcant tasks from the remark pause to concurrent (unregistering nmethods, freeing code blobs and cld/metaspace purging)
   * Maybe move Serial/Parallel GC metaspace purging closer to other unlinking/purging code to keep things local and allow easier logging.
 - Only run test case on debug VMs, sufficient
 - 8320331 g1 full gc "during" verification accesses half-unloaded metadata

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

Changes: https://git.openjdk.org/jdk/pull/16767/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=16767&range=02
  Stats: 123 lines in 20 files changed: 108 ins; 4 del; 11 mod
  Patch: https://git.openjdk.org/jdk/pull/16767.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16767/head:pull/16767

PR: https://git.openjdk.org/jdk/pull/16767


More information about the hotspot-gc-dev mailing list