From a23331a4505c830fd576a45ef52e990ebc2d9ed0 Mon Sep 17 00:00:00 2001 From: Mat Carter Date: Mon, 24 Feb 2025 19:53:47 +0000 Subject: [PATCH 01/16] 8354897: Support Soft/Weak Reference in AOT cache (imported from Leyden repo) --- src/hotspot/share/cds/aotClassInitializer.cpp | 3 +- src/hotspot/share/cds/cdsConfig.cpp | 3 - src/hotspot/share/cds/metaspaceShared.cpp | 12 - src/hotspot/share/classfile/javaClasses.cpp | 6 - src/hotspot/share/classfile/vmSymbols.hpp | 1 - .../java/lang/invoke/LambdaFormEditor.java | 19 +- .../java/lang/invoke/MethodHandleNatives.java | 18 -- .../classes/java/lang/invoke/MethodType.java | 41 --- .../java/lang/invoke/MethodTypeForm.java | 44 +-- .../classes/java/lang/ref/Reference.java | 4 + .../aotClassLinking/WeakReferenceTest.java | 295 ++++++++++++++++++ 11 files changed, 314 insertions(+), 132 deletions(-) create mode 100644 test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/WeakReferenceTest.java diff --git a/src/hotspot/share/cds/aotClassInitializer.cpp b/src/hotspot/share/cds/aotClassInitializer.cpp index 297f8109eb4ba..be7e1f31d8a52 100644 --- a/src/hotspot/share/cds/aotClassInitializer.cpp +++ b/src/hotspot/share/cds/aotClassInitializer.cpp @@ -338,7 +338,8 @@ bool AOTClassInitializer::can_archive_initialized_mirror(InstanceKlass* ik) { bool AOTClassInitializer::is_runtime_setup_required(InstanceKlass* ik) { return ik == vmClasses::Class_klass() || ik == vmClasses::internal_Unsafe_klass() || - ik == vmClasses::ConcurrentHashMap_klass(); + ik == vmClasses::ConcurrentHashMap_klass() || + ik == vmClasses::Reference_klass(); } void AOTClassInitializer::call_runtime_setup(JavaThread* current, InstanceKlass* ik) { diff --git a/src/hotspot/share/cds/cdsConfig.cpp b/src/hotspot/share/cds/cdsConfig.cpp index 64ad07b0cf816..9af83653351cf 100644 --- a/src/hotspot/share/cds/cdsConfig.cpp +++ b/src/hotspot/share/cds/cdsConfig.cpp @@ -536,9 +536,6 @@ bool CDSConfig::check_vm_args_consistency(bool patch_mod_javabase, bool mode_fla // run to another which resulting in non-determinstic CDS archives. // Disable UseStringDeduplication while dumping CDS archive. UseStringDeduplication = false; - - // Don't use SoftReferences so that objects used by java.lang.invoke tables can be archived. - Arguments::PropertyList_add(new SystemProperty("java.lang.invoke.MethodHandleNatives.USE_SOFT_CACHE", "false", false)); } // RecordDynamicDumpInfo is not compatible with ArchiveClassesAtExit diff --git a/src/hotspot/share/cds/metaspaceShared.cpp b/src/hotspot/share/cds/metaspaceShared.cpp index ef2a6dcb8e63c..7d4b39591de3b 100644 --- a/src/hotspot/share/cds/metaspaceShared.cpp +++ b/src/hotspot/share/cds/metaspaceShared.cpp @@ -967,18 +967,6 @@ void MetaspaceShared::preload_and_dump_impl(StaticArchiveBuilder& builder, TRAPS HeapShared::reset_archived_object_states(CHECK); } - 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"); - - JavaValue result(T_VOID); - JavaCalls::call_static(&result, vmClasses::MethodType_klass(), - 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. diff --git a/src/hotspot/share/classfile/javaClasses.cpp b/src/hotspot/share/classfile/javaClasses.cpp index a2ad1dce5e4a8..7b30f24017808 100644 --- a/src/hotspot/share/classfile/javaClasses.cpp +++ b/src/hotspot/share/classfile/javaClasses.cpp @@ -5460,12 +5460,6 @@ bool JavaClasses::is_supported_for_archiving(oop obj) { } } - if (klass->is_subclass_of(vmClasses::Reference_klass())) { - // It's problematic to archive Reference objects. One of the reasons is that - // Reference::discovered may pull in unwanted objects (see JDK-8284336) - return false; - } - return true; } #endif diff --git a/src/hotspot/share/classfile/vmSymbols.hpp b/src/hotspot/share/classfile/vmSymbols.hpp index e66066738ef38..f9f6bd0725432 100644 --- a/src/hotspot/share/classfile/vmSymbols.hpp +++ b/src/hotspot/share/classfile/vmSymbols.hpp @@ -719,7 +719,6 @@ class SerializeClosure; JFR_TEMPLATES(template) \ \ /* CDS */ \ - template(createArchivedObjects, "createArchivedObjects") \ template(dumpSharedArchive, "dumpSharedArchive") \ template(dumpSharedArchive_signature, "(ZLjava/lang/String;)Ljava/lang/String;") \ template(generateLambdaFormHolderClasses, "generateLambdaFormHolderClasses") \ diff --git a/src/java.base/share/classes/java/lang/invoke/LambdaFormEditor.java b/src/java.base/share/classes/java/lang/invoke/LambdaFormEditor.java index 6a25bf9c0a83a..471de5aa48fd4 100644 --- a/src/java.base/share/classes/java/lang/invoke/LambdaFormEditor.java +++ b/src/java.base/share/classes/java/lang/invoke/LambdaFormEditor.java @@ -38,7 +38,6 @@ import static java.lang.invoke.MethodHandleImpl.Intrinsic; import static java.lang.invoke.MethodHandleImpl.NF_loop; import static java.lang.invoke.MethodHandleImpl.makeIntrinsic; -import static java.lang.invoke.MethodHandleNatives.USE_SOFT_CACHE; /** Transforms on LFs. * A lambda-form editor can derive new LFs from its base LF. @@ -90,17 +89,12 @@ static LambdaFormEditor lambdaFormEditor(LambdaForm lambdaForm) { * Tightly coupled with the TransformKey class, which is used to lookup existing * Transforms. */ - private static final class Transform { - final Object cache; + private static final class Transform extends SoftReference { final long packedBytes; final byte[] fullBytes; private Transform(long packedBytes, byte[] fullBytes, LambdaForm result) { - if (USE_SOFT_CACHE) { - cache = new SoftReference(result); - } else { - cache = result; - } + super(result); this.packedBytes = packedBytes; this.fullBytes = fullBytes; } @@ -141,15 +135,6 @@ public String toString() { } return buf.toString(); } - - @SuppressWarnings("unchecked") - public LambdaForm get() { - if (cache instanceof LambdaForm lf) { - return lf; - } else { - return ((SoftReference)cache).get(); - } - } } /** diff --git a/src/java.base/share/classes/java/lang/invoke/MethodHandleNatives.java b/src/java.base/share/classes/java/lang/invoke/MethodHandleNatives.java index 9df7d25258d8e..4e29af859bb0b 100644 --- a/src/java.base/share/classes/java/lang/invoke/MethodHandleNatives.java +++ b/src/java.base/share/classes/java/lang/invoke/MethodHandleNatives.java @@ -665,22 +665,4 @@ static boolean canBeCalledVirtual(MemberName symbolicRef, Class definingClass return (definingClass.isAssignableFrom(symbolicRefClass) || // Msym overrides Mdef symbolicRefClass.isInterface()); // Mdef implements Msym } - - //--- AOTCache support - - /** - * In normal execution, this is set to true, so that LambdaFormEditor and MethodTypeForm will - * use soft references to allow class unloading. - * - * When dumping the AOTCache, this is set to false so that no cached heap objects will - * contain soft references (which are not yet supported by AOTCache - see JDK-8341587). AOTCache - * only stores LambdaFormEditors and MethodTypeForms for classes in the boot/platform/app loaders. - * Such classes will never be unloaded, so it's OK to use hard references. - */ - static final boolean USE_SOFT_CACHE; - - static { - USE_SOFT_CACHE = Boolean.parseBoolean( - System.getProperty("java.lang.invoke.MethodHandleNatives.USE_SOFT_CACHE", "true")); - } } diff --git a/src/java.base/share/classes/java/lang/invoke/MethodType.java b/src/java.base/share/classes/java/lang/invoke/MethodType.java index 5e1c0f8581c9a..697676db2c155 100644 --- a/src/java.base/share/classes/java/lang/invoke/MethodType.java +++ b/src/java.base/share/classes/java/lang/invoke/MethodType.java @@ -31,8 +31,6 @@ import java.util.Arrays; import java.util.Collections; import java.util.function.Supplier; -import java.util.HashMap; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; @@ -42,7 +40,6 @@ import jdk.internal.util.ReferencedKeySet; import jdk.internal.util.ReferenceKey; -import jdk.internal.misc.CDS; import jdk.internal.vm.annotation.Stable; import sun.invoke.util.BytecodeDescriptor; import sun.invoke.util.VerifyType; @@ -394,17 +391,6 @@ private static MethodType makeImpl(Class rtype, Class[] ptypes, boolean tr ptypes = NO_PTYPES; trusted = true; } MethodType primordialMT = new MethodType(rtype, ptypes); - if (archivedMethodTypes != null) { - // If this JVM process reads from archivedMethodTypes, it never - // modifies the table. So there's no need for synchronization. - // See copyInternTable() below. - assert CDS.isUsingArchive(); - MethodType mt = archivedMethodTypes.get(primordialMT); - if (mt != null) { - return mt; - } - } - MethodType mt = internTable.get(primordialMT); if (mt != null) return mt; @@ -425,7 +411,6 @@ private static MethodType makeImpl(Class rtype, Class[] ptypes, boolean tr } private static final @Stable MethodType[] objectOnlyTypes = new MethodType[20]; - private static @Stable HashMap archivedMethodTypes; /** * Finds or creates a method type whose components are {@code Object} with an optional trailing {@code Object[]} array. @@ -1396,30 +1381,4 @@ private Object readResolve() { wrapAlt = null; return mt; } - - static HashMap copyInternTable() { - HashMap copy = new HashMap<>(); - - for (Iterator i = internTable.iterator(); i.hasNext(); ) { - MethodType t = i.next(); - copy.put(t, t); - } - - return copy; - } - - // This is called from C code, at the very end of Java code execution - // during the AOT cache assembly phase. - static void createArchivedObjects() { - // After the archivedMethodTypes field is assigned, this table - // is never modified. So we don't need synchronization when reading from - // it (which happens only in a future JVM process, never in the current process). - // - // @implNote CDS.isDumpingStaticArchive() is mutually exclusive with - // CDS.isUsingArchive(); at most one of them can return true for any given JVM - // process. - assert CDS.isDumpingStaticArchive(); - archivedMethodTypes = copyInternTable(); - internTable.clear(); - } } diff --git a/src/java.base/share/classes/java/lang/invoke/MethodTypeForm.java b/src/java.base/share/classes/java/lang/invoke/MethodTypeForm.java index 8bbc4dd5f727a..d5272337585cc 100644 --- a/src/java.base/share/classes/java/lang/invoke/MethodTypeForm.java +++ b/src/java.base/share/classes/java/lang/invoke/MethodTypeForm.java @@ -30,7 +30,6 @@ import java.lang.ref.SoftReference; import static java.lang.invoke.MethodHandleStatics.newIllegalArgumentException; -import static java.lang.invoke.MethodHandleNatives.USE_SOFT_CACHE; /** * Shared information for a group of method types, which differ @@ -52,7 +51,7 @@ final class MethodTypeForm { final MethodType basicType; // the canonical erasure, with primitives simplified // Cached adapter information: - private final Object[] methodHandles; + private final SoftReference[] methodHandles; // Indexes into methodHandles: static final int @@ -62,7 +61,7 @@ final class MethodTypeForm { MH_LIMIT = 3; // Cached lambda form information, for basic types only: - private final Object[] lambdaForms; + private final SoftReference[] lambdaForms; private SoftReference interpretEntry; @@ -111,16 +110,9 @@ public MethodType basicType() { return basicType; } - @SuppressWarnings("unchecked") public MethodHandle cachedMethodHandle(int which) { - Object entry = methodHandles[which]; - if (entry == null) { - return null; - } else if (entry instanceof MethodHandle mh) { - return mh; - } else { - return ((SoftReference)entry).get(); - } + SoftReference entry = methodHandles[which]; + return (entry != null) ? entry.get() : null; } public synchronized MethodHandle setCachedMethodHandle(int which, MethodHandle mh) { @@ -129,24 +121,13 @@ public synchronized MethodHandle setCachedMethodHandle(int which, MethodHandle m if (prev != null) { return prev; } - if (USE_SOFT_CACHE) { - methodHandles[which] = new SoftReference<>(mh); - } else { - methodHandles[which] = mh; - } + methodHandles[which] = new SoftReference<>(mh); return mh; } - @SuppressWarnings("unchecked") public LambdaForm cachedLambdaForm(int which) { - Object entry = lambdaForms[which]; - if (entry == null) { - return null; - } else if (entry instanceof LambdaForm lf) { - return lf; - } else { - return ((SoftReference)entry).get(); - } + SoftReference entry = lambdaForms[which]; + return (entry != null) ? entry.get() : null; } public synchronized LambdaForm setCachedLambdaForm(int which, LambdaForm form) { @@ -155,11 +136,7 @@ public synchronized LambdaForm setCachedLambdaForm(int which, LambdaForm form) { if (prev != null) { return prev; } - if (USE_SOFT_CACHE) { - lambdaForms[which] = new SoftReference<>(form); - } else { - lambdaForms[which] = form; - } + lambdaForms[which] = new SoftReference<>(form); return form; } @@ -181,6 +158,7 @@ public synchronized MemberName setCachedInterpretEntry(MemberName mn) { * This MTF will stand for that type and all un-erased variations. * Eagerly compute some basic properties of the type, common to all variations. */ + @SuppressWarnings({"rawtypes", "unchecked"}) protected MethodTypeForm(MethodType erasedType) { this.erasedType = erasedType; @@ -221,8 +199,8 @@ protected MethodTypeForm(MethodType erasedType) { this.primitiveCount = primitiveCount; this.parameterSlotCount = (short)pslotCount; - this.lambdaForms = new Object[LF_LIMIT]; - this.methodHandles = new Object[MH_LIMIT]; + this.lambdaForms = new SoftReference[LF_LIMIT]; + this.methodHandles = new SoftReference[MH_LIMIT]; } else { this.basicType = MethodType.methodType(basicReturnType, basicPtypes, true); // fill in rest of data from the basic type: diff --git a/src/java.base/share/classes/java/lang/ref/Reference.java b/src/java.base/share/classes/java/lang/ref/Reference.java index 13ba76e5dd2f2..995fe3c3d867e 100644 --- a/src/java.base/share/classes/java/lang/ref/Reference.java +++ b/src/java.base/share/classes/java/lang/ref/Reference.java @@ -307,6 +307,10 @@ static void startReferenceHandlerThread(ThreadGroup tg) { } static { + runtimeSetup(); + } + + private static void runtimeSetup() { // provide access in SharedSecrets SharedSecrets.setJavaLangRefAccess(new JavaLangRefAccess() { @Override diff --git a/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/WeakReferenceTest.java b/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/WeakReferenceTest.java new file mode 100644 index 0000000000000..2a7a8a69ffffc --- /dev/null +++ b/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/WeakReferenceTest.java @@ -0,0 +1,295 @@ +/* + * Copyright (c) 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. + * + */ + +/* + * @test various test cases for archived WeakReference objects. + * @requires vm.cds.write.archived.java.heap + * @requires vm.cds.supports.aot.class.linking + * @requires vm.debug + * @comment work around JDK-8345635 + * @requires !vm.jvmci.enabled + * @library /test/jdk/lib/testlibrary /test/lib /test/hotspot/jtreg/runtime/cds/appcds + * @build WeakReferenceTest + * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar weakref.jar + * WeakReferenceTestApp WeakReferenceTestApp$Inner ShouldNotBeAOTInited ShouldNotBeArchived SharedQueue + * @run driver WeakReferenceTest AOT + */ + +import java.lang.ref.WeakReference; +import java.lang.ref.ReferenceQueue; +import jdk.test.lib.cds.CDSAppTester; +import jdk.test.lib.process.OutputAnalyzer; +import jdk.test.lib.helpers.ClassFileInstaller; + +public class WeakReferenceTest { + static final String appJar = ClassFileInstaller.getJarPath("weakref.jar"); + static final String mainClass = "WeakReferenceTestApp"; + + public static void main(String[] args) throws Exception { + Tester t = new Tester(); + t.run(args); + } + + static class Tester extends CDSAppTester { + public Tester() { + super(mainClass); + } + + @Override + public String classpath(RunMode runMode) { + return appJar; + } + + @Override + public String[] vmArgs(RunMode runMode) { + if (runMode == RunMode.ASSEMBLY) { + return new String[] { + "-Xlog:gc,cds+class=debug", + "-XX:AOTInitTestClass=WeakReferenceTestApp", + "-Xlog:cds+map,cds+map+oops=trace:file=cds.oops.txt:none:filesize=0", + }; + } else { + return new String[] { + "-Xlog:gc", + }; + } + } + + @Override + public String[] appCommandLine(RunMode runMode) { + return new String[] { + mainClass, + runMode.toString(), + }; + } + + @Override + public void checkExecution(OutputAnalyzer out, RunMode runMode) throws Exception { + out.shouldHaveExitValue(0); + out.shouldNotContain("Unexpected exception:"); + } + } +} + +class WeakReferenceTestApp { + // This class is NOT aot-initialized + static class Inner { + static boolean WeakReferenceTestApp_clinit_executed; + } + + static { + Inner.WeakReferenceTestApp_clinit_executed = true; + + // During the assembly phase, this block of code is called during the assembly + // phase (triggered by the -XX:AOTInitTestClass=WeakReferenceTestApp flag). + // It runs the clinit_for_testXXX() method to set up the aot-initialized data structures + // that are used by each testXXX() function. + // + // Note that this function is also called during the training run. + // This function is NOT called during the production run, because WeakReferenceTestApp + // is aot-initialized. + + clinit_for_testCollectedInAssembly(); + clinit_for_testWeakReferenceCollection(); + clinit_for_testQueue(); + } + + static WeakReference makeRef() { + System.out.println("WeakReferenceTestApp::makeRef() is executed"); + WeakReference r = new WeakReference(root); + System.out.println("r.get() = " + r.get()); + + ShouldNotBeAOTInited.doit(); + return r; + } + + static WeakReference makeRef2() { + return new WeakReference(new WeakReferenceTestApp()); + } + + public static void main(String[] args) { + try { + runTests(args); + } catch (Throwable t) { + System.err.println("Unexpected exception:"); + t.printStackTrace(); + System.exit(1); + } + } + + static void runTests(String[] args) throws Exception { + boolean isProduction = args[0].equals("PRODUCTION"); + + if (isProduction && Inner.WeakReferenceTestApp_clinit_executed) { + throw new RuntimeException("WeakReferenceTestApp should have been aot-inited"); + } + + if (isProduction) { + // A GC should have happened before the heap objects are written into + // the AOT cache. So any unreachable referents should have been collected. + } else { + // We are in the training run. Simulate the GC mentioned in the above comment, + // so the test cases should observe the same states as in the production run. + System.gc(); + } + + testCollectedInAssembly(isProduction); + testWeakReferenceCollection(isProduction); + testQueue(isProduction); + } + + //---------------------------------------------------------------------- + // Set up for testCollectedInAssembly() + static WeakReference refToCollectedObj; + + static void clinit_for_testCollectedInAssembly() { + // The referent will be GC-ed in the assembly run when the JVM forces a full GC. + refToCollectedObj = new WeakReference(new String("collected in assembly")); + } + + // [TEST CASE] Test the storage of a WeakReference whose referent has been collected during the assembly phase. + static void testCollectedInAssembly(boolean isProduction) { + System.out.println("refToCollectedObj.get() = " + refToCollectedObj.get()); + System.out.println("refToCollectedObj.isEnqueued() = " + refToCollectedObj.isEnqueued()); + + if (refToCollectedObj.get() != null) { + throw new RuntimeException("refToCollectedObj.get() should have been GC'ed"); + } + + /* + * FIXME -- why does this fail, even in training run? + + if (!refToCollectedObj.isEnqueued()) { + throw new RuntimeException("refToCollectedObj.isEnqueued() should be true"); + } + */ + } + + //---------------------------------------------------------------------- + // Set up for testWeakReferenceCollection() + static Object root; + static WeakReference ref; + + static void clinit_for_testWeakReferenceCollection() { + root = new WeakReferenceTestApp(); + ref = makeRef(); + } + + // [TEST CASE] A WeakReference allocated in assembly phase should be collectable in the production run + static void testWeakReferenceCollection(boolean isProduction) { + WeakReference ref2 = makeRef2(); + System.out.println("ref.get() = " + ref.get()); // created during assembly phase + System.out.println("ref2.get() = " + ref2.get()); // created during production run + + if (ref.get() == null) { + throw new RuntimeException("ref.get() should not be null"); + } + if (ref2.get() == null) { + throw new RuntimeException("ref2.get() should not be null"); + } + + System.out.println("... running GC ..."); + root = null; + System.gc(); + + System.out.println("ref.get() = " + ref.get()); + System.out.println("ref2.get() = " + ref2.get()); + + if (ref.get() != null) { + throw new RuntimeException("ref.get() should be null"); + } + if (ref2.get() != null) { + throw new RuntimeException("ref2.get() should be null"); + } + + System.out.println("ShouldNotBeAOTInited.doit_executed = " + ShouldNotBeAOTInited.doit_executed); + if (isProduction && ShouldNotBeAOTInited.doit_executed) { + throw new RuntimeException("ShouldNotBeAOTInited should not have been aot-inited"); + } + } + + //---------------------------------------------------------------------- + // Set up for testQueue() + static WeakReference refWithQueue; + static SharedQueue sharedQueueInstance; + + static void clinit_for_testQueue() { + // Make sure SharedQueue is also cached in *initialized* state. + sharedQueueInstance = SharedQueue.sharedQueueInstance; + + refWithQueue = new WeakReference(String.class, SharedQueue.queue()); + ShouldNotBeArchived.ref = new WeakReference(ShouldNotBeArchived.instance, SharedQueue.queue()); + + + // Set to 2 in training run and assembly phase, but this state shouldn't be stored in + // AOT cache. + ShouldNotBeArchived.state = 2; + } + + // [TEST CASE] Unrelated WeakReferences shouldn't be cached even if they are registered with the same queue + static void testQueue(boolean isProduction) { + System.out.println("refWithQueue.get() = " + refWithQueue.get()); + System.out.println("ShouldNotBeArchived.state = " + ShouldNotBeArchived.state); + + // [1] Although refWithQueue and ShouldNotBeArchived.ref are registered with the same queue, as both + // of their referents are strongly referenced, they are not added to the queue's "head". + // (Per javadoc: "registered reference objects are appended by the garbage collector after the + // appropriate reachability changes are detected"); + // [2] When the assembly phase scans refWithQueue, it shouldn't discover ShouldNotBeArchived.ref (via the queue), + // so ShouldNotBeArchived.ref should not be stored in the AOT cache. + // [3] As a result, ShouldNotBeArchived should be cached in the *not initialized" state. Its + // will be executed in the production run to set ShouldNotBeArchived.state to 1. + if (isProduction && ShouldNotBeArchived.state != 1) { + throw new RuntimeException("ShouldNotBeArchived should be 1 but is " + ShouldNotBeArchived.state); + } + } +} + +class ShouldNotBeAOTInited { + static WeakReference ref; + static boolean doit_executed; + static { + System.out.println("ShouldNotBeAOTInited. called"); + } + static void doit() { + System.out.println("ShouldNotBeAOTInited.doit()> called"); + doit_executed = true; + ref = new WeakReference(new ShouldNotBeAOTInited()); + } +} + +class ShouldNotBeArchived { + static ShouldNotBeArchived instance = new ShouldNotBeArchived(); + static WeakReference ref; + static int state = 1; +} + +class SharedQueue { + static SharedQueue sharedQueueInstance = new SharedQueue(); + private ReferenceQueue theQueue = new ReferenceQueue(); + + static ReferenceQueue queue() { + return sharedQueueInstance.theQueue; + } +} From c6beb854c7b33304bb12d8942ebc1c2fee99a52f Mon Sep 17 00:00:00 2001 From: iklam Date: Fri, 18 Apr 2025 11:25:37 -0700 Subject: [PATCH 02/16] Added code to process Universe::reference_pending_list() --- src/hotspot/share/cds/metaspaceShared.cpp | 46 ++++++++++++ src/hotspot/share/cds/metaspaceShared.hpp | 1 + .../java/lang/invoke/MethodHandleNatives.java | 2 +- .../classes/java/lang/invoke/MethodType.java | 2 +- .../classes/java/lang/ref/Reference.java | 9 ++- .../aotClassLinking/TestSetupAOTTest.java | 57 ++++++++++++++ test/setup_aot/TestSetupAOT.java | 74 +++++++++++++++---- 7 files changed, 174 insertions(+), 17 deletions(-) create mode 100644 test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/TestSetupAOTTest.java diff --git a/src/hotspot/share/cds/metaspaceShared.cpp b/src/hotspot/share/cds/metaspaceShared.cpp index 7d4b39591de3b..801c1cbf93f72 100644 --- a/src/hotspot/share/cds/metaspaceShared.cpp +++ b/src/hotspot/share/cds/metaspaceShared.cpp @@ -902,6 +902,51 @@ void MetaspaceShared::exercise_runtime_cds_code(TRAPS) { CDSProtectionDomain::to_file_URL("dummy.jar", Handle(), CHECK); } +#if INCLUDE_CDS_JAVA_HEAP +void MetaspaceShared::process_pending_references(TRAPS) { + precond(CDSConfig::is_dumping_heap()); + precond(CDSConfig::allow_only_single_java_thread()); + + // When dumping heap objects, the JVM runs a single Java thread, so the ReferenceHandler + // thread is not running. We need to ensure that all objects in Universe::reference_pending_list() + // are processed. Otherwise we might pull in unwanted objects into the archived heap. Example: + // + // class UnwantedWeakRef extends WeakReference { Object junk = ...; } + // WeakReference A = ...; + // UnwantedWeakRef B = ...; + // + // If the referent of both A and B are collected, A and B will be placed in + // the Universe::reference_pending_list() by the GC, which A.next == B. + // + // When HeapShared::archiveObject() find A, it will find B from A.next and end up + // dumping the object B.junk that we don't want. + // + // After ReferenceHandler::processPendingReferences0() completes, A.next will be + // cleared and we won't be able to find B anymore. + + Symbol* klass_name = SymbolTable::new_symbol("java/lang/ref/Reference$ReferenceHandler"); + Klass* k = SystemDictionary::resolve_or_fail(klass_name, true, CHECK); + Symbol* method_name = SymbolTable::new_symbol("processPendingReferences0"); + Symbol* method_sig = vmSymbols::void_method_signature(); + + while (true) { + { + MonitorLocker ml(Heap_lock); + if (!Universe::has_reference_pending_list()) { + break; + } + } + + JavaValue result(T_VOID); + JavaCalls::call_static(&result, k, method_name, method_sig, CHECK); + + // In case ReferenceHandler::processPendingReferences0() creates garbage and causes + // more refs to be added to Universe::reference_pending_list() + Universe::heap()->collect(GCCause::_java_lang_system_gc); + } +} +#endif + void MetaspaceShared::preload_and_dump_impl(StaticArchiveBuilder& builder, TRAPS) { if (CDSConfig::is_dumping_classic_static_archive()) { // We are running with -Xshare:dump @@ -984,6 +1029,7 @@ void MetaspaceShared::preload_and_dump_impl(StaticArchiveBuilder& builder, TRAPS // Do this at the very end, when no Java code will be executed. Otherwise // some new strings may be added to the intern table. StringTable::allocate_shared_strings_array(CHECK); + process_pending_references(CHECK); } else { log_info(cds)("Not dumping heap, reset CDSConfig::_is_using_optimized_module_handling"); CDSConfig::stop_using_optimized_module_handling(); diff --git a/src/hotspot/share/cds/metaspaceShared.hpp b/src/hotspot/share/cds/metaspaceShared.hpp index e03994be1b993..ea56132b381db 100644 --- a/src/hotspot/share/cds/metaspaceShared.hpp +++ b/src/hotspot/share/cds/metaspaceShared.hpp @@ -195,5 +195,6 @@ class MetaspaceShared : AllStatic { static MapArchiveResult map_archive(FileMapInfo* mapinfo, char* mapped_base_address, ReservedSpace rs); static void unmap_archive(FileMapInfo* mapinfo); static void get_default_classlist(char* default_classlist, const size_t buf_size); + static void process_pending_references(TRAPS); }; #endif // SHARE_CDS_METASPACESHARED_HPP diff --git a/src/java.base/share/classes/java/lang/invoke/MethodHandleNatives.java b/src/java.base/share/classes/java/lang/invoke/MethodHandleNatives.java index 4e29af859bb0b..0db7a6a8ddb98 100644 --- a/src/java.base/share/classes/java/lang/invoke/MethodHandleNatives.java +++ b/src/java.base/share/classes/java/lang/invoke/MethodHandleNatives.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2008, 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 diff --git a/src/java.base/share/classes/java/lang/invoke/MethodType.java b/src/java.base/share/classes/java/lang/invoke/MethodType.java index 697676db2c155..704d4994291d2 100644 --- a/src/java.base/share/classes/java/lang/invoke/MethodType.java +++ b/src/java.base/share/classes/java/lang/invoke/MethodType.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2008, 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 diff --git a/src/java.base/share/classes/java/lang/ref/Reference.java b/src/java.base/share/classes/java/lang/ref/Reference.java index 995fe3c3d867e..b62efb47f8aa8 100644 --- a/src/java.base/share/classes/java/lang/ref/Reference.java +++ b/src/java.base/share/classes/java/lang/ref/Reference.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 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 @@ -208,6 +208,13 @@ public void run() { processPendingReferences(); } } + + // Called by the JVM during AOT cache creation (which does not + // launch the ReferenceHandler thread). + static void processPendingReferences0() { + Unsafe.getUnsafe().ensureClassInitialized(Cleaner.class); + processPendingReferences(); + } } /* diff --git a/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/TestSetupAOTTest.java b/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/TestSetupAOTTest.java new file mode 100644 index 0000000000000..a9519a6ef769e --- /dev/null +++ b/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/TestSetupAOTTest.java @@ -0,0 +1,57 @@ +/* + * Copyright (c) 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. + * + */ + +/** + * @test + * @summary This is a test case for creating an AOT cache using the setup_aot/TestSetupAOT.java program, which + * is used for running HotSpot tests in the "AOT mode" + * (E.g., make test JTREG=AOT_JDK=true TEST=open/test/hotspot/jtreg/runtime/invokedynamic) + * @requires vm.cds + * @comment work around JDK-8345635 + * @requires !vm.jvmci.enabled + * @library /test/lib /test/setup_aot + * @build TestSetupAOTTest JavacBenchApp TestSetupAOT + * @run driver jdk.test.lib.helpers.ClassFileInstaller + * TestSetupAOT + * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar app.jar + * TestSetupAOT + * TestSetupAOT$ToolOutput + * JavacBenchApp + * JavacBenchApp$ClassFile + * JavacBenchApp$FileManager + * JavacBenchApp$SourceFile + * @run driver TestSetupAOTTest + */ + +import jdk.test.lib.cds.SimpleCDSAppTester; +import jdk.test.lib.process.OutputAnalyzer; + +public class TestSetupAOTTest { + public static void main(String... args) throws Exception { + SimpleCDSAppTester.of("TestSetupAOT") + .classpath("app.jar") + .appCommandLine("TestSetupAOT", ".") + .runAOTWorkflow(); + } +} diff --git a/test/setup_aot/TestSetupAOT.java b/test/setup_aot/TestSetupAOT.java index 89a6dd30c72cf..13e269692bae0 100644 --- a/test/setup_aot/TestSetupAOT.java +++ b/test/setup_aot/TestSetupAOT.java @@ -23,7 +23,10 @@ * questions. */ +import java.io.ByteArrayOutputStream; import java.io.File; +import java.io.PrintStream; +import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.List; import java.util.Locale; @@ -60,6 +63,31 @@ public static void main(String[] args) throws Throwable { LOGGER.log(Level.FINE, "Done"); } + static class ToolOutput { + ByteArrayOutputStream baos; + PrintStream ps; + String output; + + ToolOutput() throws Exception { + baos = new ByteArrayOutputStream(); + ps = new PrintStream(baos, true, StandardCharsets.UTF_8.name()); + } + void finish() throws Exception { + output = baos.toString(StandardCharsets.UTF_8.name()); + System.out.println(output); + } + + ToolOutput shouldContain(String... substrings) { + for (String s : substrings) { + if (!output.contains(s)) { + throw new RuntimeException("\"" + s + "\" missing from tool output"); + } + } + + return this; + } + } + static void runJDKTools(String[] args) throws Throwable { String tmpDir = args[0]; System.out.println("Working Directory = " + System.getProperty("user.dir")); @@ -68,26 +96,33 @@ static void runJDKTools(String[] args) throws Throwable { // ------------------------------ // javac - execTool("javac", "--help"); + execTool("javac", "--help") + .shouldContain("Usage: javac "); JavacBenchApp.main(new String[] {"5"}); // ------------------------------ // javap - execTool("javap", "--help"); + execTool("javap", "--help") + .shouldContain("Show package/protected/public classes"); execTool("javap", "-c", "-private", "-v", "-verify", "java.lang.System", "java/util/stream/IntStream", - "jdk.internal.module.ModuleBootstrap"); + "jdk.internal.module.ModuleBootstrap") + .shouldContain("Compiled from \"System.java\"", + "public static java.io.Console console()"); // ------------------------------ // jlink String jlinkOutput = tmpDir + File.separator + "jlinkOutput"; - execTool("jlink", "--help"); - execTool("jlink", "--list-plugins"); + execTool("jlink", "--help") + .shouldContain("Compression to use in compressing resources"); + execTool("jlink", "--list-plugins") + .shouldContain("List of available plugins", + "--generate-cds-archive "); deleteAll(jlinkOutput); execTool("jlink", "--add-modules", "java.base", "--strip-debug", "--output", jlinkOutput); @@ -98,20 +133,27 @@ static void runJDKTools(String[] args) throws Throwable { String jarOutput = tmpDir + File.separator + "tmp.jar"; - execTool("jar", "--help"); + execTool("jar", "--help") + .shouldContain("--main-class=CLASSNAME"); deleteAll(jarOutput); - execTool("jar", "cvf", jarOutput, "TestSetupAOT.class"); - execTool("jar", "uvf", jarOutput, "TestSetupAOT.class"); - execTool("jar", "tvf", jarOutput); - execTool("jar", "--describe-module", "--file=" + jarOutput); + execTool("jar", "cvf", jarOutput, "TestSetupAOT.class") + .shouldContain("adding: TestSetupAOT.class"); + execTool("jar", "uvf", jarOutput, "TestSetupAOT.class") + .shouldContain("adding: TestSetupAOT.class"); + execTool("jar", "tvf", jarOutput) + .shouldContain("META-INF/MANIFEST.MF"); + execTool("jar", "--describe-module", "--file=" + jarOutput) + .shouldContain("Unable to derive module descriptor for: ./tmp.jar"); deleteAll(jarOutput); // ------------------------------ // jdeps - execTool("jdeps", "--help"); - execTool("jdeps", "-v", "TestSetupAOT.class"); + execTool("jdeps", "--help") + .shouldContain("--ignore-missing-deps"); + execTool("jdeps", "-v", "TestSetupAOT.class") + .shouldContain("-> JavacBenchApp"); } static void deleteAll(String f) { @@ -129,7 +171,7 @@ static void deleteAll(File f) { f.delete(); } - static void execTool(String tool, String... args) throws Throwable { + static ToolOutput execTool(String tool, String... args) throws Throwable { System.out.println("== Running tool ======================================================"); System.out.print(tool); for (String s : args) { @@ -138,9 +180,13 @@ static void execTool(String tool, String... args) throws Throwable { System.out.println(); System.out.println("======================================================================"); + ToolOutput output = new ToolOutput(); ToolProvider t = ToolProvider.findFirst(tool) .orElseThrow(() -> new RuntimeException(tool + " not found")); - t.run(System.out, System.out, args); + t.run(output.ps, output.ps, args); + + output.finish(); + return output; } From 2d82e3677dc7e9451fd0ea94c5330dd7fec74743 Mon Sep 17 00:00:00 2001 From: iklam Date: Wed, 23 Apr 2025 21:48:37 -0700 Subject: [PATCH 03/16] Avoid the need to Universe::reference_pending_list() and remove assumptions on GC behavior. Suggested by @fisk --- src/hotspot/share/cds/archiveHeapWriter.cpp | 18 ++++-- src/hotspot/share/cds/heapShared.cpp | 41 +++++++----- src/hotspot/share/cds/heapShared.hpp | 4 +- src/hotspot/share/cds/metaspaceShared.cpp | 62 +++++-------------- src/hotspot/share/cds/metaspaceShared.hpp | 1 - .../classes/java/lang/invoke/MethodType.java | 6 ++ .../classes/java/lang/ref/Reference.java | 7 --- .../share/classes/jdk/internal/misc/CDS.java | 15 +++++ .../jdk/internal/util/ReferencedKeyMap.java | 35 +++++++++++ .../jdk/internal/util/ReferencedKeySet.java | 4 ++ 10 files changed, 117 insertions(+), 76 deletions(-) diff --git a/src/hotspot/share/cds/archiveHeapWriter.cpp b/src/hotspot/share/cds/archiveHeapWriter.cpp index 5684066105f1f..2b6d403d83528 100644 --- a/src/hotspot/share/cds/archiveHeapWriter.cpp +++ b/src/hotspot/share/cds/archiveHeapWriter.cpp @@ -22,6 +22,7 @@ * */ +#include "cds/aotReferenceObjSupport.hpp" #include "cds/archiveHeapWriter.hpp" #include "cds/cdsConfig.hpp" #include "cds/filemap.hpp" @@ -607,18 +608,27 @@ class ArchiveHeapWriter::EmbeddedOopRelocator: public BasicOopIterateClosure { oop _src_obj; address _buffered_obj; CHeapBitMap* _oopmap; - + bool _is_java_lang_ref; public: EmbeddedOopRelocator(oop src_obj, address buffered_obj, CHeapBitMap* oopmap) : - _src_obj(src_obj), _buffered_obj(buffered_obj), _oopmap(oopmap) {} + _src_obj(src_obj), _buffered_obj(buffered_obj), _oopmap(oopmap) + { + _is_java_lang_ref = AOTReferenceObjSupport::check_if_ref_obj(src_obj); + } void do_oop(narrowOop *p) { EmbeddedOopRelocator::do_oop_work(p); } void do_oop( oop *p) { EmbeddedOopRelocator::do_oop_work(p); } private: template void do_oop_work(T *p) { - size_t field_offset = pointer_delta(p, _src_obj, sizeof(char)); - ArchiveHeapWriter::relocate_field_in_buffer((T*)(_buffered_obj + field_offset), _oopmap); + int field_offset = pointer_delta_as_int((char*)p, cast_from_oop(_src_obj)); + T* field_addr = (T*)(_buffered_obj + field_offset); + if (_is_java_lang_ref && AOTReferenceObjSupport::skip_field(field_offset)) { + // Do not copy these fields. Set them to null + *field_addr = (T)0x0; + } else { + ArchiveHeapWriter::relocate_field_in_buffer(field_addr, _oopmap); + } } }; diff --git a/src/hotspot/share/cds/heapShared.cpp b/src/hotspot/share/cds/heapShared.cpp index ce98b2b93b772..95b4d3102f442 100644 --- a/src/hotspot/share/cds/heapShared.cpp +++ b/src/hotspot/share/cds/heapShared.cpp @@ -25,6 +25,7 @@ #include "cds/aotArtifactFinder.hpp" #include "cds/aotClassInitializer.hpp" #include "cds/aotClassLocation.hpp" +#include "cds/aotReferenceObjSupport.hpp" #include "cds/archiveBuilder.hpp" #include "cds/archiveHeapLoader.hpp" #include "cds/archiveHeapWriter.hpp" @@ -1363,34 +1364,37 @@ void HeapShared::clear_archived_roots_of(Klass* k) { } } -// Push all oops that are referenced by _referencing_obj onto the _stack. -class HeapShared::ReferentPusher: public BasicOopIterateClosure { +// Push all oop fields (or oop array elemenets in case of an objArray) in +// _referencing_obj onto the _stack. +class HeapShared::OopFieldPusher: public BasicOopIterateClosure { PendingOopStack* _stack; GrowableArray _found_oop_fields; int _level; bool _record_klasses_only; KlassSubGraphInfo* _subgraph_info; oop _referencing_obj; + bool _is_java_lang_ref; public: - ReferentPusher(PendingOopStack* stack, - int level, - bool record_klasses_only, - KlassSubGraphInfo* subgraph_info, - oop orig) : + OopFieldPusher(PendingOopStack* stack, + int level, + bool record_klasses_only, + KlassSubGraphInfo* subgraph_info, + oop orig) : _stack(stack), _found_oop_fields(), _level(level), _record_klasses_only(record_klasses_only), _subgraph_info(subgraph_info), _referencing_obj(orig) { + _is_java_lang_ref = AOTReferenceObjSupport::check_if_ref_obj(orig); } - void do_oop(narrowOop *p) { ReferentPusher::do_oop_work(p); } - void do_oop( oop *p) { ReferentPusher::do_oop_work(p); } + void do_oop(narrowOop *p) { OopFieldPusher::do_oop_work(p); } + void do_oop( oop *p) { OopFieldPusher::do_oop_work(p); } - ~ReferentPusher() { + ~OopFieldPusher() { while (_found_oop_fields.length() > 0) { // This produces the exact same traversal order as the previous version - // of ReferentPusher that recurses on the C stack -- a depth-first search, + // of OopFieldPusher that recurses on the C stack -- a depth-first search, // walking the oop fields in _referencing_obj by ascending field offsets. oop obj = _found_oop_fields.pop(); _stack->push(PendingOop(obj, _referencing_obj, _level + 1)); @@ -1401,12 +1405,17 @@ class HeapShared::ReferentPusher: public BasicOopIterateClosure { template void do_oop_work(T *p) { oop obj = RawAccess<>::oop_load(p); if (!CompressedOops::is_null(obj)) { - size_t field_delta = pointer_delta(p, _referencing_obj, sizeof(char)); + int field_offset = pointer_delta_as_int((char*)p, cast_from_oop(_referencing_obj)); + + if (_is_java_lang_ref && AOTReferenceObjSupport::skip_field(field_offset)) { + // Do not follow these fields. They will be cleared to null. + return; + } if (!_record_klasses_only && log_is_enabled(Debug, cds, heap)) { ResourceMark rm; - log_debug(cds, heap)("(%d) %s[%zu] ==> " PTR_FORMAT " size %zu %s", _level, - _referencing_obj->klass()->external_name(), field_delta, + log_debug(cds, heap)("(%d) %s[%d] ==> " PTR_FORMAT " size %zu %s", _level, + _referencing_obj->klass()->external_name(), field_offset, p2i(obj), obj->size() * HeapWordSize, obj->klass()->external_name()); if (log_is_enabled(Trace, cds, heap)) { LogTarget(Trace, cds, heap) log; @@ -1586,7 +1595,7 @@ bool HeapShared::walk_one_object(PendingOopStack* stack, int level, KlassSubGrap // Find all the oops that are referenced by orig_obj, push them onto the stack // so we can work on them next. ResourceMark rm; - ReferentPusher pusher(stack, level, record_klasses_only, subgraph_info, orig_obj); + OopFieldPusher pusher(stack, level, record_klasses_only, subgraph_info, orig_obj); orig_obj->oop_iterate(&pusher); } @@ -1613,7 +1622,7 @@ bool HeapShared::walk_one_object(PendingOopStack* stack, int level, KlassSubGrap // - No java.lang.Class instance (java mirror) can be included inside // an archived sub-graph. Mirror can only be the sub-graph entry object. // -// The Java heap object sub-graph archiving process (see ReferentPusher): +// The Java heap object sub-graph archiving process (see OopFieldPusher): // // 1) Java object sub-graph archiving starts from a given static field // within a Class instance (java mirror). If the static field is a diff --git a/src/hotspot/share/cds/heapShared.hpp b/src/hotspot/share/cds/heapShared.hpp index 04c9ae9138123..f4e86aa5895a3 100644 --- a/src/hotspot/share/cds/heapShared.hpp +++ b/src/hotspot/share/cds/heapShared.hpp @@ -164,8 +164,8 @@ class HeapShared: AllStatic { static void count_allocation(size_t size); static void print_stats(); - static void debug_trace(); public: + static void debug_trace(); static unsigned oop_hash(oop const& p); static unsigned string_oop_hash(oop const& string) { return java_lang_String::hash_code(string); @@ -357,7 +357,7 @@ class HeapShared: AllStatic { int level() const { return _level; } }; - class ReferentPusher; + class OopFieldPusher; using PendingOopStack = GrowableArrayCHeap; static PendingOop _object_being_archived; diff --git a/src/hotspot/share/cds/metaspaceShared.cpp b/src/hotspot/share/cds/metaspaceShared.cpp index 801c1cbf93f72..91d3087ef83a1 100644 --- a/src/hotspot/share/cds/metaspaceShared.cpp +++ b/src/hotspot/share/cds/metaspaceShared.cpp @@ -28,6 +28,7 @@ #include "cds/aotClassLocation.hpp" #include "cds/aotConstantPoolResolver.hpp" #include "cds/aotLinkedClassBulkLoader.hpp" +#include "cds/aotReferenceObjSupport.hpp" #include "cds/archiveBuilder.hpp" #include "cds/archiveHeapLoader.hpp" #include "cds/archiveHeapWriter.hpp" @@ -902,51 +903,6 @@ void MetaspaceShared::exercise_runtime_cds_code(TRAPS) { CDSProtectionDomain::to_file_URL("dummy.jar", Handle(), CHECK); } -#if INCLUDE_CDS_JAVA_HEAP -void MetaspaceShared::process_pending_references(TRAPS) { - precond(CDSConfig::is_dumping_heap()); - precond(CDSConfig::allow_only_single_java_thread()); - - // When dumping heap objects, the JVM runs a single Java thread, so the ReferenceHandler - // thread is not running. We need to ensure that all objects in Universe::reference_pending_list() - // are processed. Otherwise we might pull in unwanted objects into the archived heap. Example: - // - // class UnwantedWeakRef extends WeakReference { Object junk = ...; } - // WeakReference A = ...; - // UnwantedWeakRef B = ...; - // - // If the referent of both A and B are collected, A and B will be placed in - // the Universe::reference_pending_list() by the GC, which A.next == B. - // - // When HeapShared::archiveObject() find A, it will find B from A.next and end up - // dumping the object B.junk that we don't want. - // - // After ReferenceHandler::processPendingReferences0() completes, A.next will be - // cleared and we won't be able to find B anymore. - - Symbol* klass_name = SymbolTable::new_symbol("java/lang/ref/Reference$ReferenceHandler"); - Klass* k = SystemDictionary::resolve_or_fail(klass_name, true, CHECK); - Symbol* method_name = SymbolTable::new_symbol("processPendingReferences0"); - Symbol* method_sig = vmSymbols::void_method_signature(); - - while (true) { - { - MonitorLocker ml(Heap_lock); - if (!Universe::has_reference_pending_list()) { - break; - } - } - - JavaValue result(T_VOID); - JavaCalls::call_static(&result, k, method_name, method_sig, CHECK); - - // In case ReferenceHandler::processPendingReferences0() creates garbage and causes - // more refs to be added to Universe::reference_pending_list() - Universe::heap()->collect(GCCause::_java_lang_system_gc); - } -} -#endif - void MetaspaceShared::preload_and_dump_impl(StaticArchiveBuilder& builder, TRAPS) { if (CDSConfig::is_dumping_classic_static_archive()) { // We are running with -Xshare:dump @@ -1007,11 +963,26 @@ void MetaspaceShared::preload_and_dump_impl(StaticArchiveBuilder& builder, TRAPS #if INCLUDE_CDS_JAVA_HEAP if (CDSConfig::is_dumping_heap()) { ArchiveHeapWriter::init(); + AOTReferenceObjSupport::initialize(CHECK); if (CDSConfig::is_dumping_full_module_graph()) { ClassLoaderDataShared::ensure_module_entry_tables_exist(); HeapShared::reset_archived_object_states(CHECK); } + if (CDSConfig::is_dumping_method_handles()) { + // This assert means that the MethodType and MethodTypeForm tables won't be + // updated concurrently, so we can remove GC'ed entries ... + assert(CDSConfig::allow_only_single_java_thread(), "Required"); + + TempNewSymbol method_name = SymbolTable::new_symbol("prepareForAOTCache"); + JavaValue result(T_VOID); + JavaCalls::call_static(&result, vmClasses::MethodType_klass(), + method_name, + 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. @@ -1029,7 +1000,6 @@ void MetaspaceShared::preload_and_dump_impl(StaticArchiveBuilder& builder, TRAPS // Do this at the very end, when no Java code will be executed. Otherwise // some new strings may be added to the intern table. StringTable::allocate_shared_strings_array(CHECK); - process_pending_references(CHECK); } else { log_info(cds)("Not dumping heap, reset CDSConfig::_is_using_optimized_module_handling"); CDSConfig::stop_using_optimized_module_handling(); diff --git a/src/hotspot/share/cds/metaspaceShared.hpp b/src/hotspot/share/cds/metaspaceShared.hpp index ea56132b381db..e03994be1b993 100644 --- a/src/hotspot/share/cds/metaspaceShared.hpp +++ b/src/hotspot/share/cds/metaspaceShared.hpp @@ -195,6 +195,5 @@ class MetaspaceShared : AllStatic { static MapArchiveResult map_archive(FileMapInfo* mapinfo, char* mapped_base_address, ReservedSpace rs); static void unmap_archive(FileMapInfo* mapinfo); static void get_default_classlist(char* default_classlist, const size_t buf_size); - static void process_pending_references(TRAPS); }; #endif // SHARE_CDS_METASPACESHARED_HPP diff --git a/src/java.base/share/classes/java/lang/invoke/MethodType.java b/src/java.base/share/classes/java/lang/invoke/MethodType.java index 704d4994291d2..2455c667ed221 100644 --- a/src/java.base/share/classes/java/lang/invoke/MethodType.java +++ b/src/java.base/share/classes/java/lang/invoke/MethodType.java @@ -1381,4 +1381,10 @@ private Object readResolve() { wrapAlt = null; return mt; } + + // This is called from C code, at the very end of Java code execution + // during the AOT cache assembly phase. + private static void prepareForAOTCache() { + internTable.prepareForAOTCache(); + } } diff --git a/src/java.base/share/classes/java/lang/ref/Reference.java b/src/java.base/share/classes/java/lang/ref/Reference.java index b62efb47f8aa8..fb177157089bf 100644 --- a/src/java.base/share/classes/java/lang/ref/Reference.java +++ b/src/java.base/share/classes/java/lang/ref/Reference.java @@ -208,13 +208,6 @@ public void run() { processPendingReferences(); } } - - // Called by the JVM during AOT cache creation (which does not - // launch the ReferenceHandler thread). - static void processPendingReferences0() { - Unsafe.getUnsafe().ensureClassInitialized(Cleaner.class); - processPendingReferences(); - } } /* diff --git a/src/java.base/share/classes/jdk/internal/misc/CDS.java b/src/java.base/share/classes/jdk/internal/misc/CDS.java index f070738e0c767..07a757ed0c146 100644 --- a/src/java.base/share/classes/jdk/internal/misc/CDS.java +++ b/src/java.base/share/classes/jdk/internal/misc/CDS.java @@ -82,9 +82,24 @@ public static boolean isDumpingStaticArchive() { return (configStatus & IS_DUMPING_STATIC_ARCHIVE) != 0; } + public static boolean isSingleThreadVM() { + return isDumpingStaticArchive(); + } + private static native int getCDSConfigStatus(); private static native void logLambdaFormInvoker(String line); + + private static ArrayList keepAliveList; + + public static void keepAlive(Object s) { + assert isSingleThreadVM(); + if (keepAliveList == null) { + keepAliveList = new ArrayList<>(); + } + keepAliveList.add(s); + } + /** * Initialize archived static fields in the given Class using archived * values from CDS dump time. Also initialize the classes of objects in diff --git a/src/java.base/share/classes/jdk/internal/util/ReferencedKeyMap.java b/src/java.base/share/classes/jdk/internal/util/ReferencedKeyMap.java index 766aca20b2167..7b6fc75a74746 100644 --- a/src/java.base/share/classes/jdk/internal/util/ReferencedKeyMap.java +++ b/src/java.base/share/classes/jdk/internal/util/ReferencedKeyMap.java @@ -42,6 +42,7 @@ import java.util.stream.Stream; import jdk.internal.access.SharedSecrets; +import jdk.internal.misc.CDS; /** * This class provides management of {@link Map maps} where it is desirable to @@ -336,6 +337,40 @@ public void removeStaleReferences() { } } + @SuppressWarnings("unchecked") + public void prepareForAOTCache() { + // We are running the AOT assembly phase. The JVM has a single Java thread, so + // we don't have any concurrent threads that may modify the map while we are + // iterating its keys. + // + // Also, the java.lang.ref.Reference$ReferenceHandler thread is not running, + // so even if the GC has put some of the keys on the pending ReferencePendingList, + // none of the keys would have been added to the stale queue yet. + assert CDS.isSingleThreadVM(); + + for (ReferenceKey key : map.keySet()) { + Object referent = key.get(); + if (referent == null) { + // We don't need this key anymore. Add to stale queue + ((Reference)key).enqueue(); + } else { + // Make sure the referent cannot be collected. Otherwise, when + // the referent is collected, the GC may push the key onto + // Universe::reference_pending_list() at an unpredictable time, + // making it difficult to correctly serialize the key's + // state into the CDS archive. + // + // See aotReferenceObjSupport.cpp for more info. + CDS.keepAlive(referent); + } + Reference.reachabilityFence(referent); + } + + // Remove all keys enqueued above + removeStaleReferences(); + } + + /** * Puts an entry where the key and the value are the same. Used for * interning values in a set. diff --git a/src/java.base/share/classes/jdk/internal/util/ReferencedKeySet.java b/src/java.base/share/classes/jdk/internal/util/ReferencedKeySet.java index 355af10aa7c31..a22227a11c4de 100644 --- a/src/java.base/share/classes/jdk/internal/util/ReferencedKeySet.java +++ b/src/java.base/share/classes/jdk/internal/util/ReferencedKeySet.java @@ -193,4 +193,8 @@ public T intern(T e) { public T intern(T e, UnaryOperator interner) { return ReferencedKeyMap.intern(map, e, interner); } + + public void prepareForAOTCache() { + map.prepareForAOTCache(); + } } From dc5ad267814c499b72f93f76d8f4a2bf7153c894 Mon Sep 17 00:00:00 2001 From: iklam Date: Wed, 23 Apr 2025 21:54:10 -0700 Subject: [PATCH 04/16] Added missing files --- .../share/cds/aotReferenceObjSupport.cpp | 148 ++++++++++++++++++ .../share/cds/aotReferenceObjSupport.hpp | 42 +++++ 2 files changed, 190 insertions(+) create mode 100644 src/hotspot/share/cds/aotReferenceObjSupport.cpp create mode 100644 src/hotspot/share/cds/aotReferenceObjSupport.hpp diff --git a/src/hotspot/share/cds/aotReferenceObjSupport.cpp b/src/hotspot/share/cds/aotReferenceObjSupport.cpp new file mode 100644 index 0000000000000..be417a7242b01 --- /dev/null +++ b/src/hotspot/share/cds/aotReferenceObjSupport.cpp @@ -0,0 +1,148 @@ +/* + * Copyright (c) 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/aotReferenceObjSupport.hpp" +#include "cds/heapShared.hpp" +#include "classfile/javaClasses.hpp" +#include "classfile/symbolTable.hpp" +#include "classfile/systemDictionary.hpp" +#include "classfile/vmSymbols.hpp" +#include "logging/log.hpp" +#include "memory/resourceArea.hpp" +#include "memory/universe.hpp" +#include "oops/oop.inline.hpp" +#include "oops/oopHandle.inline.hpp" +#include "runtime/fieldDescriptor.inline.hpp" + +// Handling of java.lang.ref.Reference objects in the AOT cache +// ============================================================ +// +// When AOTArtifactFinder finds an oop which is a instance of java.lang.ref.Reference: +// +// - We check if the oop is eligible to be stored in the AOT cache. If not, the AOT cache +// creation fails -- see AOTReferenceObjSupport::check_if_ref_obj() +// +// - Otherwise, we store the oop into the AOT cache, but we unconditionally reset its +// "next" and "discovered" fields to null. Otherwise, if AOTArtifactFinder follows these +// fields, it may found unrelated objects that we don't intent to cache. +// +// Eligibility +// =========== +// +// [1] A reference that does not require special clean up (i.e., Reference::queue == ReferenceQueue::NULL) +// is eligible. +// +// [2] A reference that REQUIRE specials clean up (i.e., Reference::queue != ReferenceQueue::NULL) +// is eligible ONLY if it has not been put into the "pending" state by the GC (See Reference.java). +// +// AOTReferenceObjSupport::check_if_ref_obj() detects the "pending" state by checking the "next" and +// "discovered" fields of the oop. +// +// As of this version, the only oops in group [2] that can be found by AOTArtifactFinder are +// the keys used by ReferencedKeyMap in the implementation of MethodType::internTable. +// ReferencedKeyMap::prepareForAOTCache ensures that all keys found by AOTArtifactFinder are eligible. +// +// The purpose of the error check in check_if_ref_obj() is to guard against changes in the JDK core +// libs that might introduce new types of oops in group [2] into the AOT cache. +// +// +// Reasons for the eligibility restrictions +// ======================================== +// +// Reference handling is complex. In this version, we implement only enough functionality to support +// the use of Weak/Soft references used by java.lang.invoke. +// +// We intent to evolve the implementation in the future by +// -- implementing more prepareForAOTCache() operations for other use cases, and/or +// -- relaxing the eligibility restrictions. + +static OopHandle _null_queue; + +void AOTReferenceObjSupport::initialize(TRAPS) { + TempNewSymbol class_name = SymbolTable::new_symbol("java/lang/ref/ReferenceQueue"); + Klass* k = SystemDictionary::resolve_or_fail(class_name, true, CHECK); + InstanceKlass* ik = InstanceKlass::cast(k); + ik->initialize(CHECK); + + TempNewSymbol field_name = SymbolTable::new_symbol("NULL"); + fieldDescriptor fd; + bool found = ik->find_local_field(field_name, vmSymbols::referencequeue_signature(), &fd); + precond(found); + precond(fd.is_static()); + + _null_queue = OopHandle(Universe::vm_global(), ik->java_mirror()->obj_field(fd.offset())); +} + +bool AOTReferenceObjSupport::check_if_ref_obj(oop obj) { + // We have a single Java thread. This means java.lang.ref.Reference$ReferenceHandler thread + // is not running. Otherwise the checks for next/discovered may not work. + precond(CDSConfig::allow_only_single_java_thread()); + + if (obj->klass()->is_subclass_of(vmClasses::Reference_klass())) { + oop referent = obj->obj_field(java_lang_ref_Reference::referent_offset()); + oop queue = obj->obj_field(java_lang_ref_Reference::queue_offset()); + oop next = java_lang_ref_Reference::next(obj); + oop discovered = java_lang_ref_Reference::discovered(obj); + + if (next != nullptr || discovered != nullptr) { + if (queue != _null_queue.resolve()) { + ResourceMark rm; + log_error(cds, heap)("Cannot archive reference object " PTR_FORMAT " of class %s", + p2i(obj), obj->klass()->external_name()); + log_error(cds, heap)("referent = " PTR_FORMAT + ", queue = " PTR_FORMAT + ", next = " PTR_FORMAT + ", discovered = " PTR_FORMAT, + p2i(referent), p2i(queue), p2i(next), p2i(discovered)); + log_error(cds, heap)("This object requires special clean up as its queue is not ReferenceQueue::NULL (" + PTR_FORMAT ")", p2i(_null_queue.resolve())); + HeapShared::debug_trace(); + MetaspaceShared::unrecoverable_writing_error(); + } + } + + if (log_is_enabled(Info, cds, ref)) { + ResourceMark rm; + log_info(cds, ref)("Reference obj:" + " r=" PTR_FORMAT + " q=" PTR_FORMAT + " n=" PTR_FORMAT + " d=" PTR_FORMAT + " %s", + p2i(referent), + p2i(queue), + p2i(next), + p2i(discovered), + obj->klass()->external_name()); + } + return true; + } else { + return false; + } +} + +bool AOTReferenceObjSupport::skip_field(int field_offset) { + return (field_offset == java_lang_ref_Reference::next_offset() || + field_offset == java_lang_ref_Reference::discovered_offset()); +} diff --git a/src/hotspot/share/cds/aotReferenceObjSupport.hpp b/src/hotspot/share/cds/aotReferenceObjSupport.hpp new file mode 100644 index 0000000000000..7f14695ac07eb --- /dev/null +++ b/src/hotspot/share/cds/aotReferenceObjSupport.hpp @@ -0,0 +1,42 @@ +/* + * Copyright (c) 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_CDS_AOTREFERENCEOBJSUPPORT_HPP +#define SHARE_CDS_AOTREFERENCEOBJSUPPORT_HPP + +#include "memory/allStatic.hpp" +#include "oops/oopsHierarchy.hpp" +#include "utilities/exceptions.hpp" + +// Support for ahead-of-time allocated instances of java.lang.ref.Reference + +class AOTReferenceObjSupport : AllStatic { + +public: + static void initialize(TRAPS); + static bool check_if_ref_obj(oop obj); + static bool skip_field(int field_offset); +}; + +#endif // SHARE_CDS_AOTREFERENCEOBJSUPPORT_HPP From 2544cc7179f3792f13d160029765ad8be0d622a0 Mon Sep 17 00:00:00 2001 From: iklam Date: Thu, 24 Apr 2025 08:46:06 -0700 Subject: [PATCH 05/16] cleaned up test cases --- .../aotClassLinking/TestSetupAOTTest.java | 2 +- .../aotClassLinking/WeakReferenceTest.java | 97 +++++-------------- 2 files changed, 27 insertions(+), 72 deletions(-) diff --git a/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/TestSetupAOTTest.java b/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/TestSetupAOTTest.java index a9519a6ef769e..f88af3caed5b6 100644 --- a/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/TestSetupAOTTest.java +++ b/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/TestSetupAOTTest.java @@ -34,7 +34,7 @@ * @build TestSetupAOTTest JavacBenchApp TestSetupAOT * @run driver jdk.test.lib.helpers.ClassFileInstaller * TestSetupAOT - * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar app.jar + * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar app.jar * TestSetupAOT * TestSetupAOT$ToolOutput * JavacBenchApp diff --git a/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/WeakReferenceTest.java b/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/WeakReferenceTest.java index 2a7a8a69ffffc..a8fd876ee093d 100644 --- a/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/WeakReferenceTest.java +++ b/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/WeakReferenceTest.java @@ -101,31 +101,18 @@ static class Inner { static { Inner.WeakReferenceTestApp_clinit_executed = true; - // During the assembly phase, this block of code is called during the assembly - // phase (triggered by the -XX:AOTInitTestClass=WeakReferenceTestApp flag). - // It runs the clinit_for_testXXX() method to set up the aot-initialized data structures + // This static {} block is executed the training run (which uses no AOT cache). + // + // During the assembly phase, this static {} block of is also executed + // (triggered by the -XX:AOTInitTestClass=WeakReferenceTestApp flag). + // It runs the aot_init_for_testXXX() method to set up the aot-initialized data structures // that are used by each testXXX() function. // - // Note that this function is also called during the training run. - // This function is NOT called during the production run, because WeakReferenceTestApp + // This block is NOT executed during the production run, because WeakReferenceTestApp // is aot-initialized. - clinit_for_testCollectedInAssembly(); - clinit_for_testWeakReferenceCollection(); - clinit_for_testQueue(); - } - - static WeakReference makeRef() { - System.out.println("WeakReferenceTestApp::makeRef() is executed"); - WeakReference r = new WeakReference(root); - System.out.println("r.get() = " + r.get()); - - ShouldNotBeAOTInited.doit(); - return r; - } - - static WeakReference makeRef2() { - return new WeakReference(new WeakReferenceTestApp()); + aot_init_for_testCollectedInAssembly(); + aot_init_for_testWeakReferenceCollection(); } public static void main(String[] args) { @@ -156,14 +143,13 @@ static void runTests(String[] args) throws Exception { testCollectedInAssembly(isProduction); testWeakReferenceCollection(isProduction); - testQueue(isProduction); } //---------------------------------------------------------------------- // Set up for testCollectedInAssembly() static WeakReference refToCollectedObj; - static void clinit_for_testCollectedInAssembly() { + static void aot_init_for_testCollectedInAssembly() { // The referent will be GC-ed in the assembly run when the JVM forces a full GC. refToCollectedObj = new WeakReference(new String("collected in assembly")); } @@ -171,19 +157,10 @@ static void clinit_for_testCollectedInAssembly() { // [TEST CASE] Test the storage of a WeakReference whose referent has been collected during the assembly phase. static void testCollectedInAssembly(boolean isProduction) { System.out.println("refToCollectedObj.get() = " + refToCollectedObj.get()); - System.out.println("refToCollectedObj.isEnqueued() = " + refToCollectedObj.isEnqueued()); if (refToCollectedObj.get() != null) { throw new RuntimeException("refToCollectedObj.get() should have been GC'ed"); } - - /* - * FIXME -- why does this fail, even in training run? - - if (!refToCollectedObj.isEnqueued()) { - throw new RuntimeException("refToCollectedObj.isEnqueued() should be true"); - } - */ } //---------------------------------------------------------------------- @@ -191,11 +168,25 @@ static void testCollectedInAssembly(boolean isProduction) { static Object root; static WeakReference ref; - static void clinit_for_testWeakReferenceCollection() { - root = new WeakReferenceTestApp(); + static void aot_init_for_testWeakReferenceCollection() { + root = new String("to be collected in production"); ref = makeRef(); } + static WeakReference makeRef() { + System.out.println("WeakReferenceTestApp::makeRef() is executed"); + WeakReference r = new WeakReference(root); + System.out.println("r.get() = " + r.get()); + + ShouldNotBeAOTInited.doit(); + return r; + } + + static WeakReference makeRef2() { + return new WeakReference(new String("to be collected in production")); + } + + // [TEST CASE] A WeakReference allocated in assembly phase should be collectable in the production run static void testWeakReferenceCollection(boolean isProduction) { WeakReference ref2 = makeRef2(); @@ -210,7 +201,7 @@ static void testWeakReferenceCollection(boolean isProduction) { } System.out.println("... running GC ..."); - root = null; + root = null; // make ref.referent() eligible for collection System.gc(); System.out.println("ref.get() = " + ref.get()); @@ -228,42 +219,6 @@ static void testWeakReferenceCollection(boolean isProduction) { throw new RuntimeException("ShouldNotBeAOTInited should not have been aot-inited"); } } - - //---------------------------------------------------------------------- - // Set up for testQueue() - static WeakReference refWithQueue; - static SharedQueue sharedQueueInstance; - - static void clinit_for_testQueue() { - // Make sure SharedQueue is also cached in *initialized* state. - sharedQueueInstance = SharedQueue.sharedQueueInstance; - - refWithQueue = new WeakReference(String.class, SharedQueue.queue()); - ShouldNotBeArchived.ref = new WeakReference(ShouldNotBeArchived.instance, SharedQueue.queue()); - - - // Set to 2 in training run and assembly phase, but this state shouldn't be stored in - // AOT cache. - ShouldNotBeArchived.state = 2; - } - - // [TEST CASE] Unrelated WeakReferences shouldn't be cached even if they are registered with the same queue - static void testQueue(boolean isProduction) { - System.out.println("refWithQueue.get() = " + refWithQueue.get()); - System.out.println("ShouldNotBeArchived.state = " + ShouldNotBeArchived.state); - - // [1] Although refWithQueue and ShouldNotBeArchived.ref are registered with the same queue, as both - // of their referents are strongly referenced, they are not added to the queue's "head". - // (Per javadoc: "registered reference objects are appended by the garbage collector after the - // appropriate reachability changes are detected"); - // [2] When the assembly phase scans refWithQueue, it shouldn't discover ShouldNotBeArchived.ref (via the queue), - // so ShouldNotBeArchived.ref should not be stored in the AOT cache. - // [3] As a result, ShouldNotBeArchived should be cached in the *not initialized" state. Its - // will be executed in the production run to set ShouldNotBeArchived.state to 1. - if (isProduction && ShouldNotBeArchived.state != 1) { - throw new RuntimeException("ShouldNotBeArchived should be 1 but is " + ShouldNotBeArchived.state); - } - } } class ShouldNotBeAOTInited { From 189839999dc1a66eb50b566c275630ff20d9379b Mon Sep 17 00:00:00 2001 From: iklam Date: Thu, 24 Apr 2025 18:08:09 -0700 Subject: [PATCH 06/16] Work around hotspot/jtreg/sources/TestNoNULL.java --- src/hotspot/share/cds/aotReferenceObjSupport.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/hotspot/share/cds/aotReferenceObjSupport.cpp b/src/hotspot/share/cds/aotReferenceObjSupport.cpp index be417a7242b01..c4a06a56937b5 100644 --- a/src/hotspot/share/cds/aotReferenceObjSupport.cpp +++ b/src/hotspot/share/cds/aotReferenceObjSupport.cpp @@ -50,10 +50,10 @@ // Eligibility // =========== // -// [1] A reference that does not require special clean up (i.e., Reference::queue == ReferenceQueue::NULL) +// [1] A reference that does not require special clean up (i.e., Reference::queue == _null_queue.resolve()) // is eligible. // -// [2] A reference that REQUIRE specials clean up (i.e., Reference::queue != ReferenceQueue::NULL) +// [2] A reference that REQUIRE specials clean up (i.e., Reference::queue != _null_queue.resolve()) // is eligible ONLY if it has not been put into the "pending" state by the GC (See Reference.java). // // AOTReferenceObjSupport::check_if_ref_obj() detects the "pending" state by checking the "next" and @@ -85,7 +85,7 @@ void AOTReferenceObjSupport::initialize(TRAPS) { InstanceKlass* ik = InstanceKlass::cast(k); ik->initialize(CHECK); - TempNewSymbol field_name = SymbolTable::new_symbol("NULL"); + TempNewSymbol field_name = SymbolTable::new_symbol("N""ULL"); fieldDescriptor fd; bool found = ik->find_local_field(field_name, vmSymbols::referencequeue_signature(), &fd); precond(found); @@ -115,7 +115,7 @@ bool AOTReferenceObjSupport::check_if_ref_obj(oop obj) { ", next = " PTR_FORMAT ", discovered = " PTR_FORMAT, p2i(referent), p2i(queue), p2i(next), p2i(discovered)); - log_error(cds, heap)("This object requires special clean up as its queue is not ReferenceQueue::NULL (" + log_error(cds, heap)("This object requires special clean up as its queue is not ReferenceQueue::N" "ULL (" PTR_FORMAT ")", p2i(_null_queue.resolve())); HeapShared::debug_trace(); MetaspaceShared::unrecoverable_writing_error(); From 73786f2b9135956e08e13aadba89f48a10c745e4 Mon Sep 17 00:00:00 2001 From: iklam Date: Thu, 24 Apr 2025 18:38:00 -0700 Subject: [PATCH 07/16] Fixed error when running "make test JTREG=AOT_JDK=true TEST=open/test/hotspot/jtreg/runtime/invokedynamic" --- test/setup_aot/TestSetupAOT.java | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/test/setup_aot/TestSetupAOT.java b/test/setup_aot/TestSetupAOT.java index 13e269692bae0..7d120ea8d1b10 100644 --- a/test/setup_aot/TestSetupAOT.java +++ b/test/setup_aot/TestSetupAOT.java @@ -32,6 +32,8 @@ import java.util.Locale; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.spi.ToolProvider; import java.util.stream.Stream; import static java.util.stream.Collectors.*; @@ -86,6 +88,17 @@ ToolOutput shouldContain(String... substrings) { return this; } + + ToolOutput shouldMatch(String... regexps) { + for (String regexp : regexps) { + Pattern pattern = Pattern.compile(regexp, Pattern.MULTILINE); + if (!pattern.matcher(output).find()) { + throw new RuntimeException("Pattern \"" + regexp + "\" missing from tool output"); + } + } + + return this; + } } static void runJDKTools(String[] args) throws Throwable { @@ -144,7 +157,7 @@ static void runJDKTools(String[] args) throws Throwable { execTool("jar", "tvf", jarOutput) .shouldContain("META-INF/MANIFEST.MF"); execTool("jar", "--describe-module", "--file=" + jarOutput) - .shouldContain("Unable to derive module descriptor for: ./tmp.jar"); + .shouldMatch("Unable to derive module descriptor for: .*/tmp.jar"); deleteAll(jarOutput); // ------------------------------ From 2c31ca7c9633ee164c5567d016b663b88740f6d7 Mon Sep 17 00:00:00 2001 From: iklam Date: Thu, 24 Apr 2025 18:46:34 -0700 Subject: [PATCH 08/16] Refactored code --- .../share/cds/aotReferenceObjSupport.cpp | 24 +++++++++++++++++-- .../share/cds/aotReferenceObjSupport.hpp | 1 + src/hotspot/share/cds/metaspaceShared.cpp | 17 +++---------- 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/src/hotspot/share/cds/aotReferenceObjSupport.cpp b/src/hotspot/share/cds/aotReferenceObjSupport.cpp index c4a06a56937b5..3edda9520bcaa 100644 --- a/src/hotspot/share/cds/aotReferenceObjSupport.cpp +++ b/src/hotspot/share/cds/aotReferenceObjSupport.cpp @@ -34,6 +34,7 @@ #include "oops/oop.inline.hpp" #include "oops/oopHandle.inline.hpp" #include "runtime/fieldDescriptor.inline.hpp" +#include "runtime/javaCalls.hpp" // Handling of java.lang.ref.Reference objects in the AOT cache // ============================================================ @@ -61,12 +62,11 @@ // // As of this version, the only oops in group [2] that can be found by AOTArtifactFinder are // the keys used by ReferencedKeyMap in the implementation of MethodType::internTable. -// ReferencedKeyMap::prepareForAOTCache ensures that all keys found by AOTArtifactFinder are eligible. +// stabilize_cached_reference_objects() ensures that all keys found by AOTArtifactFinder are eligible. // // The purpose of the error check in check_if_ref_obj() is to guard against changes in the JDK core // libs that might introduce new types of oops in group [2] into the AOT cache. // -// // Reasons for the eligibility restrictions // ======================================== // @@ -77,6 +77,8 @@ // -- implementing more prepareForAOTCache() operations for other use cases, and/or // -- relaxing the eligibility restrictions. +#if INCLUDE_CDS_JAVA_HEAP + static OopHandle _null_queue; void AOTReferenceObjSupport::initialize(TRAPS) { @@ -94,6 +96,22 @@ void AOTReferenceObjSupport::initialize(TRAPS) { _null_queue = OopHandle(Universe::vm_global(), ik->java_mirror()->obj_field(fd.offset())); } +// Ensure that all group [2] references found by AOTArtifactFinder are eligible. +void AOTReferenceObjSupport::stabilize_cached_reference_objects(TRAPS) { + if (CDSConfig::is_dumping_method_handles()) { + // This assert means that the MethodType and MethodTypeForm tables won't be + // updated concurrently, so we can remove GC'ed entries ... + assert(CDSConfig::allow_only_single_java_thread(), "Required"); + + TempNewSymbol method_name = SymbolTable::new_symbol("prepareForAOTCache"); + JavaValue result(T_VOID); + JavaCalls::call_static(&result, vmClasses::MethodType_klass(), + method_name, + vmSymbols::void_method_signature(), + CHECK); + } +} + bool AOTReferenceObjSupport::check_if_ref_obj(oop obj) { // We have a single Java thread. This means java.lang.ref.Reference$ReferenceHandler thread // is not running. Otherwise the checks for next/discovered may not work. @@ -146,3 +164,5 @@ bool AOTReferenceObjSupport::skip_field(int field_offset) { return (field_offset == java_lang_ref_Reference::next_offset() || field_offset == java_lang_ref_Reference::discovered_offset()); } + +#endif // INCLUDE_CDS_JAVA_HEAP diff --git a/src/hotspot/share/cds/aotReferenceObjSupport.hpp b/src/hotspot/share/cds/aotReferenceObjSupport.hpp index 7f14695ac07eb..bd3fbe2d18075 100644 --- a/src/hotspot/share/cds/aotReferenceObjSupport.hpp +++ b/src/hotspot/share/cds/aotReferenceObjSupport.hpp @@ -35,6 +35,7 @@ class AOTReferenceObjSupport : AllStatic { public: static void initialize(TRAPS); + static void stabilize_cached_reference_objects(TRAPS); static bool check_if_ref_obj(oop obj); static bool skip_field(int field_offset); }; diff --git a/src/hotspot/share/cds/metaspaceShared.cpp b/src/hotspot/share/cds/metaspaceShared.cpp index 91d3087ef83a1..0b722913f034f 100644 --- a/src/hotspot/share/cds/metaspaceShared.cpp +++ b/src/hotspot/share/cds/metaspaceShared.cpp @@ -963,25 +963,14 @@ void MetaspaceShared::preload_and_dump_impl(StaticArchiveBuilder& builder, TRAPS #if INCLUDE_CDS_JAVA_HEAP if (CDSConfig::is_dumping_heap()) { ArchiveHeapWriter::init(); - AOTReferenceObjSupport::initialize(CHECK); + if (CDSConfig::is_dumping_full_module_graph()) { ClassLoaderDataShared::ensure_module_entry_tables_exist(); HeapShared::reset_archived_object_states(CHECK); } - if (CDSConfig::is_dumping_method_handles()) { - // This assert means that the MethodType and MethodTypeForm tables won't be - // updated concurrently, so we can remove GC'ed entries ... - assert(CDSConfig::allow_only_single_java_thread(), "Required"); - - TempNewSymbol method_name = SymbolTable::new_symbol("prepareForAOTCache"); - JavaValue result(T_VOID); - JavaCalls::call_static(&result, vmClasses::MethodType_klass(), - method_name, - vmSymbols::void_method_signature(), - CHECK); - } - + AOTReferenceObjSupport::initialize(CHECK); + AOTReferenceObjSupport::stabilize_cached_reference_objects(CHECK); if (CDSConfig::is_initing_classes_at_dump_time()) { // java.lang.Class::reflectionFactory cannot be archived yet. We set this field From 643eec2af2ddf2dd80a245b2f67e7a3c8149424e Mon Sep 17 00:00:00 2001 From: iklam Date: Thu, 24 Apr 2025 21:05:29 -0700 Subject: [PATCH 09/16] Fixed windows path issues --- test/setup_aot/TestSetupAOT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/setup_aot/TestSetupAOT.java b/test/setup_aot/TestSetupAOT.java index 7d120ea8d1b10..e078279e716ce 100644 --- a/test/setup_aot/TestSetupAOT.java +++ b/test/setup_aot/TestSetupAOT.java @@ -157,7 +157,7 @@ static void runJDKTools(String[] args) throws Throwable { execTool("jar", "tvf", jarOutput) .shouldContain("META-INF/MANIFEST.MF"); execTool("jar", "--describe-module", "--file=" + jarOutput) - .shouldMatch("Unable to derive module descriptor for: .*/tmp.jar"); + .shouldMatch("Unable to derive module descriptor for: .*tmp.jar"); deleteAll(jarOutput); // ------------------------------ From bba16eef8287cd9ff04818d23f057137c7c5f0fa Mon Sep 17 00:00:00 2001 From: iklam Date: Thu, 24 Apr 2025 22:40:32 -0700 Subject: [PATCH 10/16] @fisk comment --- src/hotspot/share/cds/heapShared.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/hotspot/share/cds/heapShared.cpp b/src/hotspot/share/cds/heapShared.cpp index 95b4d3102f442..ef71d5895f6b8 100644 --- a/src/hotspot/share/cds/heapShared.cpp +++ b/src/hotspot/share/cds/heapShared.cpp @@ -1403,10 +1403,9 @@ class HeapShared::OopFieldPusher: public BasicOopIterateClosure { protected: template void do_oop_work(T *p) { - oop obj = RawAccess<>::oop_load(p); + int field_offset = pointer_delta_as_int((char*)p, cast_from_oop(_referencing_obj)); + oop obj = HeapAccess::oop_load_at(_referencing_obj, field_offset); if (!CompressedOops::is_null(obj)) { - int field_offset = pointer_delta_as_int((char*)p, cast_from_oop(_referencing_obj)); - if (_is_java_lang_ref && AOTReferenceObjSupport::skip_field(field_offset)) { // Do not follow these fields. They will be cleared to null. return; From a25aab03bec23b1332067d8ecbda0590ddfdb077 Mon Sep 17 00:00:00 2001 From: iklam Date: Sun, 27 Apr 2025 22:12:13 -0700 Subject: [PATCH 11/16] @DanHeidinga comments --- src/hotspot/share/cds/aotReferenceObjSupport.cpp | 4 ++-- src/java.base/share/classes/jdk/internal/misc/CDS.java | 5 ++++- .../share/classes/jdk/internal/util/ReferencedKeyMap.java | 2 +- .../share/classes/jdk/internal/util/ReferencedKeySet.java | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/hotspot/share/cds/aotReferenceObjSupport.cpp b/src/hotspot/share/cds/aotReferenceObjSupport.cpp index 3edda9520bcaa..d1f0a24b7da0a 100644 --- a/src/hotspot/share/cds/aotReferenceObjSupport.cpp +++ b/src/hotspot/share/cds/aotReferenceObjSupport.cpp @@ -46,7 +46,7 @@ // // - Otherwise, we store the oop into the AOT cache, but we unconditionally reset its // "next" and "discovered" fields to null. Otherwise, if AOTArtifactFinder follows these -// fields, it may found unrelated objects that we don't intent to cache. +// fields, it may found unrelated objects that we don't intend to cache. // // Eligibility // =========== @@ -73,7 +73,7 @@ // Reference handling is complex. In this version, we implement only enough functionality to support // the use of Weak/Soft references used by java.lang.invoke. // -// We intent to evolve the implementation in the future by +// We intend to evolve the implementation in the future by // -- implementing more prepareForAOTCache() operations for other use cases, and/or // -- relaxing the eligibility restrictions. diff --git a/src/java.base/share/classes/jdk/internal/misc/CDS.java b/src/java.base/share/classes/jdk/internal/misc/CDS.java index 07a757ed0c146..bf2eb183d604f 100644 --- a/src/java.base/share/classes/jdk/internal/misc/CDS.java +++ b/src/java.base/share/classes/jdk/internal/misc/CDS.java @@ -90,10 +90,13 @@ public static boolean isSingleThreadVM() { private static native void logLambdaFormInvoker(String line); + // Used only when dumping static archive to keep weak references alive to + // ensure that Soft/Weak Reference objects can be reliably archived. private static ArrayList keepAliveList; public static void keepAlive(Object s) { - assert isSingleThreadVM(); + assert isSingleThreadVM(); // no need for synchronization + assert isDumpingStaticArchive(); if (keepAliveList == null) { keepAliveList = new ArrayList<>(); } diff --git a/src/java.base/share/classes/jdk/internal/util/ReferencedKeyMap.java b/src/java.base/share/classes/jdk/internal/util/ReferencedKeyMap.java index 7b6fc75a74746..150b9d752bbd9 100644 --- a/src/java.base/share/classes/jdk/internal/util/ReferencedKeyMap.java +++ b/src/java.base/share/classes/jdk/internal/util/ReferencedKeyMap.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2023, 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 diff --git a/src/java.base/share/classes/jdk/internal/util/ReferencedKeySet.java b/src/java.base/share/classes/jdk/internal/util/ReferencedKeySet.java index a22227a11c4de..960b63b12d37d 100644 --- a/src/java.base/share/classes/jdk/internal/util/ReferencedKeySet.java +++ b/src/java.base/share/classes/jdk/internal/util/ReferencedKeySet.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2023, 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 From 331acfaec3212bc74a9c01638d2336b6aa5aa623 Mon Sep 17 00:00:00 2001 From: iklam Date: Tue, 29 Apr 2025 11:47:42 -0700 Subject: [PATCH 12/16] @fisk offline comments -- tighten up and simplify eligibility check; @DanHeidinga comment -- renamed to MethodType::assemblySetup() --- src/hotspot/share/cds/aotArtifactFinder.cpp | 2 + .../share/cds/aotReferenceObjSupport.cpp | 103 ++++++++++++++---- .../share/cds/aotReferenceObjSupport.hpp | 1 + .../classes/java/lang/invoke/MethodType.java | 2 +- .../share/classes/jdk/internal/misc/CDS.java | 9 ++ .../aotClassLinking/WeakReferenceTest.java | 73 +++++++++++-- 6 files changed, 159 insertions(+), 31 deletions(-) diff --git a/src/hotspot/share/cds/aotArtifactFinder.cpp b/src/hotspot/share/cds/aotArtifactFinder.cpp index 65eb06ca7f093..d87b501150b59 100644 --- a/src/hotspot/share/cds/aotArtifactFinder.cpp +++ b/src/hotspot/share/cds/aotArtifactFinder.cpp @@ -25,6 +25,7 @@ #include "cds/aotClassLinker.hpp" #include "cds/aotArtifactFinder.hpp" #include "cds/aotClassInitializer.hpp" +#include "cds/aotReferenceObjSupport.hpp" #include "cds/dumpTimeClassInfo.inline.hpp" #include "cds/heapShared.hpp" #include "cds/lambdaProxyClassDictionary.hpp" @@ -73,6 +74,7 @@ void AOTArtifactFinder::find_artifacts() { // Note, if a class is not excluded, it does NOT mean it will be automatically included // into the AOT cache -- that will be decided by the code below. SystemDictionaryShared::finish_exclusion_checks(); + AOTReferenceObjSupport::init_keep_alive_objs_table(); start_scanning_for_oops(); diff --git a/src/hotspot/share/cds/aotReferenceObjSupport.cpp b/src/hotspot/share/cds/aotReferenceObjSupport.cpp index d1f0a24b7da0a..938de6de149a7 100644 --- a/src/hotspot/share/cds/aotReferenceObjSupport.cpp +++ b/src/hotspot/share/cds/aotReferenceObjSupport.cpp @@ -35,6 +35,7 @@ #include "oops/oopHandle.inline.hpp" #include "runtime/fieldDescriptor.inline.hpp" #include "runtime/javaCalls.hpp" +#include "utilities/resourceHash.hpp" // Handling of java.lang.ref.Reference objects in the AOT cache // ============================================================ @@ -55,10 +56,7 @@ // is eligible. // // [2] A reference that REQUIRE specials clean up (i.e., Reference::queue != _null_queue.resolve()) -// is eligible ONLY if it has not been put into the "pending" state by the GC (See Reference.java). -// -// AOTReferenceObjSupport::check_if_ref_obj() detects the "pending" state by checking the "next" and -// "discovered" fields of the oop. +// is eligible ONLY if its referent is not null. // // As of this version, the only oops in group [2] that can be found by AOTArtifactFinder are // the keys used by ReferencedKeyMap in the implementation of MethodType::internTable. @@ -74,11 +72,35 @@ // the use of Weak/Soft references used by java.lang.invoke. // // We intend to evolve the implementation in the future by -// -- implementing more prepareForAOTCache() operations for other use cases, and/or +// -- implementing more assemblySetup() operations for other use cases, and/or // -- relaxing the eligibility restrictions. +// +// +// null referents for group [1] +// ============================ +// +// Any cached reference R1 of group [1] is allowed to have a null referent. +// This can happen in the following situations: +// (a) R1.clear() was called by Java code during the assembly phase. +// (b) The referent has been collected, and R1 is in the "pending" state. +// In case (b), the "next" and "discovered" fields of the cached copy of R1 will +// be set to null. During the production run: +// - It would appear to the Java program as if immediately during VM start-up, the referent +// was collected and ReferenceThread completed processing of R1. +// - It would appear to the GC as if immediately during VM start-up, the Java program called +// R1.clear(). #if INCLUDE_CDS_JAVA_HEAP + +class KeepAliveObjectsTable : public ResourceHashtable {}; + +static KeepAliveObjectsTable* _keep_alive_objs_table; +static OopHandle _keep_alive_objs_array; static OopHandle _null_queue; void AOTReferenceObjSupport::initialize(TRAPS) { @@ -103,41 +125,76 @@ void AOTReferenceObjSupport::stabilize_cached_reference_objects(TRAPS) { // updated concurrently, so we can remove GC'ed entries ... assert(CDSConfig::allow_only_single_java_thread(), "Required"); - TempNewSymbol method_name = SymbolTable::new_symbol("prepareForAOTCache"); - JavaValue result(T_VOID); - JavaCalls::call_static(&result, vmClasses::MethodType_klass(), + { + TempNewSymbol method_name = SymbolTable::new_symbol("assemblySetup"); + JavaValue result(T_VOID); + JavaCalls::call_static(&result, vmClasses::MethodType_klass(), method_name, vmSymbols::void_method_signature(), CHECK); + } + + { + Symbol* cds_name = vmSymbols::jdk_internal_misc_CDS(); + Klass* cds_klass = SystemDictionary::resolve_or_fail(cds_name, true /*throw error*/, CHECK); + TempNewSymbol method_name = SymbolTable::new_symbol("getKeepAliveObjects"); + TempNewSymbol method_sig = SymbolTable::new_symbol("()[Ljava/lang/Object;"); + JavaValue result(T_OBJECT); + JavaCalls::call_static(&result, cds_klass, method_name, method_sig, CHECK); + + _keep_alive_objs_array = OopHandle(Universe::vm_global(), result.get_oop()); + } + } +} + +void AOTReferenceObjSupport::init_keep_alive_objs_table() { + assert_at_safepoint(); // _keep_alive_objs_table uses raw oops + oop a = _keep_alive_objs_array.resolve(); + if (a != nullptr) { + precond(a->is_objArray()); + precond(CDSConfig::is_dumping_method_handles()); + objArrayOop array = objArrayOop(a); + + _keep_alive_objs_table = new (mtClass)KeepAliveObjectsTable(); + for (int i = 0; i < array->length(); i++) { + oop obj = array->obj_at(i); + _keep_alive_objs_table->put(obj, true); // The array may have duplicated entries but that's OK. + } } } +// Returns true IFF obj is an instance of java.lang.ref.Reference. If so, perform extra eligibility checks. bool AOTReferenceObjSupport::check_if_ref_obj(oop obj) { // We have a single Java thread. This means java.lang.ref.Reference$ReferenceHandler thread // is not running. Otherwise the checks for next/discovered may not work. precond(CDSConfig::allow_only_single_java_thread()); + assert_at_safepoint(); // _keep_alive_objs_table uses raw oops if (obj->klass()->is_subclass_of(vmClasses::Reference_klass())) { oop referent = obj->obj_field(java_lang_ref_Reference::referent_offset()); oop queue = obj->obj_field(java_lang_ref_Reference::queue_offset()); oop next = java_lang_ref_Reference::next(obj); oop discovered = java_lang_ref_Reference::discovered(obj); + bool needs_special_cleanup = (queue != _null_queue.resolve()); + + // If you see the errors below, you probably modified the implementation of java.lang.invoke. + // Please check the comments at the top of this file. + if (needs_special_cleanup && (referent == nullptr || !_keep_alive_objs_table->contains(referent))) { + ResourceMark rm; - if (next != nullptr || discovered != nullptr) { - if (queue != _null_queue.resolve()) { - ResourceMark rm; - log_error(cds, heap)("Cannot archive reference object " PTR_FORMAT " of class %s", - p2i(obj), obj->klass()->external_name()); - log_error(cds, heap)("referent = " PTR_FORMAT - ", queue = " PTR_FORMAT - ", next = " PTR_FORMAT - ", discovered = " PTR_FORMAT, - p2i(referent), p2i(queue), p2i(next), p2i(discovered)); - log_error(cds, heap)("This object requires special clean up as its queue is not ReferenceQueue::N" "ULL (" - PTR_FORMAT ")", p2i(_null_queue.resolve())); - HeapShared::debug_trace(); - MetaspaceShared::unrecoverable_writing_error(); - } + log_error(cds, heap)("Cannot archive reference object " PTR_FORMAT " of class %s", + p2i(obj), obj->klass()->external_name()); + log_error(cds, heap)("referent = " PTR_FORMAT + ", queue = " PTR_FORMAT + ", next = " PTR_FORMAT + ", discovered = " PTR_FORMAT, + p2i(referent), p2i(queue), p2i(next), p2i(discovered)); + log_error(cds, heap)("This object requires special clean up as its queue is not ReferenceQueue::N" "ULL (" + PTR_FORMAT ")", p2i(_null_queue.resolve())); + log_error(cds, heap)("%s", (referent == nullptr) ? + "referent cannot be null" : "referent is not registered with CDS.keepAlive()"); + HeapShared::debug_trace(); + MetaspaceShared::unrecoverable_writing_error(); } if (log_is_enabled(Info, cds, ref)) { diff --git a/src/hotspot/share/cds/aotReferenceObjSupport.hpp b/src/hotspot/share/cds/aotReferenceObjSupport.hpp index bd3fbe2d18075..29bca751dbeb2 100644 --- a/src/hotspot/share/cds/aotReferenceObjSupport.hpp +++ b/src/hotspot/share/cds/aotReferenceObjSupport.hpp @@ -36,6 +36,7 @@ class AOTReferenceObjSupport : AllStatic { public: static void initialize(TRAPS); static void stabilize_cached_reference_objects(TRAPS); + static void init_keep_alive_objs_table(); static bool check_if_ref_obj(oop obj); static bool skip_field(int field_offset); }; diff --git a/src/java.base/share/classes/java/lang/invoke/MethodType.java b/src/java.base/share/classes/java/lang/invoke/MethodType.java index 2455c667ed221..3d15ce6871086 100644 --- a/src/java.base/share/classes/java/lang/invoke/MethodType.java +++ b/src/java.base/share/classes/java/lang/invoke/MethodType.java @@ -1384,7 +1384,7 @@ private Object readResolve() { // This is called from C code, at the very end of Java code execution // during the AOT cache assembly phase. - private static void prepareForAOTCache() { + private static void assemblySetup() { internTable.prepareForAOTCache(); } } diff --git a/src/java.base/share/classes/jdk/internal/misc/CDS.java b/src/java.base/share/classes/jdk/internal/misc/CDS.java index bf2eb183d604f..94696551645c9 100644 --- a/src/java.base/share/classes/jdk/internal/misc/CDS.java +++ b/src/java.base/share/classes/jdk/internal/misc/CDS.java @@ -103,6 +103,15 @@ public static void keepAlive(Object s) { keepAliveList.add(s); } + // This is called by native JVM code at the very end of Java execution before + // dumping the static archive. + // It collects the objects from keepAliveList so that they can be easily processed + // by the native JVM code to check that any Reference objects that need special + // clean up must have been registed with keepAlive() + private static Object[] getKeepAliveObjects() { + return keepAliveList.toArray(); + } + /** * Initialize archived static fields in the given Class using archived * values from CDS dump time. Also initialize the classes of objects in diff --git a/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/WeakReferenceTest.java b/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/WeakReferenceTest.java index a8fd876ee093d..1e93bd2c0375f 100644 --- a/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/WeakReferenceTest.java +++ b/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/WeakReferenceTest.java @@ -33,6 +33,7 @@ * @build WeakReferenceTest * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar weakref.jar * WeakReferenceTestApp WeakReferenceTestApp$Inner ShouldNotBeAOTInited ShouldNotBeArchived SharedQueue + * WeakReferenceTestBadApp1 WeakReferenceTestBadApp2 * @run driver WeakReferenceTest AOT */ @@ -41,19 +42,40 @@ import jdk.test.lib.cds.CDSAppTester; import jdk.test.lib.process.OutputAnalyzer; import jdk.test.lib.helpers.ClassFileInstaller; +import jtreg.SkippedException; public class WeakReferenceTest { static final String appJar = ClassFileInstaller.getJarPath("weakref.jar"); - static final String mainClass = "WeakReferenceTestApp"; + + static final String goodApp = "WeakReferenceTestApp"; + static final String badApp1 = "WeakReferenceTestBadApp1"; + static final String badApp2 = "WeakReferenceTestBadApp2"; public static void main(String[] args) throws Exception { - Tester t = new Tester(); - t.run(args); + new Tester(goodApp).run(args); + + runBadApp(badApp1, args); + runBadApp(badApp2, args); + } + + static void runBadApp(String badApp, String[] args) throws Exception { + try { + new Tester(badApp).run(args); + throw new RuntimeException(badApp + " did not fail in assembly phase as expected"); + } catch (SkippedException e) { + System.out.println("Negative test: expected SkippedException"); + } } static class Tester extends CDSAppTester { - public Tester() { + String mainClass; + public Tester(String mainClass) { super(mainClass); + this.mainClass = mainClass; + + if (mainClass != goodApp) { + setCheckExitValue(false); + } } @Override @@ -66,7 +88,7 @@ public String[] vmArgs(RunMode runMode) { if (runMode == RunMode.ASSEMBLY) { return new String[] { "-Xlog:gc,cds+class=debug", - "-XX:AOTInitTestClass=WeakReferenceTestApp", + "-XX:AOTInitTestClass=" + mainClass, "-Xlog:cds+map,cds+map+oops=trace:file=cds.oops.txt:none:filesize=0", }; } else { @@ -86,6 +108,17 @@ public String[] appCommandLine(RunMode runMode) { @Override public void checkExecution(OutputAnalyzer out, RunMode runMode) throws Exception { + if (runMode == RunMode.ASSEMBLY && mainClass != goodApp) { + out.shouldNotHaveExitValue(0); + out.shouldMatch("Cannot archive reference object .* of class java.lang.ref.WeakReference"); + if (mainClass == badApp1) { + out.shouldContain("referent cannot be null"); + } else { + out.shouldContain("referent is not registered with CDS.keepAlive()"); + } + throw new SkippedException("Assembly phase expected to fail"); + } + out.shouldHaveExitValue(0); out.shouldNotContain("Unexpected exception:"); } @@ -93,8 +126,7 @@ public void checkExecution(OutputAnalyzer out, RunMode runMode) throws Exception } class WeakReferenceTestApp { - // This class is NOT aot-initialized - static class Inner { + static class Inner { // This class is NOT aot-initialized static boolean WeakReferenceTestApp_clinit_executed; } @@ -248,3 +280,30 @@ static ReferenceQueue queue() { return sharedQueueInstance.theQueue; } } + +class WeakReferenceTestBadApp1 { + static WeakReference refWithQueue; + static SharedQueue sharedQueueInstance; + + static { + // See comments in aotReferenceObjSupport.cpp: group [2] references cannot have null referent. + sharedQueueInstance = SharedQueue.sharedQueueInstance; + refWithQueue = new WeakReference(String.class, SharedQueue.queue()); + refWithQueue.clear(); + } + + public static void main(String args[]) {} +} + +class WeakReferenceTestBadApp2 { + static WeakReference refWithQueue; + static SharedQueue sharedQueueInstance; + + static { + // See comments in aotReferenceObjSupport.cpp: group [2] references must be registered with CDS.keepAlive() + sharedQueueInstance = SharedQueue.sharedQueueInstance; + refWithQueue = new WeakReference(String.class, SharedQueue.queue()); + } + + public static void main(String args[]) {} +} From 01f0ea0eff49bd8af124a1972e23429faec6944f Mon Sep 17 00:00:00 2001 From: iklam Date: Tue, 29 Apr 2025 15:10:25 -0700 Subject: [PATCH 13/16] @fisk comment -- use proper HeapAccess to load referent; Also refactor AOTReferenceObjSupport::is_enabled() --- .../share/cds/aotReferenceObjSupport.cpp | 22 ++++++++++++++++--- .../share/cds/aotReferenceObjSupport.hpp | 1 + src/hotspot/share/classfile/javaClasses.cpp | 5 +++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/hotspot/share/cds/aotReferenceObjSupport.cpp b/src/hotspot/share/cds/aotReferenceObjSupport.cpp index 938de6de149a7..e462a36294093 100644 --- a/src/hotspot/share/cds/aotReferenceObjSupport.cpp +++ b/src/hotspot/share/cds/aotReferenceObjSupport.cpp @@ -103,7 +103,17 @@ static KeepAliveObjectsTable* _keep_alive_objs_table; static OopHandle _keep_alive_objs_array; static OopHandle _null_queue; +bool AOTReferenceObjSupport::is_enabled() { + // For simplicity, AOTReferenceObjSupport is enabled only when dumping method handles. + // Otherwise we won't see Reference objects in the AOT cache. Let's be conservative now. + return CDSConfig::is_dumping_method_handles(); +} + void AOTReferenceObjSupport::initialize(TRAPS) { + if (!AOTReferenceObjSupport::is_enabled()) { + return; + } + TempNewSymbol class_name = SymbolTable::new_symbol("java/lang/ref/ReferenceQueue"); Klass* k = SystemDictionary::resolve_or_fail(class_name, true, CHECK); InstanceKlass* ik = InstanceKlass::cast(k); @@ -120,7 +130,7 @@ void AOTReferenceObjSupport::initialize(TRAPS) { // Ensure that all group [2] references found by AOTArtifactFinder are eligible. void AOTReferenceObjSupport::stabilize_cached_reference_objects(TRAPS) { - if (CDSConfig::is_dumping_method_handles()) { + if (AOTReferenceObjSupport::is_enabled()) { // This assert means that the MethodType and MethodTypeForm tables won't be // updated concurrently, so we can remove GC'ed entries ... assert(CDSConfig::allow_only_single_java_thread(), "Required"); @@ -152,7 +162,7 @@ void AOTReferenceObjSupport::init_keep_alive_objs_table() { oop a = _keep_alive_objs_array.resolve(); if (a != nullptr) { precond(a->is_objArray()); - precond(CDSConfig::is_dumping_method_handles()); + precond(AOTReferenceObjSupport::is_enabled()); objArrayOop array = objArrayOop(a); _keep_alive_objs_table = new (mtClass)KeepAliveObjectsTable(); @@ -171,7 +181,13 @@ bool AOTReferenceObjSupport::check_if_ref_obj(oop obj) { assert_at_safepoint(); // _keep_alive_objs_table uses raw oops if (obj->klass()->is_subclass_of(vmClasses::Reference_klass())) { - oop referent = obj->obj_field(java_lang_ref_Reference::referent_offset()); + precond(AOTReferenceObjSupport::is_enabled()); + precond(JavaClasses::is_supported_for_archiving(obj)); + precond(_keep_alive_objs_table != nullptr); + + // GC needs to know about this load, It will keep referent alive until the current safepoint ends. + oop referent = HeapAccess::oop_load_at(obj, java_lang_ref_Reference::referent_offset()); + oop queue = obj->obj_field(java_lang_ref_Reference::queue_offset()); oop next = java_lang_ref_Reference::next(obj); oop discovered = java_lang_ref_Reference::discovered(obj); diff --git a/src/hotspot/share/cds/aotReferenceObjSupport.hpp b/src/hotspot/share/cds/aotReferenceObjSupport.hpp index 29bca751dbeb2..628d1f5acfebd 100644 --- a/src/hotspot/share/cds/aotReferenceObjSupport.hpp +++ b/src/hotspot/share/cds/aotReferenceObjSupport.hpp @@ -39,6 +39,7 @@ class AOTReferenceObjSupport : AllStatic { static void init_keep_alive_objs_table(); static bool check_if_ref_obj(oop obj); static bool skip_field(int field_offset); + static bool is_enabled(); }; #endif // SHARE_CDS_AOTREFERENCEOBJSUPPORT_HPP diff --git a/src/hotspot/share/classfile/javaClasses.cpp b/src/hotspot/share/classfile/javaClasses.cpp index 7fe313cdcb3f4..c7cca9682fe84 100644 --- a/src/hotspot/share/classfile/javaClasses.cpp +++ b/src/hotspot/share/classfile/javaClasses.cpp @@ -22,6 +22,7 @@ * */ +#include "cds/aotReferenceObjSupport.hpp" #include "cds/archiveBuilder.hpp" #include "cds/archiveHeapLoader.hpp" #include "cds/cdsConfig.hpp" @@ -5460,6 +5461,10 @@ bool JavaClasses::is_supported_for_archiving(oop obj) { } } + if (!AOTReferenceObjSupport::is_enabled() && klass->is_subclass_of(vmClasses::Reference_klass())) { + return false; + } + return true; } #endif From d0929acdf6ad021fa498d77504d39c9a5b60a625 Mon Sep 17 00:00:00 2001 From: iklam Date: Tue, 29 Apr 2025 15:55:41 -0700 Subject: [PATCH 14/16] 8354890: AOT-initialize j.l.i.MethodHandleImpl and inner classes --- src/hotspot/share/cds/aotClassInitializer.cpp | 5 ++ src/hotspot/share/cds/cdsHeapVerifier.cpp | 5 -- src/hotspot/share/classfile/vmClassMacros.hpp | 1 + src/hotspot/share/oops/instanceKlass.cpp | 7 ++- .../java/lang/invoke/MethodHandleImpl.java | 4 ++ .../aotClassLinking/MethodHandleTest.java | 52 +++++++++++++++++-- 6 files changed, 65 insertions(+), 9 deletions(-) diff --git a/src/hotspot/share/cds/aotClassInitializer.cpp b/src/hotspot/share/cds/aotClassInitializer.cpp index be7e1f31d8a52..6f45c979880ca 100644 --- a/src/hotspot/share/cds/aotClassInitializer.cpp +++ b/src/hotspot/share/cds/aotClassInitializer.cpp @@ -321,6 +321,10 @@ bool AOTClassInitializer::can_archive_initialized_mirror(InstanceKlass* ik) { if (is_allowed(indy_specs, ik)) { return true; } + + if (ik->name()->starts_with("java/lang/invoke/MethodHandleImpl")) { + return true; + } } #ifdef ASSERT @@ -339,6 +343,7 @@ bool AOTClassInitializer::is_runtime_setup_required(InstanceKlass* ik) { return ik == vmClasses::Class_klass() || ik == vmClasses::internal_Unsafe_klass() || ik == vmClasses::ConcurrentHashMap_klass() || + ik == vmClasses::MethodHandleImpl_klass() || ik == vmClasses::Reference_klass(); } diff --git a/src/hotspot/share/cds/cdsHeapVerifier.cpp b/src/hotspot/share/cds/cdsHeapVerifier.cpp index 9bab62dabe665..64276739641ec 100644 --- a/src/hotspot/share/cds/cdsHeapVerifier.cpp +++ b/src/hotspot/share/cds/cdsHeapVerifier.cpp @@ -138,11 +138,6 @@ CDSHeapVerifier::CDSHeapVerifier() : _archived_objs(0), _problems(0) "CD_Object_array", // E same as <...>ConstantUtils.CD_Object_array::CD_Object "INVOKER_SUPER_DESC"); // E same as java.lang.constant.ConstantDescs::CD_Object - ADD_EXCL("java/lang/invoke/MethodHandleImpl$ArrayAccessor", - "OBJECT_ARRAY_GETTER", // D - "OBJECT_ARRAY_SETTER", // D - "OBJECT_ARRAY_LENGTH"); // D - } # undef ADD_EXCL diff --git a/src/hotspot/share/classfile/vmClassMacros.hpp b/src/hotspot/share/classfile/vmClassMacros.hpp index 351b9b0b53f24..1edc13054d474 100644 --- a/src/hotspot/share/classfile/vmClassMacros.hpp +++ b/src/hotspot/share/classfile/vmClassMacros.hpp @@ -114,6 +114,7 @@ do_klass(VarHandle_klass, java_lang_invoke_VarHandle ) \ do_klass(MemberName_klass, java_lang_invoke_MemberName ) \ do_klass(ResolvedMethodName_klass, java_lang_invoke_ResolvedMethodName ) \ + do_klass(MethodHandleImpl_klass, java_lang_invoke_MethodHandleImpl ) \ do_klass(MethodHandleNatives_klass, java_lang_invoke_MethodHandleNatives ) \ do_klass(LambdaForm_klass, java_lang_invoke_LambdaForm ) \ do_klass(MethodType_klass, java_lang_invoke_MethodType ) \ diff --git a/src/hotspot/share/oops/instanceKlass.cpp b/src/hotspot/share/oops/instanceKlass.cpp index 32445759491b3..3c48311b8717f 100644 --- a/src/hotspot/share/oops/instanceKlass.cpp +++ b/src/hotspot/share/oops/instanceKlass.cpp @@ -860,6 +860,12 @@ void InstanceKlass::initialize_with_aot_initialized_mirror(TRAPS) { return; } + if (is_runtime_setup_required()) { + // Need to take the slow path, which will call the runtimeSetup() function instead + // of + initialize(CHECK); + return; + } if (log_is_enabled(Info, cds, init)) { ResourceMark rm; log_info(cds, init)("%s (aot-inited)", external_name()); @@ -878,7 +884,6 @@ void InstanceKlass::initialize_with_aot_initialized_mirror(TRAPS) { #endif set_init_thread(THREAD); - AOTClassInitializer::call_runtime_setup(THREAD, this); set_initialization_state_and_notify(fully_initialized, CHECK); } #endif diff --git a/src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java b/src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java index cd97413182b19..87005c8f13010 100644 --- a/src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java +++ b/src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java @@ -1526,6 +1526,10 @@ private static NamedFunction createFunction(byte func) { } static { + runtimeSetup(); + } + + private static void runtimeSetup() { SharedSecrets.setJavaLangInvokeAccess(new JavaLangInvokeAccess() { @Override public Class getDeclaringClass(Object rmname) { diff --git a/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/MethodHandleTest.java b/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/MethodHandleTest.java index 84f89ebe215a8..765d9cf6de45f 100644 --- a/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/MethodHandleTest.java +++ b/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/MethodHandleTest.java @@ -87,6 +87,7 @@ public String[] appCommandLine(RunMode runMode) { @Override public void checkExecution(OutputAnalyzer out, RunMode runMode) throws Exception { out.shouldHaveExitValue(0); + out.shouldContain("SwitchBootstraps.typeSwitch: 5678"); if (!runMode.isProductionRun()) { // MethodHandleTestApp should be initialized in the assembly phase as well, @@ -95,6 +96,7 @@ public void checkExecution(OutputAnalyzer out, RunMode runMode) throws Exception } else { // Make sure MethodHandleTestApp is aot-initialized in the production run. out.shouldNotContain("MethodHandleTestApp."); + out.shouldContain("intElm = 777"); } } } @@ -141,17 +143,24 @@ static class B { static VarHandle staticVH; static VarHandle instanceVH; + static MethodHandle arrayGetMH; + static { System.out.println("MethodHandleTestApp."); try { - setupCachedStatics(); + setupCachedMHs(); + invokeUnsupportedBSMs(); } catch (Throwable t) { throw new RuntimeException("Unexpected exception", t); } } - static void setupCachedStatics() throws Throwable { + // This method is executed during the assembly phase. + // + // Store some MHs into the AOT cache. Make sure they can be used during the production run. + // Also check that the class initialization order is consistent with specification. + static void setupCachedMHs() throws Throwable { MethodHandles.Lookup LOOKUP = MethodHandles.lookup(); virtualMH = LOOKUP.findVirtual(A.class, "virtualMethod", MethodType.methodType(void.class)); instanceVH = LOOKUP.findVarHandle(B.class, "instanceField", long.class); @@ -161,11 +170,13 @@ static void setupCachedStatics() throws Throwable { A.staticMethod(); staticMH = LOOKUP.findStatic(A.class, "staticMethod", MethodType.methodType(void.class)); - // Make sure B is initialized before create staticVH, but the AOT-cached staticVH // should still include the init barrier even if B was initialized in the assembly phase. B.staticField += 5678; staticVH = LOOKUP.findStaticVarHandle(B.class, "staticField", long.class); + + // Array access MHs + arrayGetMH = MethodHandles.arrayElementGetter(int[].class); } private static Object invoke(MethodHandle mh, Object ... args) { @@ -184,6 +195,7 @@ public static void main(String[] args) throws Throwable { testMethodHandles(isProduction); testVarHandles(isProduction); + invokeUnsupportedBSMs(); } @@ -212,6 +224,14 @@ static void testMethodHandles(boolean isProduction) throws Throwable { throw new RuntimeException("state_A should be 6 but is: " + state_A); } } + + // (3) Test an array access MH + int[] intArray = new int[] {111, 222, 777}; + int intElm = (Integer)arrayGetMH.invoke(intArray, 2); + System.out.println("intElm = " + intElm); + if (intElm != 777) { + throw new RuntimeException("intElm should be 777 but is: " + intElm); + } } static void testVarHandles(boolean isProduction) throws Throwable { @@ -245,4 +265,30 @@ static void testVarHandles(boolean isProduction) throws Throwable { } } } + + // This method is executed during the assembly phase. + // + // Try to invoke some BSMs that are normally not executed in the assembly phase. However, these + // BSMs may be executed in rare cases (such as when loading signed classes -- see JDK-8353330.) + // Let's make sure the assembly phase can tolerate such BSMs, even if the call sites that they + // produce are not stored into the AOT cache. + static void invokeUnsupportedBSMs() { + int n = testTypeSwitch((Integer)1234); + System.out.println("SwitchBootstraps.typeSwitch: " + n); + if (n != 5678) { + throw new RuntimeException("n should be " + 5678 + " but is: " + n); + } + } + + static int testTypeSwitch(Number n) { + // The BSM is java/lang/runtime/SwitchBootstraps::typeSwitch + return switch (n) { + case Integer in -> { + yield 5678; + } + default -> { + yield 0; + } + }; + } } From 0f6a2e0abced42fddfc051bb5e47425a9ab7fdd1 Mon Sep 17 00:00:00 2001 From: iklam Date: Wed, 30 Apr 2025 14:35:32 -0700 Subject: [PATCH 15/16] Added more test case to increase coverage on possible core-lib usage patterns for MethodHandles --- .../share/cds/aotConstantPoolResolver.cpp | 2 +- src/hotspot/share/cds/cdsHeapVerifier.cpp | 4 + .../aotClassLinking/MethodHandleTest.java | 110 +++++++++++++++++- 3 files changed, 110 insertions(+), 6 deletions(-) diff --git a/src/hotspot/share/cds/aotConstantPoolResolver.cpp b/src/hotspot/share/cds/aotConstantPoolResolver.cpp index 15ca2b2c2a0d7..b34650adf92b5 100644 --- a/src/hotspot/share/cds/aotConstantPoolResolver.cpp +++ b/src/hotspot/share/cds/aotConstantPoolResolver.cpp @@ -525,7 +525,7 @@ bool AOTConstantPoolResolver::is_indy_resolution_deterministic(ConstantPool* cp, Symbol* factory_type_sig = cp->uncached_signature_ref_at(cp_index); if (log_is_enabled(Debug, cds, resolve)) { ResourceMark rm; - log_debug(cds, resolve)("Checking indy callsite signature [%d]: %s", cp_index, factory_type_sig->as_C_string()); + log_debug(cds, resolve)("Checking lambda callsite signature [%d]: %s", cp_index, factory_type_sig->as_C_string()); } if (!check_lambda_metafactory_signature(cp, factory_type_sig)) { diff --git a/src/hotspot/share/cds/cdsHeapVerifier.cpp b/src/hotspot/share/cds/cdsHeapVerifier.cpp index 64276739641ec..2e96f818c141c 100644 --- a/src/hotspot/share/cds/cdsHeapVerifier.cpp +++ b/src/hotspot/share/cds/cdsHeapVerifier.cpp @@ -138,6 +138,10 @@ CDSHeapVerifier::CDSHeapVerifier() : _archived_objs(0), _problems(0) "CD_Object_array", // E same as <...>ConstantUtils.CD_Object_array::CD_Object "INVOKER_SUPER_DESC"); // E same as java.lang.constant.ConstantDescs::CD_Object + ADD_EXCL("java/lang/runtime/ObjectMethods", "CLASS_IS_INSTANCE", // D + "FALSE", // D + "TRUE", // D + "ZERO"); // D } # undef ADD_EXCL diff --git a/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/MethodHandleTest.java b/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/MethodHandleTest.java index 765d9cf6de45f..12ace08df5e17 100644 --- a/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/MethodHandleTest.java +++ b/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/MethodHandleTest.java @@ -33,16 +33,22 @@ * @build MethodHandleTest * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar mh.jar * MethodHandleTestApp MethodHandleTestApp$A MethodHandleTestApp$B + * UnsupportedBSMs UnsupportedBSMs$MyEnum + * ObjectMethodsTest ObjectMethodsTest$C * @run driver MethodHandleTest AOT */ +import java.io.Serializable; +import java.lang.invoke.CallSite; import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; import java.lang.invoke.VarHandle; +import java.lang.runtime.ObjectMethods; import jdk.test.lib.cds.CDSAppTester; import jdk.test.lib.process.OutputAnalyzer; import jdk.test.lib.helpers.ClassFileInstaller; +import static java.lang.invoke.MethodType.methodType; public class MethodHandleTest { static final String appJar = ClassFileInstaller.getJarPath("mh.jar"); @@ -145,12 +151,17 @@ static class B { static MethodHandle arrayGetMH; + // Created in assembly phase. + // Used in production run. + static MethodHandle ObjectMethodsTest_handle; + static { System.out.println("MethodHandleTestApp."); try { setupCachedMHs(); - invokeUnsupportedBSMs(); + ObjectMethodsTest_handle = ObjectMethodsTest.makeHandle(); + UnsupportedBSMs.invokeUnsupportedBSMs(); } catch (Throwable t) { throw new RuntimeException("Unexpected exception", t); } @@ -195,9 +206,11 @@ public static void main(String[] args) throws Throwable { testMethodHandles(isProduction); testVarHandles(isProduction); - invokeUnsupportedBSMs(); - } + ObjectMethodsTest.testEqualsC(ObjectMethodsTest_handle); + + UnsupportedBSMs.invokeUnsupportedBSMs(); + } static void testMethodHandles(boolean isProduction) throws Throwable { state_A = 0; @@ -265,23 +278,91 @@ static void testVarHandles(boolean isProduction) throws Throwable { } } } +} + +// Excerpt from test/jdk/java/lang/runtime/ObjectMethodsTest.java +class ObjectMethodsTest { + public static class C { + static final MethodType EQUALS_DESC = methodType(boolean.class, C.class, Object.class); + static final MethodType HASHCODE_DESC = methodType(int.class, C.class); + static final MethodType TO_STRING_DESC = methodType(String.class, C.class); + + static final MethodHandle[] ACCESSORS = accessors(); + static final String NAME_LIST = "x;y"; + private static MethodHandle[] accessors() { + try { + return new MethodHandle[]{ + MethodHandles.lookup().findGetter(C.class, "x", int.class), + MethodHandles.lookup().findGetter(C.class, "y", int.class), + }; + } catch (Exception e) { + throw new AssertionError(e); + } + } + + private final int x; + private final int y; + C (int x, int y) { this.x = x; this.y = y; } + public int x() { return x; } + public int y() { return y; } + } + + public static MethodHandle makeHandle() throws Throwable { + MethodHandles.Lookup LOOKUP = MethodHandles.lookup(); + + CallSite cs = (CallSite)ObjectMethods.bootstrap(LOOKUP, "equals", C.EQUALS_DESC, C.class, C.NAME_LIST, C.ACCESSORS); + return cs.dynamicInvoker(); + } + + public static void testEqualsC(MethodHandle handle) throws Throwable { + C c = new C(5, 5); + assertTrue((boolean)handle.invokeExact(c, (Object)c)); + assertTrue((boolean)handle.invokeExact(c, (Object)new C(5, 5))); + assertFalse((boolean)handle.invokeExact(c, (Object)new C(5, 4))); + assertFalse((boolean)handle.invokeExact(c, (Object)new C(4, 5))); + assertFalse((boolean)handle.invokeExact(c, (Object)null)); + assertFalse((boolean)handle.invokeExact(c, new Object())); + } + private static void assertTrue(boolean b) { + if (b != true) { + throw new RuntimeException("Assertion fails"); + } + } + + private static void assertFalse(boolean b) { + assertTrue(!b); + } +} + +class UnsupportedBSMs { // This method is executed during the assembly phase. // // Try to invoke some BSMs that are normally not executed in the assembly phase. However, these // BSMs may be executed in rare cases (such as when loading signed classes -- see JDK-8353330.) // Let's make sure the assembly phase can tolerate such BSMs, even if the call sites that they // produce are not stored into the AOT cache. - static void invokeUnsupportedBSMs() { + // + // Hopefully with enough testing in here, we can avoid situations where innocent changes in + // core libs might cause the AOT assembly phase to fail. + static void invokeUnsupportedBSMs() throws Throwable { int n = testTypeSwitch((Integer)1234); System.out.println("SwitchBootstraps.typeSwitch: " + n); if (n != 5678) { throw new RuntimeException("n should be " + 5678 + " but is: " + n); } + + Object o = getRunnableAndSerializable(); + System.out.println(o.getClass()); + if (!(o instanceof Runnable) || !(o instanceof Serializable)) { + throw new RuntimeException("o has wrong interfaces"); + } + + statementEnum(MyEnum.A); } static int testTypeSwitch(Number n) { - // The BSM is java/lang/runtime/SwitchBootstraps::typeSwitch + // BSM = java/lang/runtime/SwitchBootstraps::typeSwitch return switch (n) { case Integer in -> { yield 5678; @@ -291,4 +372,23 @@ static int testTypeSwitch(Number n) { } }; } + + static Runnable getRunnableAndSerializable() { + // BSM = java/lang/invoke/LambdaMetafactory.altMetafactory + return (Runnable & Serializable) () -> { + System.out.println("Inside getRunnableAndSerializable"); + }; + } + + // Excerpt from test/langtools/tools/javac/patterns/EnumTypeChanges.java + enum MyEnum { A, B; } + static String statementEnum(MyEnum e) { + // BSM = java/lang/runtime/SwitchBootstraps.enumSwitch + switch (e) { + case A -> { return "A"; } + case B -> { return "B"; } + case MyEnum e1 when e1 == null -> throw new AssertionError(); + default -> { return "D"; } + } + } } From a1e3743b22ada3d6691c94fba4330b6f755214e4 Mon Sep 17 00:00:00 2001 From: iklam Date: Mon, 5 May 2025 16:45:52 -0700 Subject: [PATCH 16/16] Comments from @liach and @ExE-Boss --- .../share/classes/java/lang/invoke/MethodHandleImpl.java | 2 +- src/java.base/share/classes/java/lang/ref/Reference.java | 2 +- .../runtime/cds/appcds/aotClassLinking/MethodHandleTest.java | 5 ++++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java b/src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java index d86226d5ee6f6..a759ec6162e1e 100644 --- a/src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java +++ b/src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java @@ -1525,11 +1525,11 @@ private static NamedFunction createFunction(byte func) { } } - // Called from JVM when loading an AOT cache static { runtimeSetup(); } + // Also called from JVM when loading an AOT cache private static void runtimeSetup() { SharedSecrets.setJavaLangInvokeAccess(new JavaLangInvokeAccess() { @Override diff --git a/src/java.base/share/classes/java/lang/ref/Reference.java b/src/java.base/share/classes/java/lang/ref/Reference.java index e109b974adc55..e3e0258c3c949 100644 --- a/src/java.base/share/classes/java/lang/ref/Reference.java +++ b/src/java.base/share/classes/java/lang/ref/Reference.java @@ -306,11 +306,11 @@ static void startReferenceHandlerThread(ThreadGroup tg) { handler.start(); } - // Called from JVM when loading an AOT cache static { runtimeSetup(); } + // Also called from JVM when loading an AOT cache private static void runtimeSetup() { // provide access in SharedSecrets SharedSecrets.setJavaLangRefAccess(new JavaLangRefAccess() { diff --git a/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/MethodHandleTest.java b/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/MethodHandleTest.java index 12ace08df5e17..bc07991eeb2eb 100644 --- a/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/MethodHandleTest.java +++ b/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/MethodHandleTest.java @@ -358,7 +358,10 @@ static void invokeUnsupportedBSMs() throws Throwable { throw new RuntimeException("o has wrong interfaces"); } - statementEnum(MyEnum.A); + String s = statementEnum(MyEnum.A); + if (!s.equals("A")) { + throw new RuntimeException("enum switch incorrect"); + } } static int testTypeSwitch(Number n) {