[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8306929: Avoid CleanClassLoaderDataMetaspaces safepoints when previous versions are shared
From:       Coleen Phillimore <coleenp () openjdk ! org>
Date:       2023-04-28 16:16:23
Message-ID: ApQG7C_ZSdVZ9oai99pQvCLcehPVpzEUpPUl6XZoDRQ=.df09c689-ab40-44d1-b821-986aa69c458b () github ! com
[Download RAW message or body]

On Fri, 28 Apr 2023 12:48:44 GMT, Stefan Johansson <sjohanss@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


[prev in list] [next in list] [prev in thread] [next in thread]