From 003be0dee2f6c190697ec0a923546362c50cc0e5 Mon Sep 17 00:00:00 2001 From: Calvin Cheung Date: Thu, 3 Jul 2025 19:40:22 +0000 Subject: [PATCH] 8361325: Refactor ClassLoaderExt Reviewed-by: coleenp, sspitsyn --- src/hotspot/share/cds/archiveBuilder.cpp | 1 - src/hotspot/share/cds/cdsProtectionDomain.cpp | 1 - src/hotspot/share/cds/classListParser.cpp | 2 +- src/hotspot/share/cds/filemap.cpp | 1 - src/hotspot/share/cds/heapShared.cpp | 1 - src/hotspot/share/cds/metaspaceShared.cpp | 1 - src/hotspot/share/classfile/classLoader.cpp | 55 ++++++++- src/hotspot/share/classfile/classLoader.hpp | 2 + .../share/classfile/classLoaderExt.cpp | 108 ------------------ .../share/classfile/classLoaderExt.hpp | 47 -------- src/hotspot/share/classfile/modules.cpp | 8 +- .../share/classfile/systemDictionary.cpp | 1 - .../classfile/systemDictionaryShared.cpp | 1 - src/hotspot/share/prims/jvmtiEnv.cpp | 3 +- 14 files changed, 63 insertions(+), 169 deletions(-) delete mode 100644 src/hotspot/share/classfile/classLoaderExt.cpp delete mode 100644 src/hotspot/share/classfile/classLoaderExt.hpp diff --git a/src/hotspot/share/cds/archiveBuilder.cpp b/src/hotspot/share/cds/archiveBuilder.cpp index fca9d970cd4..7f8c7787152 100644 --- a/src/hotspot/share/cds/archiveBuilder.cpp +++ b/src/hotspot/share/cds/archiveBuilder.cpp @@ -38,7 +38,6 @@ #include "cds/regeneratedClasses.hpp" #include "classfile/classLoader.hpp" #include "classfile/classLoaderDataShared.hpp" -#include "classfile/classLoaderExt.hpp" #include "classfile/javaClasses.hpp" #include "classfile/symbolTable.hpp" #include "classfile/systemDictionaryShared.hpp" diff --git a/src/hotspot/share/cds/cdsProtectionDomain.cpp b/src/hotspot/share/cds/cdsProtectionDomain.cpp index e743df67796..d509e8afc12 100644 --- a/src/hotspot/share/cds/cdsProtectionDomain.cpp +++ b/src/hotspot/share/cds/cdsProtectionDomain.cpp @@ -27,7 +27,6 @@ #include "cds/cdsProtectionDomain.hpp" #include "classfile/classLoader.hpp" #include "classfile/classLoaderData.inline.hpp" -#include "classfile/classLoaderExt.hpp" #include "classfile/javaClasses.hpp" #include "classfile/moduleEntry.hpp" #include "classfile/systemDictionaryShared.hpp" diff --git a/src/hotspot/share/cds/classListParser.cpp b/src/hotspot/share/cds/classListParser.cpp index 9cf703c627f..9ee7afcbc3e 100644 --- a/src/hotspot/share/cds/classListParser.cpp +++ b/src/hotspot/share/cds/classListParser.cpp @@ -30,7 +30,7 @@ #include "cds/lambdaProxyClassDictionary.hpp" #include "cds/metaspaceShared.hpp" #include "cds/unregisteredClasses.hpp" -#include "classfile/classLoaderExt.hpp" +#include "classfile/classLoader.hpp" #include "classfile/javaClasses.inline.hpp" #include "classfile/symbolTable.hpp" #include "classfile/systemDictionary.hpp" diff --git a/src/hotspot/share/cds/filemap.cpp b/src/hotspot/share/cds/filemap.cpp index 35b46099e1b..779f7b867c2 100644 --- a/src/hotspot/share/cds/filemap.cpp +++ b/src/hotspot/share/cds/filemap.cpp @@ -39,7 +39,6 @@ #include "classfile/classLoader.hpp" #include "classfile/classLoader.inline.hpp" #include "classfile/classLoaderData.inline.hpp" -#include "classfile/classLoaderExt.hpp" #include "classfile/symbolTable.hpp" #include "classfile/systemDictionaryShared.hpp" #include "classfile/vmClasses.hpp" diff --git a/src/hotspot/share/cds/heapShared.cpp b/src/hotspot/share/cds/heapShared.cpp index df045b40583..3f624687e39 100644 --- a/src/hotspot/share/cds/heapShared.cpp +++ b/src/hotspot/share/cds/heapShared.cpp @@ -38,7 +38,6 @@ #include "cds/metaspaceShared.hpp" #include "cds/regeneratedClasses.hpp" #include "classfile/classLoaderData.hpp" -#include "classfile/classLoaderExt.hpp" #include "classfile/javaClasses.inline.hpp" #include "classfile/modules.hpp" #include "classfile/stringTable.hpp" diff --git a/src/hotspot/share/cds/metaspaceShared.cpp b/src/hotspot/share/cds/metaspaceShared.cpp index 6e0385fc99c..2f817ec3e71 100644 --- a/src/hotspot/share/cds/metaspaceShared.cpp +++ b/src/hotspot/share/cds/metaspaceShared.cpp @@ -49,7 +49,6 @@ #include "cds/metaspaceShared.hpp" #include "classfile/classLoaderDataGraph.hpp" #include "classfile/classLoaderDataShared.hpp" -#include "classfile/classLoaderExt.hpp" #include "classfile/javaClasses.inline.hpp" #include "classfile/loaderConstraints.hpp" #include "classfile/modules.hpp" diff --git a/src/hotspot/share/classfile/classLoader.cpp b/src/hotspot/share/classfile/classLoader.cpp index e6c10859371..359970a3bcd 100644 --- a/src/hotspot/share/classfile/classLoader.cpp +++ b/src/hotspot/share/classfile/classLoader.cpp @@ -25,11 +25,11 @@ #include "cds/aotClassLocation.hpp" #include "cds/cds_globals.hpp" #include "cds/cdsConfig.hpp" +#include "cds/dynamicArchive.hpp" #include "cds/heapShared.hpp" #include "classfile/classFileStream.hpp" #include "classfile/classLoader.inline.hpp" #include "classfile/classLoaderData.inline.hpp" -#include "classfile/classLoaderExt.hpp" #include "classfile/classLoadInfo.hpp" #include "classfile/javaClasses.hpp" #include "classfile/klassFactory.hpp" @@ -1283,7 +1283,47 @@ void ClassLoader::record_result(JavaThread* current, InstanceKlass* ik, const char* const file_name = file_name_for_class_name(class_name, ik->name()->utf8_length()); assert(file_name != nullptr, "invariant"); - ClassLoaderExt::record_result_for_builtin_loader(checked_cast(classpath_index), ik, redefined); + record_result_for_builtin_loader(checked_cast(classpath_index), ik, redefined); +} + +void ClassLoader::record_result_for_builtin_loader(s2 classpath_index, InstanceKlass* result, bool redefined) { + assert(CDSConfig::is_dumping_archive(), "sanity"); + + oop loader = result->class_loader(); + if (SystemDictionary::is_system_class_loader(loader)) { + AOTClassLocationConfig::dumptime_set_has_app_classes(); + } else if (SystemDictionary::is_platform_class_loader(loader)) { + AOTClassLocationConfig::dumptime_set_has_platform_classes(); + } else { + precond(loader == nullptr); + } + + if (CDSConfig::is_dumping_preimage_static_archive() || CDSConfig::is_dumping_dynamic_archive()) { + if (!AOTClassLocationConfig::dumptime()->is_valid_classpath_index(classpath_index, result)) { + classpath_index = -1; + } + } + + AOTClassLocationConfig::dumptime_update_max_used_index(classpath_index); + result->set_shared_classpath_index(classpath_index); + +#if INCLUDE_CDS_JAVA_HEAP + if (CDSConfig::is_dumping_heap() && AllowArchivingWithJavaAgent && result->defined_by_boot_loader() && + classpath_index < 0 && redefined) { + // When dumping the heap (which happens only during static dump), classes for the built-in + // loaders are always loaded from known locations (jimage, classpath or modulepath), + // so classpath_index should always be >= 0. + // The only exception is when a java agent is used during dump time (for testing + // purposes only). If a class is transformed by the agent, the AOTClassLocation of + // this class may point to an unknown location. This may break heap object archiving, + // which requires all the boot classes to be from known locations. This is an + // uncommon scenario (even in test cases). Let's simply disable heap object archiving. + ResourceMark rm; + log_warning(aot)("heap objects cannot be written because class %s maybe modified by ClassFileLoadHook.", + result->external_name()); + CDSConfig::disable_heap_dumping(); + } +#endif // INCLUDE_CDS_JAVA_HEAP } void ClassLoader::record_hidden_class(InstanceKlass* ik) { @@ -1308,6 +1348,17 @@ void ClassLoader::record_hidden_class(InstanceKlass* ik) { } } } + +void ClassLoader::append_boot_classpath(ClassPathEntry* new_entry) { + if (CDSConfig::is_using_archive()) { + warning("Sharing is only supported for boot loader classes because bootstrap classpath has been appended"); + FileMapInfo::current_info()->set_has_platform_or_app_classes(false); + if (DynamicArchive::is_mapped()) { + FileMapInfo::dynamic_info()->set_has_platform_or_app_classes(false); + } + } + add_to_boot_append_entries(new_entry); +} #endif // INCLUDE_CDS // Initialize the class loader's access to methods in libzip. Parse and diff --git a/src/hotspot/share/classfile/classLoader.hpp b/src/hotspot/share/classfile/classLoader.hpp index d762e6caca7..a946f1f4e25 100644 --- a/src/hotspot/share/classfile/classLoader.hpp +++ b/src/hotspot/share/classfile/classLoader.hpp @@ -345,7 +345,9 @@ class ClassLoader: AllStatic { static char* uri_to_path(const char* uri); static void record_result(JavaThread* current, InstanceKlass* ik, const ClassFileStream* stream, bool redefined); + static void record_result_for_builtin_loader(s2 classpath_index, InstanceKlass* result, bool redefined); static void record_hidden_class(InstanceKlass* ik); + static void append_boot_classpath(ClassPathEntry* new_entry); #endif static char* lookup_vm_options(); diff --git a/src/hotspot/share/classfile/classLoaderExt.cpp b/src/hotspot/share/classfile/classLoaderExt.cpp deleted file mode 100644 index cd57f4e5e25..00000000000 --- a/src/hotspot/share/classfile/classLoaderExt.cpp +++ /dev/null @@ -1,108 +0,0 @@ -/* - * Copyright (c) 2015, 2025, 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. - * - */ - -#include "cds/aotClassLocation.hpp" -#include "cds/cds_globals.hpp" -#include "cds/cdsConfig.hpp" -#include "cds/dynamicArchive.hpp" -#include "cds/filemap.hpp" -#include "cds/heapShared.hpp" -#include "classfile/classFileParser.hpp" -#include "classfile/classLoader.inline.hpp" -#include "classfile/classLoaderExt.hpp" -#include "classfile/classLoaderData.inline.hpp" -#include "classfile/classLoadInfo.hpp" -#include "classfile/klassFactory.hpp" -#include "classfile/modules.hpp" -#include "classfile/systemDictionary.hpp" -#include "classfile/vmSymbols.hpp" -#include "gc/shared/collectedHeap.hpp" -#include "logging/log.hpp" -#include "memory/allocation.inline.hpp" -#include "memory/resourceArea.hpp" -#include "oops/instanceKlass.hpp" -#include "oops/klass.inline.hpp" -#include "oops/oop.inline.hpp" -#include "oops/symbol.hpp" -#include "runtime/arguments.hpp" -#include "runtime/handles.inline.hpp" -#include "runtime/java.hpp" -#include "runtime/os.hpp" -#include "utilities/checkedCast.hpp" -#include "utilities/stringUtils.hpp" - -void ClassLoaderExt::append_boot_classpath(ClassPathEntry* new_entry) { - if (CDSConfig::is_using_archive()) { - warning("Sharing is only supported for boot loader classes because bootstrap classpath has been appended"); - FileMapInfo::current_info()->set_has_platform_or_app_classes(false); - if (DynamicArchive::is_mapped()) { - FileMapInfo::dynamic_info()->set_has_platform_or_app_classes(false); - } - } - ClassLoader::add_to_boot_append_entries(new_entry); -} - -int ClassLoaderExt::compare_module_names(const char** p1, const char** p2) { - return strcmp(*p1, *p2); -} - -void ClassLoaderExt::record_result_for_builtin_loader(s2 classpath_index, InstanceKlass* result, bool redefined) { - assert(CDSConfig::is_dumping_archive(), "sanity"); - - oop loader = result->class_loader(); - if (SystemDictionary::is_system_class_loader(loader)) { - AOTClassLocationConfig::dumptime_set_has_app_classes(); - } else if (SystemDictionary::is_platform_class_loader(loader)) { - AOTClassLocationConfig::dumptime_set_has_platform_classes(); - } else { - precond(loader == nullptr); - } - - if (CDSConfig::is_dumping_preimage_static_archive() || CDSConfig::is_dumping_dynamic_archive()) { - if (!AOTClassLocationConfig::dumptime()->is_valid_classpath_index(classpath_index, result)) { - classpath_index = -1; - } - } - - AOTClassLocationConfig::dumptime_update_max_used_index(classpath_index); - result->set_shared_classpath_index(classpath_index); - -#if INCLUDE_CDS_JAVA_HEAP - if (CDSConfig::is_dumping_heap() && AllowArchivingWithJavaAgent && result->defined_by_boot_loader() && - classpath_index < 0 && redefined) { - // When dumping the heap (which happens only during static dump), classes for the built-in - // loaders are always loaded from known locations (jimage, classpath or modulepath), - // so classpath_index should always be >= 0. - // The only exception is when a java agent is used during dump time (for testing - // purposes only). If a class is transformed by the agent, the AOTClassLocation of - // this class may point to an unknown location. This may break heap object archiving, - // which requires all the boot classes to be from known locations. This is an - // uncommon scenario (even in test cases). Let's simply disable heap object archiving. - ResourceMark rm; - log_warning(aot)("heap objects cannot be written because class %s maybe modified by ClassFileLoadHook.", - result->external_name()); - CDSConfig::disable_heap_dumping(); - } -#endif // INCLUDE_CDS_JAVA_HEAP -} diff --git a/src/hotspot/share/classfile/classLoaderExt.hpp b/src/hotspot/share/classfile/classLoaderExt.hpp deleted file mode 100644 index ee6b59bde14..00000000000 --- a/src/hotspot/share/classfile/classLoaderExt.hpp +++ /dev/null @@ -1,47 +0,0 @@ -/* - * Copyright (c) 2014, 2025, 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. - * - */ - -#ifndef SHARE_CLASSFILE_CLASSLOADEREXT_HPP -#define SHARE_CLASSFILE_CLASSLOADEREXT_HPP - -#include "classfile/classLoader.hpp" -#include "classfile/moduleEntry.hpp" -#include "utilities/macros.hpp" - -class ClassListParser; - -class ClassLoaderExt: public ClassLoader { // AllStatic -public: -#if INCLUDE_CDS -public: - // Called by JVMTI code to add boot classpath - - static void append_boot_classpath(ClassPathEntry* new_entry); - - static int compare_module_names(const char** p1, const char** p2); - static void record_result_for_builtin_loader(s2 classpath_index, InstanceKlass* result, bool redefined); -#endif // INCLUDE_CDS -}; - -#endif // SHARE_CLASSFILE_CLASSLOADEREXT_HPP diff --git a/src/hotspot/share/classfile/modules.cpp b/src/hotspot/share/classfile/modules.cpp index a2206c842bd..72e56b03a58 100644 --- a/src/hotspot/share/classfile/modules.cpp +++ b/src/hotspot/share/classfile/modules.cpp @@ -30,7 +30,6 @@ #include "classfile/classLoader.hpp" #include "classfile/classLoaderData.inline.hpp" #include "classfile/classLoaderDataShared.hpp" -#include "classfile/classLoaderExt.hpp" #include "classfile/javaAssertions.hpp" #include "classfile/javaClasses.hpp" #include "classfile/javaClasses.inline.hpp" @@ -625,6 +624,11 @@ void Modules::ArchivedProperty::runtime_check() const { } } + +static int compare_module_names(const char** p1, const char** p2) { + return strcmp(*p1, *p2); +} + // Caller needs ResourceMark const char* Modules::ArchivedProperty::get_numbered_property_as_sorted_string() const { assert(_numbered, "sanity"); @@ -665,7 +669,7 @@ const char* Modules::ArchivedProperty::get_numbered_property_as_sorted_string() // list[2] = "java.base" // list[3] = "" // list[4] = "" - list.sort(ClassLoaderExt::compare_module_names); + list.sort(compare_module_names); const char* prefix = ""; stringStream st; diff --git a/src/hotspot/share/classfile/systemDictionary.cpp b/src/hotspot/share/classfile/systemDictionary.cpp index b1abb2ab0fe..c3623dc25a1 100644 --- a/src/hotspot/share/classfile/systemDictionary.cpp +++ b/src/hotspot/share/classfile/systemDictionary.cpp @@ -30,7 +30,6 @@ #include "classfile/classLoader.hpp" #include "classfile/classLoaderData.inline.hpp" #include "classfile/classLoaderDataGraph.inline.hpp" -#include "classfile/classLoaderExt.hpp" #include "classfile/classLoadInfo.hpp" #include "classfile/dictionary.hpp" #include "classfile/javaClasses.inline.hpp" diff --git a/src/hotspot/share/classfile/systemDictionaryShared.cpp b/src/hotspot/share/classfile/systemDictionaryShared.cpp index 31ff7777bf9..f863347c152 100644 --- a/src/hotspot/share/classfile/systemDictionaryShared.cpp +++ b/src/hotspot/share/classfile/systemDictionaryShared.cpp @@ -44,7 +44,6 @@ #include "classfile/classLoader.hpp" #include "classfile/classLoaderData.inline.hpp" #include "classfile/classLoaderDataGraph.hpp" -#include "classfile/classLoaderExt.hpp" #include "classfile/dictionary.hpp" #include "classfile/javaClasses.hpp" #include "classfile/javaClasses.inline.hpp" diff --git a/src/hotspot/share/prims/jvmtiEnv.cpp b/src/hotspot/share/prims/jvmtiEnv.cpp index ad83f8c6d1b..473db4f2fbc 100644 --- a/src/hotspot/share/prims/jvmtiEnv.cpp +++ b/src/hotspot/share/prims/jvmtiEnv.cpp @@ -22,7 +22,6 @@ * */ -#include "classfile/classLoaderExt.hpp" #include "classfile/javaClasses.inline.hpp" #include "classfile/stringTable.hpp" #include "classfile/modules.hpp" @@ -675,7 +674,7 @@ JvmtiEnv::AddToBootstrapClassLoaderSearch(const char* segment) { // add the jar file to the bootclasspath log_info(class, load)("opened: %s", zip_entry->name()); #if INCLUDE_CDS - ClassLoaderExt::append_boot_classpath(zip_entry); + ClassLoader::append_boot_classpath(zip_entry); #else ClassLoader::add_to_boot_append_entries(zip_entry); #endif