On Fri, 28 Apr 2023 12:48:44 GMT, Stefan Johansson <sjoha...@openjdk.org> wrote:
> Hi all, > > Please review this change to avoid CleanClassLoaderDataMetaspaces safepoint > when there is nothing that can be cleaned up. > > **Summary** > When transforming/redefining classes a previous version list is linked > together in the InstanceKlass. The original class is added to this list if it > is still used or shared. The difference between shared and used is not > currently noted. This leads to a problem when doing concurrent class > unloading, because during that we postpone some potential work to a safepoint > (since we are not in one). This is the CleanClassLoaderDataMetaspaces and it > is triggered by the ServiceThread if there is work to be done, for example if > InstanceKlass::_has_previous_versions is true. > > Since we currently does not differentiate between shared and "in use" we > always set _has_previous_versions if anything is on this list. This together > with the fact that shared previous versions should never be cleaned out leads > to this safepoint being triggered after every concurrent class unloading even > though there is nothing that can be cleaned out. > > This can be avoided by making sure the _previous_versions list is only > cleaned when there are non-shared classes on it. This change renames > `_has_previous_versions` to `_clean_previous_versions` and only updates it if > we have non-shared classes on the list. > > **Testing** > * A lot of manual testing verifying that we do get the safepoint when we > should. > * Added new test to verify expected behavior by parsing the logs. The test > uses JFR to trigger redefinition of some shared classes (when -Xshare:on). > * Mach5 run of new test and tier 1-3 This looks good. Thanks for all the testing and adding the new test. ------------- Marked as reviewed by coleenp (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13716#pullrequestreview-1406222927
