On Fri, 28 Apr 2023 12:48:44 GMT, Stefan Johansson <[email protected]> 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
src/hotspot/share/oops/instanceKlass.hpp line 718:
> 716:
> 717: private:
> 718: static bool _clean_previous_versions;
Nit: I'd suggest to name it as `_should_clean_previous_versions`.
Then the corresponding function needs to be named as
`should_clean_previous_versions()`.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13716#discussion_r1182136483
