From 63f5656642a5a8941e91b17a9465abd73a521ee1 Mon Sep 17 00:00:00 2001 From: Calvin Cheung Date: Tue, 11 Feb 2025 00:13:54 +0000 Subject: [PATCH 1/6] 8348322: AOT cache creation crashes with "All cached hidden classes must be aot-linkable" when AOTInvokeDynamicLinking is disabled --- src/hotspot/share/cds/aotArtifactFinder.cpp | 6 ++++-- src/hotspot/share/cds/aotClassLinker.cpp | 2 +- src/hotspot/share/cds/archiveBuilder.cpp | 9 +++++++-- src/hotspot/share/cds/cdsHeapVerifier.cpp | 2 ++ src/hotspot/share/cds/metaspaceShared.cpp | 3 +++ .../aotClassLinking/AOTClassLinkingVMOptions.java | 12 +++++++++++- 6 files changed, 28 insertions(+), 6 deletions(-) diff --git a/src/hotspot/share/cds/aotArtifactFinder.cpp b/src/hotspot/share/cds/aotArtifactFinder.cpp index 48f31635c63b9..445327c9d2451 100644 --- a/src/hotspot/share/cds/aotArtifactFinder.cpp +++ b/src/hotspot/share/cds/aotArtifactFinder.cpp @@ -210,8 +210,10 @@ void AOTArtifactFinder::add_cached_instance_class(InstanceKlass* ik) { scan_oops_in_instance_class(ik); if (ik->is_hidden() && CDSConfig::is_initing_classes_at_dump_time()) { bool succeed = AOTClassLinker::try_add_candidate(ik); - guarantee(succeed, "All cached hidden classes must be aot-linkable"); - add_aot_inited_class(ik); + if (CDSConfig::is_dumping_invokedynamic()) { + guarantee(succeed, "All cached hidden classes must be aot-linkable"); + add_aot_inited_class(ik); + } } } } diff --git a/src/hotspot/share/cds/aotClassLinker.cpp b/src/hotspot/share/cds/aotClassLinker.cpp index a1cacd735dd68..b53bdff119310 100644 --- a/src/hotspot/share/cds/aotClassLinker.cpp +++ b/src/hotspot/share/cds/aotClassLinker.cpp @@ -143,7 +143,7 @@ bool AOTClassLinker::try_add_candidate(InstanceKlass* ik) { if (ik->is_hidden()) { assert(ik->shared_class_loader_type() != ClassLoader::OTHER, "must have been set"); if (!CDSConfig::is_dumping_invokedynamic()) { - return false; + return true; } if (HeapShared::is_lambda_proxy_klass(ik)) { InstanceKlass* nest_host = ik->nest_host_not_null(); diff --git a/src/hotspot/share/cds/archiveBuilder.cpp b/src/hotspot/share/cds/archiveBuilder.cpp index afd2d909595f5..ea1d2a3a79bbb 100644 --- a/src/hotspot/share/cds/archiveBuilder.cpp +++ b/src/hotspot/share/cds/archiveBuilder.cpp @@ -787,6 +787,7 @@ void ArchiveBuilder::make_klasses_shareable() { const char* aotlinked_msg = ""; const char* inited_msg = ""; Klass* k = get_buffered_addr(klasses()->at(i)); + bool inited = false; k->remove_java_mirror(); #ifdef _LP64 if (UseCompactObjectHeaders) { @@ -811,7 +812,7 @@ void ArchiveBuilder::make_klasses_shareable() { InstanceKlass* ik = InstanceKlass::cast(k); InstanceKlass* src_ik = get_source_addr(ik); bool aotlinked = AOTClassLinker::is_candidate(src_ik); - bool inited = ik->has_aot_initialized_mirror(); + inited = ik->has_aot_initialized_mirror(); ADD_COUNT(num_instance_klasses); if (CDSConfig::is_dumping_dynamic_archive()) { // For static dump, class loader type are already set. @@ -892,7 +893,11 @@ void ArchiveBuilder::make_klasses_shareable() { aotlinked_msg = " aot-linked"; } if (inited) { - inited_msg = " inited"; + if (InstanceKlass::cast(k)->static_field_size() == 0) { + inited_msg = " inited (no static fields)"; + } else { + inited_msg = " inited"; + } } MetaspaceShared::rewrite_nofast_bytecodes_and_calculate_fingerprints(Thread::current(), ik); diff --git a/src/hotspot/share/cds/cdsHeapVerifier.cpp b/src/hotspot/share/cds/cdsHeapVerifier.cpp index f9e613a74cee8..2204f9b88bca0 100644 --- a/src/hotspot/share/cds/cdsHeapVerifier.cpp +++ b/src/hotspot/share/cds/cdsHeapVerifier.cpp @@ -106,6 +106,8 @@ CDSHeapVerifier::CDSHeapVerifier() : _archived_objs(0), _problems(0) ADD_EXCL("java/lang/System", "bootLayer"); // A + ADD_EXCL("java/util/Collections", "EMPTY_LIST"); // E + // A dummy object used by HashSet. The value doesn't matter and it's never // tested for equality. ADD_EXCL("java/util/HashSet", "PRESENT"); // E diff --git a/src/hotspot/share/cds/metaspaceShared.cpp b/src/hotspot/share/cds/metaspaceShared.cpp index d1b33da19d89d..bd467226828d2 100644 --- a/src/hotspot/share/cds/metaspaceShared.cpp +++ b/src/hotspot/share/cds/metaspaceShared.cpp @@ -959,12 +959,15 @@ void MetaspaceShared::preload_and_dump_impl(StaticArchiveBuilder& builder, TRAPS vmSymbols::createArchivedObjects(), vmSymbols::void_method_signature(), CHECK); + } + if (CDSConfig::is_initing_classes_at_dump_time()) { // java.lang.Class::reflectionFactory cannot be archived yet. We set this field // to null, and it will be initialized again at runtime. log_debug(cds)("Resetting Class::reflectionFactory"); TempNewSymbol method_name = SymbolTable::new_symbol("resetArchivedStates"); Symbol* method_sig = vmSymbols::void_method_signature(); + JavaValue result(T_VOID); JavaCalls::call_static(&result, vmClasses::Class_klass(), method_name, method_sig, CHECK); diff --git a/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/AOTClassLinkingVMOptions.java b/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/AOTClassLinkingVMOptions.java index 0d75d16d2b04e..4fa2e12485bb9 100644 --- a/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/AOTClassLinkingVMOptions.java +++ b/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/AOTClassLinkingVMOptions.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2024, 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 @@ -55,6 +55,7 @@ static void testCase(String s) { } public static void main(String[] args) throws Exception { + TestCommon.testDump(appJar, TestCommon.list("Hello"), "-XX:+AOTClassLinking"); @@ -80,6 +81,15 @@ public static void main(String[] args) throws Exception { .assertAbnormalExit("CDS archive has aot-linked classes." + " It cannot be used with -Djava.security.manager=default."); + // Dumping with AOTInvokeDynamicLinking disabled + TestCommon.testDump(appJar, TestCommon.list("Hello"), + "-XX:+AOTClassLinking", "-XX:-AOTInvokeDynamicLinking"); + + testCase("Archived full module graph must be enabled at runtime (with -XX:-AOTInvokeDynamicLinking)"); + TestCommon.run("-cp", appJar, "-Djdk.module.validation=1", "Hello") + .assertAbnormalExit("CDS archive has aot-linked classes." + + " It cannot be used when archived full module graph is not used"); + // NOTE: tests for ClassFileLoadHook + AOTClassLinking is in // ../jvmti/ClassFileLoadHookTest.java From ee7429fc96cccf732761fa8cbf5893d9d5d79a03 Mon Sep 17 00:00:00 2001 From: Calvin Cheung Date: Sat, 15 Feb 2025 00:11:54 +0000 Subject: [PATCH 2/6] Ioi's suggestions --- src/hotspot/share/cds/aotArtifactFinder.cpp | 2 +- src/hotspot/share/cds/aotClassInitializer.cpp | 2 +- src/hotspot/share/cds/archiveBuilder.cpp | 2 +- src/hotspot/share/cds/cdsConfig.cpp | 10 +++++++--- src/hotspot/share/cds/cdsConfig.hpp | 4 +--- src/hotspot/share/cds/cdsHeapVerifier.cpp | 2 +- src/hotspot/share/cds/classListParser.cpp | 2 +- src/hotspot/share/cds/filemap.cpp | 6 ------ src/hotspot/share/cds/filemap.hpp | 1 - src/hotspot/share/cds/heapShared.cpp | 8 ++++---- src/hotspot/share/cds/lambdaFormInvokers.cpp | 2 +- src/hotspot/share/cds/metaspaceShared.cpp | 4 ++-- src/hotspot/share/classfile/javaClasses.cpp | 2 +- src/hotspot/share/classfile/systemDictionaryShared.cpp | 4 ++-- src/hotspot/share/oops/constantPool.cpp | 2 +- src/hotspot/share/oops/cpCache.cpp | 2 +- .../aotClassLinking/AOTClassLinkingVMOptions.java | 1 - 17 files changed, 25 insertions(+), 31 deletions(-) diff --git a/src/hotspot/share/cds/aotArtifactFinder.cpp b/src/hotspot/share/cds/aotArtifactFinder.cpp index 445327c9d2451..8186356c6697e 100644 --- a/src/hotspot/share/cds/aotArtifactFinder.cpp +++ b/src/hotspot/share/cds/aotArtifactFinder.cpp @@ -210,7 +210,7 @@ void AOTArtifactFinder::add_cached_instance_class(InstanceKlass* ik) { scan_oops_in_instance_class(ik); if (ik->is_hidden() && CDSConfig::is_initing_classes_at_dump_time()) { bool succeed = AOTClassLinker::try_add_candidate(ik); - if (CDSConfig::is_dumping_invokedynamic()) { + if (CDSConfig::is_dumping_method_handles()) { guarantee(succeed, "All cached hidden classes must be aot-linkable"); add_aot_inited_class(ik); } diff --git a/src/hotspot/share/cds/aotClassInitializer.cpp b/src/hotspot/share/cds/aotClassInitializer.cpp index 5b022cae24465..b8718ffd1ae71 100644 --- a/src/hotspot/share/cds/aotClassInitializer.cpp +++ b/src/hotspot/share/cds/aotClassInitializer.cpp @@ -266,7 +266,7 @@ bool AOTClassInitializer::can_archive_initialized_mirror(InstanceKlass* ik) { } } - if (CDSConfig::is_dumping_invokedynamic()) { + if (CDSConfig::is_dumping_method_handles()) { // This table was created with the help of CDSHeapVerifier. // Also, some $Holder classes are needed. E.g., Invokers. explicitly // initializes Invokers$Holder. Since Invokers. won't be executed diff --git a/src/hotspot/share/cds/archiveBuilder.cpp b/src/hotspot/share/cds/archiveBuilder.cpp index ea1d2a3a79bbb..f8267ea4abf84 100644 --- a/src/hotspot/share/cds/archiveBuilder.cpp +++ b/src/hotspot/share/cds/archiveBuilder.cpp @@ -835,7 +835,7 @@ void ArchiveBuilder::make_klasses_shareable() { type = "bad"; assert(0, "shouldn't happen"); } - if (CDSConfig::is_dumping_invokedynamic()) { + if (CDSConfig::is_dumping_method_handles()) { assert(HeapShared::is_archivable_hidden_klass(ik), "sanity"); } else { // Legacy CDS support for lambda proxies diff --git a/src/hotspot/share/cds/cdsConfig.cpp b/src/hotspot/share/cds/cdsConfig.cpp index 5fa6a94f70b4e..71963b9b060c3 100644 --- a/src/hotspot/share/cds/cdsConfig.cpp +++ b/src/hotspot/share/cds/cdsConfig.cpp @@ -44,7 +44,6 @@ bool CDSConfig::_is_using_optimized_module_handling = true; bool CDSConfig::_is_dumping_full_module_graph = true; bool CDSConfig::_is_using_full_module_graph = true; bool CDSConfig::_has_aot_linked_classes = false; -bool CDSConfig::_has_archived_invokedynamic = false; bool CDSConfig::_old_cds_flags_used = false; char* CDSConfig::_default_archive_path = nullptr; @@ -611,8 +610,13 @@ bool CDSConfig::is_dumping_invokedynamic() { return AOTInvokeDynamicLinking && is_dumping_aot_linked_classes() && is_dumping_heap(); } -bool CDSConfig::is_loading_invokedynamic() { - return UseSharedSpaces && is_using_full_module_graph() && _has_archived_invokedynamic; +// When we are dumping aot-linked classes and we are able to write archived heap objects, we automatically +// enable the archiving of MethodHandles. This will in turn enable the archiving of MethodTypes and hidden +// classes that are used in the implementation of MethodHandles. +// Archived MethodHandles are required for higher-level optimizations such as AOT resolution of invokedynamic +// and dynamic proxies. +bool CDSConfig::is_dumping_method_handles() { + return is_dumping_aot_linked_classes() && is_dumping_heap(); } #endif // INCLUDE_CDS_JAVA_HEAP diff --git a/src/hotspot/share/cds/cdsConfig.hpp b/src/hotspot/share/cds/cdsConfig.hpp index cceaceeb6d768..0ad5859b5e4bd 100644 --- a/src/hotspot/share/cds/cdsConfig.hpp +++ b/src/hotspot/share/cds/cdsConfig.hpp @@ -39,7 +39,6 @@ class CDSConfig : public AllStatic { static bool _is_dumping_full_module_graph; static bool _is_using_full_module_graph; static bool _has_aot_linked_classes; - static bool _has_archived_invokedynamic; static char* _default_archive_path; static char* _static_archive_path; @@ -121,8 +120,7 @@ class CDSConfig : public AllStatic { static bool is_initing_classes_at_dump_time() NOT_CDS_JAVA_HEAP_RETURN_(false); static bool is_dumping_invokedynamic() NOT_CDS_JAVA_HEAP_RETURN_(false); - static bool is_loading_invokedynamic() NOT_CDS_JAVA_HEAP_RETURN_(false); - static void set_has_archived_invokedynamic() { CDS_JAVA_HEAP_ONLY(_has_archived_invokedynamic = true); } + static bool is_dumping_method_handles() NOT_CDS_JAVA_HEAP_RETURN_(false); // full_module_graph (requires optimized_module_handling) static bool is_dumping_full_module_graph() { return CDS_ONLY(_is_dumping_full_module_graph) NOT_CDS(false); } diff --git a/src/hotspot/share/cds/cdsHeapVerifier.cpp b/src/hotspot/share/cds/cdsHeapVerifier.cpp index 2204f9b88bca0..729637f47c246 100644 --- a/src/hotspot/share/cds/cdsHeapVerifier.cpp +++ b/src/hotspot/share/cds/cdsHeapVerifier.cpp @@ -129,7 +129,7 @@ CDSHeapVerifier::CDSHeapVerifier() : _archived_objs(0), _problems(0) ADD_EXCL("sun/invoke/util/ValueConversions", "ONE_INT", // E "ZERO_INT"); // E - if (CDSConfig::is_dumping_invokedynamic()) { + if (CDSConfig::is_dumping_method_handles()) { ADD_EXCL("java/lang/invoke/InvokerBytecodeGenerator", "MEMBERNAME_FACTORY", // D "CD_Object_array", // E same as <...>ConstantUtils.CD_Object_array::CD_Object "INVOKER_SUPER_DESC"); // E same as java.lang.constant.ConstantDescs::CD_Object diff --git a/src/hotspot/share/cds/classListParser.cpp b/src/hotspot/share/cds/classListParser.cpp index 1b4b6ffb22c7a..8cdf317ecb3a6 100644 --- a/src/hotspot/share/cds/classListParser.cpp +++ b/src/hotspot/share/cds/classListParser.cpp @@ -628,7 +628,7 @@ void ClassListParser::resolve_indy(JavaThread* current, Symbol* class_name_symbo } void ClassListParser::resolve_indy_impl(Symbol* class_name_symbol, TRAPS) { - if (CDSConfig::is_dumping_invokedynamic()) { + if (CDSConfig::is_dumping_method_handles()) { // The CP entry for the invokedynamic instruction will be resolved. // No need to do the following. return; diff --git a/src/hotspot/share/cds/filemap.cpp b/src/hotspot/share/cds/filemap.cpp index 15e2dfb2d92da..f27c17db3ac8c 100644 --- a/src/hotspot/share/cds/filemap.cpp +++ b/src/hotspot/share/cds/filemap.cpp @@ -227,7 +227,6 @@ void FileMapHeader::populate(FileMapInfo *info, size_t core_region_alignment, _use_optimized_module_handling = CDSConfig::is_using_optimized_module_handling(); _has_aot_linked_classes = CDSConfig::is_dumping_aot_linked_classes(); _has_full_module_graph = CDSConfig::is_dumping_full_module_graph(); - _has_archived_invokedynamic = CDSConfig::is_dumping_invokedynamic(); // The following fields are for sanity checks for whether this archive // will function correctly with this JVM and the bootclasspath it's @@ -318,7 +317,6 @@ void FileMapHeader::print(outputStream* st) { st->print_cr("- use_optimized_module_handling: %d", _use_optimized_module_handling); st->print_cr("- has_full_module_graph %d", _has_full_module_graph); st->print_cr("- has_aot_linked_classes %d", _has_aot_linked_classes); - st->print_cr("- has_archived_invokedynamic %d", _has_archived_invokedynamic); } void SharedClassPathEntry::init_as_non_existent(const char* path, TRAPS) { @@ -2659,10 +2657,6 @@ bool FileMapHeader::validate() { if (!_has_full_module_graph) { CDSConfig::stop_using_full_module_graph("archive was created without full module graph"); } - - if (_has_archived_invokedynamic) { - CDSConfig::set_has_archived_invokedynamic(); - } } return true; diff --git a/src/hotspot/share/cds/filemap.hpp b/src/hotspot/share/cds/filemap.hpp index ae11c6c81cc17..68843e1098274 100644 --- a/src/hotspot/share/cds/filemap.hpp +++ b/src/hotspot/share/cds/filemap.hpp @@ -235,7 +235,6 @@ class FileMapHeader: private CDSFileMapHeaderBase { // some expensive operations. bool _has_aot_linked_classes; // Was the CDS archive created with -XX:+AOTClassLinking bool _has_full_module_graph; // Does this CDS archive contain the full archived module graph? - bool _has_archived_invokedynamic; // Does the archive have aot-linked invokedynamic CP entries? HeapRootSegments _heap_root_segments; // Heap root segments info size_t _heap_oopmap_start_pos; // The first bit in the oopmap corresponds to this position in the heap. size_t _heap_ptrmap_start_pos; // The first bit in the ptrmap corresponds to this position in the heap. diff --git a/src/hotspot/share/cds/heapShared.cpp b/src/hotspot/share/cds/heapShared.cpp index 96adf8d240dd9..2b9e2af360838 100644 --- a/src/hotspot/share/cds/heapShared.cpp +++ b/src/hotspot/share/cds/heapShared.cpp @@ -492,7 +492,7 @@ bool HeapShared::is_string_concat_klass(InstanceKlass* ik) { } bool HeapShared::is_archivable_hidden_klass(InstanceKlass* ik) { - return CDSConfig::is_dumping_invokedynamic() && + return CDSConfig::is_dumping_method_handles() && (is_lambda_form_klass(ik) || is_lambda_proxy_klass(ik) || is_string_concat_klass(ik)); } @@ -782,7 +782,7 @@ void KlassSubGraphInfo::add_subgraph_object_klass(Klass* orig_k) { if (orig_k->is_instance_klass()) { #ifdef ASSERT InstanceKlass* ik = InstanceKlass::cast(orig_k); - if (CDSConfig::is_dumping_invokedynamic()) { + if (CDSConfig::is_dumping_method_handles()) { assert(ik->class_loader() == nullptr || HeapShared::is_lambda_proxy_klass(ik), "we can archive only instances of boot classes or lambda proxy classes"); @@ -835,7 +835,7 @@ void KlassSubGraphInfo::check_allowed_klass(InstanceKlass* ik) { } const char* lambda_msg = ""; - if (CDSConfig::is_dumping_invokedynamic()) { + if (CDSConfig::is_dumping_method_handles()) { lambda_msg = ", or a lambda proxy class"; if (HeapShared::is_lambda_proxy_klass(ik) && (ik->class_loader() == nullptr || @@ -1108,7 +1108,7 @@ void HeapShared::resolve_classes_for_subgraph_of(JavaThread* current, Klass* k) } void HeapShared::initialize_java_lang_invoke(TRAPS) { - if (CDSConfig::is_loading_invokedynamic() || CDSConfig::is_dumping_invokedynamic()) { + if (CDSConfig::is_using_aot_linked_classes() || CDSConfig::is_dumping_method_handles()) { resolve_or_init("java/lang/invoke/Invokers$Holder", true, CHECK); resolve_or_init("java/lang/invoke/MethodHandle", true, CHECK); resolve_or_init("java/lang/invoke/MethodHandleNatives", true, CHECK); diff --git a/src/hotspot/share/cds/lambdaFormInvokers.cpp b/src/hotspot/share/cds/lambdaFormInvokers.cpp index ee3e9cff782ae..bbc4500ab6992 100644 --- a/src/hotspot/share/cds/lambdaFormInvokers.cpp +++ b/src/hotspot/share/cds/lambdaFormInvokers.cpp @@ -95,7 +95,7 @@ void LambdaFormInvokers::regenerate_holder_classes(TRAPS) { return; } - if (CDSConfig::is_dumping_static_archive() && CDSConfig::is_dumping_invokedynamic()) { + if (CDSConfig::is_dumping_static_archive() && CDSConfig::is_dumping_method_handles()) { // Work around JDK-8310831, as some methods in lambda form holder classes may not get generated. log_info(cds)("Archived MethodHandles may refer to lambda form holder classes. Cannot regenerate."); return; diff --git a/src/hotspot/share/cds/metaspaceShared.cpp b/src/hotspot/share/cds/metaspaceShared.cpp index bd467226828d2..3c1e2316a5c14 100644 --- a/src/hotspot/share/cds/metaspaceShared.cpp +++ b/src/hotspot/share/cds/metaspaceShared.cpp @@ -639,7 +639,7 @@ void VM_PopulateDumpSharedSpace::doit() { DEBUG_ONLY(SystemDictionaryShared::NoClassLoadingMark nclm); _pending_method_handle_intrinsics = new (mtClassShared) GrowableArray(256, mtClassShared); - if (CDSConfig::is_dumping_aot_linked_classes()) { + if (CDSConfig::is_dumping_method_handles()) { // When dumping AOT-linked classes, some classes may have direct references to a method handle // intrinsic. The easiest thing is to save all of them into the AOT cache. SystemDictionary::get_all_method_handle_intrinsics(_pending_method_handle_intrinsics); @@ -949,7 +949,7 @@ void MetaspaceShared::preload_and_dump_impl(StaticArchiveBuilder& builder, TRAPS HeapShared::reset_archived_object_states(CHECK); } - if (CDSConfig::is_dumping_invokedynamic()) { + if (CDSConfig::is_dumping_method_handles()) { // This assert means that the MethodType and MethodTypeForm tables won't be // updated concurrently when we are saving their contents into a side table. assert(CDSConfig::allow_only_single_java_thread(), "Required"); diff --git a/src/hotspot/share/classfile/javaClasses.cpp b/src/hotspot/share/classfile/javaClasses.cpp index a9e683900199f..d9e0767a4e51b 100644 --- a/src/hotspot/share/classfile/javaClasses.cpp +++ b/src/hotspot/share/classfile/javaClasses.cpp @@ -5423,7 +5423,7 @@ void JavaClasses::serialize_offsets(SerializeClosure* soc) { bool JavaClasses::is_supported_for_archiving(oop obj) { Klass* klass = obj->klass(); - if (!CDSConfig::is_dumping_invokedynamic()) { + if (!CDSConfig::is_dumping_method_handles()) { // These are supported by CDS only when CDSConfig::is_dumping_invokedynamic() is enabled. if (klass == vmClasses::ResolvedMethodName_klass() || klass == vmClasses::MemberName_klass()) { diff --git a/src/hotspot/share/classfile/systemDictionaryShared.cpp b/src/hotspot/share/classfile/systemDictionaryShared.cpp index b8e24bb2caace..24b6958f4bfd6 100644 --- a/src/hotspot/share/classfile/systemDictionaryShared.cpp +++ b/src/hotspot/share/classfile/systemDictionaryShared.cpp @@ -292,7 +292,7 @@ bool SystemDictionaryShared::check_for_exclusion_impl(InstanceKlass* k) { if (!k->is_hidden() && k->shared_classpath_index() < 0 && is_builtin(k)) { if (k->name()->starts_with("java/lang/invoke/BoundMethodHandle$Species_")) { // This class is dynamically generated by the JDK - if (CDSConfig::is_dumping_aot_linked_classes()) { + if (CDSConfig::is_dumping_method_handles()) { k->set_shared_classpath_index(0); } else { ResourceMark rm; @@ -565,7 +565,7 @@ void SystemDictionaryShared::validate_before_archiving(InstanceKlass* k) { guarantee(!info->is_excluded(), "Should not attempt to archive excluded class %s", name); if (is_builtin(k)) { if (k->is_hidden()) { - if (!CDSConfig::is_dumping_invokedynamic()) { + if (!CDSConfig::is_dumping_method_handles()) { assert(is_registered_lambda_proxy_class(k), "unexpected hidden class %s", name); } } diff --git a/src/hotspot/share/oops/constantPool.cpp b/src/hotspot/share/oops/constantPool.cpp index 8aba2bb2ba354..415ee7426da2e 100644 --- a/src/hotspot/share/oops/constantPool.cpp +++ b/src/hotspot/share/oops/constantPool.cpp @@ -288,7 +288,7 @@ void ConstantPool::klass_at_put(int class_index, Klass* k) { template void ConstantPool::iterate_archivable_resolved_references(Function function) { objArrayOop rr = resolved_references(); - if (rr != nullptr && cache() != nullptr && CDSConfig::is_dumping_invokedynamic()) { + if (rr != nullptr && cache() != nullptr && CDSConfig::is_dumping_method_handles()) { Array* indy_entries = cache()->resolved_indy_entries(); if (indy_entries != nullptr) { for (int i = 0; i < indy_entries->length(); i++) { diff --git a/src/hotspot/share/oops/cpCache.cpp b/src/hotspot/share/oops/cpCache.cpp index f4c53c0089e62..a354e0b07a696 100644 --- a/src/hotspot/share/oops/cpCache.cpp +++ b/src/hotspot/share/oops/cpCache.cpp @@ -572,7 +572,7 @@ bool ConstantPoolCache::can_archive_resolved_method(ConstantPool* src_cp, Resolv method_entry->is_resolved(Bytecodes::_invokespecial)) { return true; } else if (method_entry->is_resolved(Bytecodes::_invokehandle)) { - if (CDSConfig::is_dumping_invokedynamic()) { + if (CDSConfig::is_dumping_method_handles()) { // invokehandle depends on archived MethodType and LambdaForms. return true; } else { diff --git a/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/AOTClassLinkingVMOptions.java b/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/AOTClassLinkingVMOptions.java index 4fa2e12485bb9..af5c619d76793 100644 --- a/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/AOTClassLinkingVMOptions.java +++ b/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/AOTClassLinkingVMOptions.java @@ -55,7 +55,6 @@ static void testCase(String s) { } public static void main(String[] args) throws Exception { - TestCommon.testDump(appJar, TestCommon.list("Hello"), "-XX:+AOTClassLinking"); From b108cbc28be3bc22059ebf94fb9d516dc244e281 Mon Sep 17 00:00:00 2001 From: Calvin Cheung Date: Tue, 18 Feb 2025 23:25:28 +0000 Subject: [PATCH 3/6] @iklam comments --- src/hotspot/share/cds/aotArtifactFinder.cpp | 7 +++---- src/hotspot/share/cds/aotClassLinker.cpp | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/hotspot/share/cds/aotArtifactFinder.cpp b/src/hotspot/share/cds/aotArtifactFinder.cpp index 8186356c6697e..350c58983d1c9 100644 --- a/src/hotspot/share/cds/aotArtifactFinder.cpp +++ b/src/hotspot/share/cds/aotArtifactFinder.cpp @@ -210,10 +210,9 @@ void AOTArtifactFinder::add_cached_instance_class(InstanceKlass* ik) { scan_oops_in_instance_class(ik); if (ik->is_hidden() && CDSConfig::is_initing_classes_at_dump_time()) { bool succeed = AOTClassLinker::try_add_candidate(ik); - if (CDSConfig::is_dumping_method_handles()) { - guarantee(succeed, "All cached hidden classes must be aot-linkable"); - add_aot_inited_class(ik); - } + assert(CDSConfig::is_dumping_method_handles(), "sanity"); + guarantee(succeed, "All cached hidden classes must be aot-linkable"); + add_aot_inited_class(ik); } } } diff --git a/src/hotspot/share/cds/aotClassLinker.cpp b/src/hotspot/share/cds/aotClassLinker.cpp index b53bdff119310..dc539eb3d553f 100644 --- a/src/hotspot/share/cds/aotClassLinker.cpp +++ b/src/hotspot/share/cds/aotClassLinker.cpp @@ -142,8 +142,8 @@ bool AOTClassLinker::try_add_candidate(InstanceKlass* ik) { if (ik->is_hidden()) { assert(ik->shared_class_loader_type() != ClassLoader::OTHER, "must have been set"); - if (!CDSConfig::is_dumping_invokedynamic()) { - return true; + if (!CDSConfig::is_dumping_method_handles()) { + return false; } if (HeapShared::is_lambda_proxy_klass(ik)) { InstanceKlass* nest_host = ik->nest_host_not_null(); From c41728cc597ff28ac61037b42ea8fc5b02649bac Mon Sep 17 00:00:00 2001 From: Calvin Cheung Date: Fri, 28 Feb 2025 23:01:02 +0000 Subject: [PATCH 4/6] @matias9927 comments --- src/hotspot/share/cds/aotArtifactFinder.cpp | 7 +++---- src/hotspot/share/cds/aotClassInitializer.cpp | 4 ++-- src/hotspot/share/cds/cdsConfig.cpp | 4 ---- src/hotspot/share/cds/cdsConfig.hpp | 1 - src/hotspot/share/cds/heapShared.cpp | 8 ++++---- src/hotspot/share/cds/metaspaceShared.cpp | 3 --- 6 files changed, 9 insertions(+), 18 deletions(-) diff --git a/src/hotspot/share/cds/aotArtifactFinder.cpp b/src/hotspot/share/cds/aotArtifactFinder.cpp index 350c58983d1c9..b0efd3dc31657 100644 --- a/src/hotspot/share/cds/aotArtifactFinder.cpp +++ b/src/hotspot/share/cds/aotArtifactFinder.cpp @@ -136,7 +136,7 @@ void AOTArtifactFinder::find_artifacts() { #if INCLUDE_CDS_JAVA_HEAP // Keep scanning until we discover no more class that need to be AOT-initialized. - if (CDSConfig::is_initing_classes_at_dump_time()) { + if (CDSConfig::is_dumping_method_handles()) { while (_pending_aot_inited_classes->length() > 0) { InstanceKlass* ik = _pending_aot_inited_classes->pop(); HeapShared::copy_and_rescan_aot_inited_mirror(ik); @@ -176,7 +176,7 @@ void AOTArtifactFinder::end_scanning_for_oops() { } void AOTArtifactFinder::add_aot_inited_class(InstanceKlass* ik) { - if (CDSConfig::is_initing_classes_at_dump_time()) { + if (CDSConfig::is_dumping_method_handles()) { assert(ik->is_initialized(), "must be"); add_cached_instance_class(ik); @@ -208,9 +208,8 @@ void AOTArtifactFinder::add_cached_instance_class(InstanceKlass* ik) { if (created) { _all_cached_classes->append(ik); scan_oops_in_instance_class(ik); - if (ik->is_hidden() && CDSConfig::is_initing_classes_at_dump_time()) { + if (ik->is_hidden() && CDSConfig::is_dumping_method_handles()) { bool succeed = AOTClassLinker::try_add_candidate(ik); - assert(CDSConfig::is_dumping_method_handles(), "sanity"); guarantee(succeed, "All cached hidden classes must be aot-linkable"); add_aot_inited_class(ik); } diff --git a/src/hotspot/share/cds/aotClassInitializer.cpp b/src/hotspot/share/cds/aotClassInitializer.cpp index b8718ffd1ae71..927c4e1036e21 100644 --- a/src/hotspot/share/cds/aotClassInitializer.cpp +++ b/src/hotspot/share/cds/aotClassInitializer.cpp @@ -94,7 +94,7 @@ bool AOTClassInitializer::is_allowed(AllowedSpec* specs, InstanceKlass* ik) { bool AOTClassInitializer::can_archive_initialized_mirror(InstanceKlass* ik) { assert(!ArchiveBuilder::current()->is_in_buffer_space(ik), "must be source klass"); - if (!CDSConfig::is_initing_classes_at_dump_time()) { + if (!CDSConfig::is_dumping_method_handles()) { return false; } @@ -107,7 +107,7 @@ bool AOTClassInitializer::can_archive_initialized_mirror(InstanceKlass* ik) { // Automatic selection for aot-inited classes // ========================================== // - // When CDSConfig::is_initing_classes_at_dump_time() is enabled, + // When CDSConfig::is_dumping_method_handles() is enabled, // AOTArtifactFinder::find_artifacts() finds the classes of all // heap objects that are reachable from HeapShared::_run_time_special_subgraph, // and mark these classes as aot-inited. This preserves the initialized diff --git a/src/hotspot/share/cds/cdsConfig.cpp b/src/hotspot/share/cds/cdsConfig.cpp index a519caa08fabb..e1e446d5ae5bc 100644 --- a/src/hotspot/share/cds/cdsConfig.cpp +++ b/src/hotspot/share/cds/cdsConfig.cpp @@ -645,10 +645,6 @@ void CDSConfig::set_has_aot_linked_classes(bool has_aot_linked_classes) { _has_aot_linked_classes |= has_aot_linked_classes; } -bool CDSConfig::is_initing_classes_at_dump_time() { - return is_dumping_heap() && is_dumping_aot_linked_classes(); -} - bool CDSConfig::is_dumping_invokedynamic() { // Requires is_dumping_aot_linked_classes(). Otherwise the classes of some archived heap // objects used by the archive indy callsites may be replaced at runtime. diff --git a/src/hotspot/share/cds/cdsConfig.hpp b/src/hotspot/share/cds/cdsConfig.hpp index 04619f3d07608..ecfef23e45359 100644 --- a/src/hotspot/share/cds/cdsConfig.hpp +++ b/src/hotspot/share/cds/cdsConfig.hpp @@ -122,7 +122,6 @@ class CDSConfig : public AllStatic { static void disable_heap_dumping() { CDS_ONLY(_disable_heap_dumping = true); } static bool is_dumping_heap() NOT_CDS_JAVA_HEAP_RETURN_(false); static bool is_loading_heap() NOT_CDS_JAVA_HEAP_RETURN_(false); - static bool is_initing_classes_at_dump_time() NOT_CDS_JAVA_HEAP_RETURN_(false); static bool is_dumping_invokedynamic() NOT_CDS_JAVA_HEAP_RETURN_(false); static bool is_dumping_method_handles() NOT_CDS_JAVA_HEAP_RETURN_(false); diff --git a/src/hotspot/share/cds/heapShared.cpp b/src/hotspot/share/cds/heapShared.cpp index 1859b83e08254..ecde0d0688fe9 100644 --- a/src/hotspot/share/cds/heapShared.cpp +++ b/src/hotspot/share/cds/heapShared.cpp @@ -1480,13 +1480,13 @@ bool HeapShared::archive_reachable_objects_from(int level, p2i(scratch_java_mirror(orig_obj))); } - if (CDSConfig::is_initing_classes_at_dump_time()) { + if (CDSConfig::is_dumping_method_handles()) { if (java_lang_Class::is_instance(orig_obj)) { orig_obj = scratch_java_mirror(orig_obj); assert(orig_obj != nullptr, "must be archived"); } } else if (java_lang_Class::is_instance(orig_obj) && subgraph_info != _dump_time_special_subgraph) { - // Without CDSConfig::is_initing_classes_at_dump_time(), we only allow archived objects to + // Without CDSConfig::is_dumping_method_handles(), we only allow archived objects to // point to the mirrors of (1) j.l.Object, (2) primitive classes, and (3) box classes. These are initialized // very early by HeapShared::init_box_classes(). if (orig_obj == vmClasses::Object_klass()->java_mirror() @@ -1549,7 +1549,7 @@ bool HeapShared::archive_reachable_objects_from(int level, WalkOopAndArchiveClosure walker(level, record_klasses_only, subgraph_info, orig_obj); orig_obj->oop_iterate(&walker); - if (CDSConfig::is_initing_classes_at_dump_time()) { + if (CDSConfig::is_dumping_method_handles()) { // The enum klasses are archived with aot-initialized mirror. // See AOTClassInitializer::can_archive_initialized_mirror(). } else { @@ -1689,7 +1689,7 @@ void HeapShared::verify_reachable_objects_from(oop obj) { #endif void HeapShared::check_special_subgraph_classes() { - if (CDSConfig::is_initing_classes_at_dump_time()) { + if (CDSConfig::is_dumping_method_handles()) { // We can have aot-initialized classes (such as Enums) that can reference objects // of arbitrary types. Currently, we trust the JEP 483 implementation to only // aot-initialize classes that are "safe". diff --git a/src/hotspot/share/cds/metaspaceShared.cpp b/src/hotspot/share/cds/metaspaceShared.cpp index 4b74ded240d42..61df5b0e0897f 100644 --- a/src/hotspot/share/cds/metaspaceShared.cpp +++ b/src/hotspot/share/cds/metaspaceShared.cpp @@ -951,15 +951,12 @@ void MetaspaceShared::preload_and_dump_impl(StaticArchiveBuilder& builder, TRAPS vmSymbols::createArchivedObjects(), vmSymbols::void_method_signature(), CHECK); - } - if (CDSConfig::is_initing_classes_at_dump_time()) { // java.lang.Class::reflectionFactory cannot be archived yet. We set this field // to null, and it will be initialized again at runtime. log_debug(cds)("Resetting Class::reflectionFactory"); TempNewSymbol method_name = SymbolTable::new_symbol("resetArchivedStates"); Symbol* method_sig = vmSymbols::void_method_signature(); - JavaValue result(T_VOID); JavaCalls::call_static(&result, vmClasses::Class_klass(), method_name, method_sig, CHECK); From fdc3836d447f63e00eba58f00c2a568c7a3f680f Mon Sep 17 00:00:00 2001 From: Calvin Cheung Date: Sat, 1 Mar 2025 00:54:32 +0000 Subject: [PATCH 5/6] @iklam comment --- src/hotspot/share/cds/aotClassInitializer.cpp | 4 ++-- src/hotspot/share/cds/cdsConfig.cpp | 6 +++++- src/hotspot/share/cds/cdsConfig.hpp | 1 + src/hotspot/share/cds/heapShared.cpp | 8 ++++---- src/hotspot/share/cds/metaspaceShared.cpp | 3 +++ 5 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/hotspot/share/cds/aotClassInitializer.cpp b/src/hotspot/share/cds/aotClassInitializer.cpp index 927c4e1036e21..c5ad36ac315bb 100644 --- a/src/hotspot/share/cds/aotClassInitializer.cpp +++ b/src/hotspot/share/cds/aotClassInitializer.cpp @@ -94,7 +94,7 @@ bool AOTClassInitializer::is_allowed(AllowedSpec* specs, InstanceKlass* ik) { bool AOTClassInitializer::can_archive_initialized_mirror(InstanceKlass* ik) { assert(!ArchiveBuilder::current()->is_in_buffer_space(ik), "must be source klass"); - if (!CDSConfig::is_dumping_method_handles()) { + if (!CDSConfig::is_initing_classes_at_dump_time()) { return false; } @@ -107,7 +107,7 @@ bool AOTClassInitializer::can_archive_initialized_mirror(InstanceKlass* ik) { // Automatic selection for aot-inited classes // ========================================== // - // When CDSConfig::is_dumping_method_handles() is enabled, + // When CDSConfig::is_initing_classes_at_dump_time is enabled, // AOTArtifactFinder::find_artifacts() finds the classes of all // heap objects that are reachable from HeapShared::_run_time_special_subgraph, // and mark these classes as aot-inited. This preserves the initialized diff --git a/src/hotspot/share/cds/cdsConfig.cpp b/src/hotspot/share/cds/cdsConfig.cpp index e1e446d5ae5bc..9178ea1a9ee5c 100644 --- a/src/hotspot/share/cds/cdsConfig.cpp +++ b/src/hotspot/share/cds/cdsConfig.cpp @@ -645,6 +645,10 @@ void CDSConfig::set_has_aot_linked_classes(bool has_aot_linked_classes) { _has_aot_linked_classes |= has_aot_linked_classes; } +bool CDSConfig::is_initing_classes_at_dump_time() { + return is_dumping_heap() && is_dumping_aot_linked_classes(); +} + bool CDSConfig::is_dumping_invokedynamic() { // Requires is_dumping_aot_linked_classes(). Otherwise the classes of some archived heap // objects used by the archive indy callsites may be replaced at runtime. @@ -657,7 +661,7 @@ bool CDSConfig::is_dumping_invokedynamic() { // Archived MethodHandles are required for higher-level optimizations such as AOT resolution of invokedynamic // and dynamic proxies. bool CDSConfig::is_dumping_method_handles() { - return is_dumping_aot_linked_classes() && is_dumping_heap(); + return is_initing_classes_at_dump_time(); } #endif // INCLUDE_CDS_JAVA_HEAP diff --git a/src/hotspot/share/cds/cdsConfig.hpp b/src/hotspot/share/cds/cdsConfig.hpp index ecfef23e45359..04619f3d07608 100644 --- a/src/hotspot/share/cds/cdsConfig.hpp +++ b/src/hotspot/share/cds/cdsConfig.hpp @@ -122,6 +122,7 @@ class CDSConfig : public AllStatic { static void disable_heap_dumping() { CDS_ONLY(_disable_heap_dumping = true); } static bool is_dumping_heap() NOT_CDS_JAVA_HEAP_RETURN_(false); static bool is_loading_heap() NOT_CDS_JAVA_HEAP_RETURN_(false); + static bool is_initing_classes_at_dump_time() NOT_CDS_JAVA_HEAP_RETURN_(false); static bool is_dumping_invokedynamic() NOT_CDS_JAVA_HEAP_RETURN_(false); static bool is_dumping_method_handles() NOT_CDS_JAVA_HEAP_RETURN_(false); diff --git a/src/hotspot/share/cds/heapShared.cpp b/src/hotspot/share/cds/heapShared.cpp index ecde0d0688fe9..1859b83e08254 100644 --- a/src/hotspot/share/cds/heapShared.cpp +++ b/src/hotspot/share/cds/heapShared.cpp @@ -1480,13 +1480,13 @@ bool HeapShared::archive_reachable_objects_from(int level, p2i(scratch_java_mirror(orig_obj))); } - if (CDSConfig::is_dumping_method_handles()) { + if (CDSConfig::is_initing_classes_at_dump_time()) { if (java_lang_Class::is_instance(orig_obj)) { orig_obj = scratch_java_mirror(orig_obj); assert(orig_obj != nullptr, "must be archived"); } } else if (java_lang_Class::is_instance(orig_obj) && subgraph_info != _dump_time_special_subgraph) { - // Without CDSConfig::is_dumping_method_handles(), we only allow archived objects to + // Without CDSConfig::is_initing_classes_at_dump_time(), we only allow archived objects to // point to the mirrors of (1) j.l.Object, (2) primitive classes, and (3) box classes. These are initialized // very early by HeapShared::init_box_classes(). if (orig_obj == vmClasses::Object_klass()->java_mirror() @@ -1549,7 +1549,7 @@ bool HeapShared::archive_reachable_objects_from(int level, WalkOopAndArchiveClosure walker(level, record_klasses_only, subgraph_info, orig_obj); orig_obj->oop_iterate(&walker); - if (CDSConfig::is_dumping_method_handles()) { + if (CDSConfig::is_initing_classes_at_dump_time()) { // The enum klasses are archived with aot-initialized mirror. // See AOTClassInitializer::can_archive_initialized_mirror(). } else { @@ -1689,7 +1689,7 @@ void HeapShared::verify_reachable_objects_from(oop obj) { #endif void HeapShared::check_special_subgraph_classes() { - if (CDSConfig::is_dumping_method_handles()) { + if (CDSConfig::is_initing_classes_at_dump_time()) { // We can have aot-initialized classes (such as Enums) that can reference objects // of arbitrary types. Currently, we trust the JEP 483 implementation to only // aot-initialize classes that are "safe". diff --git a/src/hotspot/share/cds/metaspaceShared.cpp b/src/hotspot/share/cds/metaspaceShared.cpp index 61df5b0e0897f..4b74ded240d42 100644 --- a/src/hotspot/share/cds/metaspaceShared.cpp +++ b/src/hotspot/share/cds/metaspaceShared.cpp @@ -951,12 +951,15 @@ void MetaspaceShared::preload_and_dump_impl(StaticArchiveBuilder& builder, TRAPS vmSymbols::createArchivedObjects(), vmSymbols::void_method_signature(), CHECK); + } + if (CDSConfig::is_initing_classes_at_dump_time()) { // java.lang.Class::reflectionFactory cannot be archived yet. We set this field // to null, and it will be initialized again at runtime. log_debug(cds)("Resetting Class::reflectionFactory"); TempNewSymbol method_name = SymbolTable::new_symbol("resetArchivedStates"); Symbol* method_sig = vmSymbols::void_method_signature(); + JavaValue result(T_VOID); JavaCalls::call_static(&result, vmClasses::Class_klass(), method_name, method_sig, CHECK); From f2bdcfabce0120e94b5eb0be7e0f8e512bb6ea14 Mon Sep 17 00:00:00 2001 From: Calvin Cheung Date: Mon, 3 Mar 2025 16:33:16 +0000 Subject: [PATCH 6/6] revert changes in aotArtifactFinder.cpp --- src/hotspot/share/cds/aotArtifactFinder.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hotspot/share/cds/aotArtifactFinder.cpp b/src/hotspot/share/cds/aotArtifactFinder.cpp index b0efd3dc31657..48f31635c63b9 100644 --- a/src/hotspot/share/cds/aotArtifactFinder.cpp +++ b/src/hotspot/share/cds/aotArtifactFinder.cpp @@ -136,7 +136,7 @@ void AOTArtifactFinder::find_artifacts() { #if INCLUDE_CDS_JAVA_HEAP // Keep scanning until we discover no more class that need to be AOT-initialized. - if (CDSConfig::is_dumping_method_handles()) { + if (CDSConfig::is_initing_classes_at_dump_time()) { while (_pending_aot_inited_classes->length() > 0) { InstanceKlass* ik = _pending_aot_inited_classes->pop(); HeapShared::copy_and_rescan_aot_inited_mirror(ik); @@ -176,7 +176,7 @@ void AOTArtifactFinder::end_scanning_for_oops() { } void AOTArtifactFinder::add_aot_inited_class(InstanceKlass* ik) { - if (CDSConfig::is_dumping_method_handles()) { + if (CDSConfig::is_initing_classes_at_dump_time()) { assert(ik->is_initialized(), "must be"); add_cached_instance_class(ik); @@ -208,7 +208,7 @@ void AOTArtifactFinder::add_cached_instance_class(InstanceKlass* ik) { if (created) { _all_cached_classes->append(ik); scan_oops_in_instance_class(ik); - if (ik->is_hidden() && CDSConfig::is_dumping_method_handles()) { + if (ik->is_hidden() && CDSConfig::is_initing_classes_at_dump_time()) { bool succeed = AOTClassLinker::try_add_candidate(ik); guarantee(succeed, "All cached hidden classes must be aot-linkable"); add_aot_inited_class(ik);