diff --git a/src/hotspot/share/classfile/classLoaderDataGraph.cpp b/src/hotspot/share/classfile/classLoaderDataGraph.cpp
index 72a1e78838f2e18f6ba4ce41c28f204e50252bc2..c1fb1d68f4b1b813d49ba4367d09634ab61f29c5 100644
--- a/src/hotspot/share/classfile/classLoaderDataGraph.cpp
+++ b/src/hotspot/share/classfile/classLoaderDataGraph.cpp
@@ -186,7 +186,7 @@ void ClassLoaderDataGraph::walk_metadata_and_clean_metaspaces() {
// on the stack or in the code cache, so we only have to repeat the full walk if
// they were found at that time.
// TODO: have redefinition clean old methods out of the code cache. They still exist in some places.
- bool walk_all_metadata = InstanceKlass::has_previous_versions_and_reset();
+ bool walk_all_metadata = InstanceKlass::should_clean_previous_versions_and_reset();
MetadataOnStackMark md_on_stack(walk_all_metadata, /*redefinition_walk*/false);
clean_deallocate_lists(walk_all_metadata);
diff --git a/src/hotspot/share/classfile/classLoaderDataGraph.inline.hpp b/src/hotspot/share/classfile/classLoaderDataGraph.inline.hpp
index e5b51ac277cc3132470d481f5e182c19fb22ac19..e4b10f95656b9aae8ddcb887f01e927403bd7456 100644
--- a/src/hotspot/share/classfile/classLoaderDataGraph.inline.hpp
+++ b/src/hotspot/share/classfile/classLoaderDataGraph.inline.hpp
@@ -73,7 +73,7 @@ bool ClassLoaderDataGraph::should_clean_metaspaces_and_reset() {
// Only clean metaspaces after full GC.
bool do_cleaning = _safepoint_cleanup_needed;
#if INCLUDE_JVMTI
- do_cleaning = do_cleaning && (_should_clean_deallocate_lists || InstanceKlass::has_previous_versions());
+ do_cleaning = do_cleaning && (_should_clean_deallocate_lists || InstanceKlass::should_clean_previous_versions());
#else
do_cleaning = do_cleaning && _should_clean_deallocate_lists;
#endif
diff --git a/src/hotspot/share/oops/instanceKlass.cpp b/src/hotspot/share/oops/instanceKlass.cpp
index fdee66a8866e315b746eb92712d8ca9af72165c0..2e9a5125014b0618ee1abe37110f9298e2693269 100644
--- a/src/hotspot/share/oops/instanceKlass.cpp
+++ b/src/hotspot/share/oops/instanceKlass.cpp
@@ -3941,18 +3941,18 @@ void InstanceKlass::set_init_state(ClassState state) {
// Globally, there is at least one previous version of a class to walk
// during class unloading, which is saved because old methods in the class
// are still running. Otherwise the previous version list is cleaned up.
-bool InstanceKlass::_has_previous_versions = false;
+bool InstanceKlass::_should_clean_previous_versions = false;
// Returns true if there are previous versions of a class for class
// unloading only. Also resets the flag to false. purge_previous_version
// will set the flag to true if there are any left, i.e., if there's any
// work to do for next time. This is to avoid the expensive code cache
// walk in CLDG::clean_deallocate_lists().
-bool InstanceKlass::has_previous_versions_and_reset() {
- bool ret = _has_previous_versions;
- log_trace(redefine, class, iklass, purge)("Class unloading: has_previous_versions = %s",
+bool InstanceKlass::should_clean_previous_versions_and_reset() {
+ bool ret = _should_clean_previous_versions;
+ log_trace(redefine, class, iklass, purge)("Class unloading: should_clean_previous_versions = %s",
ret ? "true" : "false");
- _has_previous_versions = false;
+ _should_clean_previous_versions = false;
return ret;
}
@@ -4027,12 +4027,17 @@ void InstanceKlass::purge_previous_version_list() {
version++;
continue;
} else {
- log_trace(redefine, class, iklass, purge)("previous version " INTPTR_FORMAT " is alive", p2i(pv_node));
assert(pvcp->pool_holder() != NULL, "Constant pool with no holder");
guarantee (!loader_data->is_unloading(), "unloaded classes can't be on the stack");
live_count++;
- // found a previous version for next time we do class unloading
- _has_previous_versions = true;
+ if (pvcp->is_shared()) {
+ // Shared previous versions can never be removed so no cleaning is needed.
+ log_trace(redefine, class, iklass, purge)("previous version " PTR_FORMAT " is shared", p2i(pv_node));
+ } else {
+ // Previous version alive, set that clean is needed for next time.
+ _should_clean_previous_versions = true;
+ log_trace(redefine, class, iklass, purge)("previous version " PTR_FORMAT " is alive", p2i(pv_node));
+ }
}
// next previous version
@@ -4132,13 +4137,19 @@ void InstanceKlass::add_previous_version(InstanceKlass* scratch_class,
return;
}
- // Add previous version if any methods are still running.
- // Set has_previous_version flag for processing during class unloading.
- _has_previous_versions = true;
- log_trace(redefine, class, iklass, add) ("scratch class added; one of its methods is on_stack.");
+ // Add previous version if any methods are still running or if this is
+ // a shared class which should never be removed.
assert(scratch_class->previous_versions() == NULL, "shouldn't have a previous version");
scratch_class->link_previous_versions(previous_versions());
link_previous_versions(scratch_class);
+ if (cp_ref->is_shared()) {
+ log_trace(redefine, class, iklass, add) ("scratch class added; class is shared");
+ } else {
+ // We only set clean_previous_versions flag for processing during class
+ // unloading for non-shared classes.
+ _should_clean_previous_versions = true;
+ log_trace(redefine, class, iklass, add) ("scratch class added; one of its methods is on_stack.");
+ }
} // end add_previous_version()
#endif // INCLUDE_JVMTI
diff --git a/src/hotspot/share/oops/instanceKlass.hpp b/src/hotspot/share/oops/instanceKlass.hpp
index 23b773d0195bbf4e6522e76d04f244e3b7a6631e..9f03bc50d4886483418c30ec6b4f9b7f51030bc6 100644
--- a/src/hotspot/share/oops/instanceKlass.hpp
+++ b/src/hotspot/share/oops/instanceKlass.hpp
@@ -815,7 +815,7 @@ public:
}
private:
- static bool _has_previous_versions;
+ static bool _should_clean_previous_versions;
public:
static void purge_previous_versions(InstanceKlass* ik) {
if (ik->has_been_redefined()) {
@@ -823,8 +823,8 @@ public:
}
}
- static bool has_previous_versions_and_reset();
- static bool has_previous_versions() { return _has_previous_versions; }
+ static bool should_clean_previous_versions_and_reset();
+ static bool should_clean_previous_versions() { return _should_clean_previous_versions; }
// JVMTI: Support for caching a class file before it is modified by an agent that can do retransformation
void set_cached_class_file(JvmtiCachedClassFileData *data) {
@@ -844,7 +844,7 @@ public:
#else // INCLUDE_JVMTI
static void purge_previous_versions(InstanceKlass* ik) { return; };
- static bool has_previous_versions_and_reset() { return false; }
+ static bool should_clean_previous_versions_and_reset() { return false; }
void set_cached_class_file(JvmtiCachedClassFileData *data) {
assert(data == NULL, "unexpected call with JVMTI disabled");
diff --git a/test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefinePreviousVersions.java b/test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefinePreviousVersions.java
index 12aefba51e5bc94aa916112fc7e7f162e7b68334..d6dc502656b511ba8c29924024758afc3a64133c 100644
--- a/test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefinePreviousVersions.java
+++ b/test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefinePreviousVersions.java
@@ -24,7 +24,7 @@
/*
* @test
* @bug 8165246 8010319
- * @summary Test has_previous_versions flag and processing during class unloading.
+ * @summary Test clean_previous_versions flag and processing during class unloading.
* @requires vm.jvmti
* @requires vm.opt.final.ClassUnloading
* @requires vm.flagless
@@ -86,15 +86,15 @@ public class RedefinePreviousVersions {
"-Xlog:redefine+class+iklass+add=trace,redefine+class+iklass+purge=trace",
"RedefinePreviousVersions");
new OutputAnalyzer(pb.start())
- .shouldContain("Class unloading: has_previous_versions = false")
- .shouldContain("Class unloading: has_previous_versions = true")
+ .shouldContain("Class unloading: should_clean_previous_versions = false")
+ .shouldContain("Class unloading: should_clean_previous_versions = true")
.shouldHaveExitValue(0);
return;
}
// Redefine a class and create some garbage
// Since there are no methods running, the previous version is never added to the
- // previous_version_list and the flag _has_previous_versions should stay false
+ // previous_version_list and the flag _should_clean_previous_versions should stay false
RedefineClassHelper.redefineClass(RedefinePreviousVersions_B.class, newB);
for (int i = 0; i < 10 ; i++) {
@@ -114,7 +114,7 @@ public class RedefinePreviousVersions {
}
// Since a method of newRunning is running, this class should be added to the previous_version_list
- // of Running, and _has_previous_versions should return true at class unloading.
+ // of Running, and _should_clean_previous_versions should return true at class unloading.
RedefineClassHelper.redefineClass(RedefinePreviousVersions_Running.class, newRunning);
for (int i = 0; i < 10 ; i++) {
diff --git a/test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineSharedClassJFR.java b/test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineSharedClassJFR.java
new file mode 100644
index 0000000000000000000000000000000000000000..6ee54be7831af4ef326f7c00203cfd594d04284f
--- /dev/null
+++ b/test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineSharedClassJFR.java
@@ -0,0 +1,111 @@
+/*
+ * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8306929
+ * @summary Verify should_clean_previous_versions when run with JFR and CDS
+ * @requires vm.jvmti
+ * @requires vm.cds
+ * @requires vm.hasJFR
+ * @requires vm.opt.final.ClassUnloading
+ * @requires vm.flagless
+ * @library /test/lib
+ * @run driver RedefineSharedClassJFR xshare-off
+ * @run driver RedefineSharedClassJFR xshare-on
+ */
+import java.util.ArrayList;
+import java.util.List;
+
+import jdk.test.lib.Platform;
+import jdk.test.lib.process.ProcessTools;
+import jdk.test.lib.process.OutputAnalyzer;
+
+import jtreg.SkippedException;
+
+public class RedefineSharedClassJFR {
+
+ private static final String SHOULD_CLEAN_TRUE = "Class unloading: should_clean_previous_versions = true";
+ private static final String SHOULD_CLEAN_FALSE = "Class unloading: should_clean_previous_versions = false";
+ private static final String SCRATCH_CLASS_ADDED_SHARED = "scratch class added; class is shared";
+ private static final String SCRATCH_CLASS_ADDED_ON_STACK = "scratch class added; one of its methods is on_stack.";
+
+ public static void main(String[] args) throws Exception {
+ // Skip test if default archive is supported.
+ if (!Platform.isDefaultCDSArchiveSupported()) {
+ throw new SkippedException("Supported platform");
+ }
+
+ // Test is run with JFR which will transform a number of classes. Depending
+ // on if the test is run with or without CDS the output will be different,
+ // due to the fact that shared classes can never be cleaned out after retranform.
+ if (args.length > 0) {
+ // When run with an argument the class is used as driver and should parse
+ // the output to verify it is correct given the command line.
+ List<String> baseCommand = List.of(
+ "-XX:StartFlightRecording",
+ "-Xlog:redefine+class+iklass+add=trace,redefine+class+iklass+purge=trace",
+ "RedefineSharedClassJFR");
+
+ if (args[0].equals("xshare-off")) {
+ // First case is with -Xshare:off. In this case no classes are shared
+ // and we should be able to clean out the retransformed classes. There
+ // is no guarantee that any classes will be in use, so just verify that
+ // no classes are added due to being shared.
+ List<String> offCommand = new ArrayList<>();
+ offCommand.add("-Xshare:off");
+ offCommand.addAll(baseCommand);
+ ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(offCommand);
+ new OutputAnalyzer(pb.start())
+ // We can't expect any of the transformed classes to be in use
+ // so the only thing we can verify is that no scratch classes
+ // are added because they are shared.
+ .shouldNotContain(SCRATCH_CLASS_ADDED_SHARED)
+ .shouldHaveExitValue(0);
+ return;
+ } else if (args[0].equals("xshare-on")) {
+ // With -Xshare:on, the shared classes can never be cleaned out. Check the
+ // logs to verify we don't try to clean when we know it is not needed.
+ List<String> onCommand = new ArrayList<>();
+ onCommand.add("-Xshare:on");
+ onCommand.addAll(baseCommand);
+ ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(onCommand);
+ new OutputAnalyzer(pb.start())
+ .shouldContain(SHOULD_CLEAN_FALSE)
+ .shouldNotContain(SHOULD_CLEAN_TRUE)
+ .shouldContain(SCRATCH_CLASS_ADDED_SHARED)
+ // If the below line occurs, then should_clean_previous_versions will be
+ // true and the above shouldContain will trigger. This check is to
+ // show the intention that we don't expect any non-shared transformed
+ // classes to be in use.
+ .shouldNotContain(SCRATCH_CLASS_ADDED_ON_STACK)
+ .shouldHaveExitValue(0);
+ return;
+ }
+ }
+
+ // When run without any argument this class acts as test and we do a system GC
+ // to trigger cleaning and get the output we want to check.
+ System.gc();
+ }
+}