On Wed, 3 May 2023 08:36:22 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 > > Stefan Johansson has updated the pull request incrementally with two > additional commits since the last revision: > > - Test refactor > - Serguei review Thank you for the update. Looks good. Thanks, Serguei ------------- Marked as reviewed by sspitsyn (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13716#pullrequestreview-1411506472
