Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions substratevm/mx.substratevm/suite.py
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@
],
"requires" : [
"jdk.management",
"jdk.jfr",
],
"requiresConcealed" : {
"java.base": [
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
* Copyright (c) 2023, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2023, 2023, Red Hat Inc. 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. Oracle designates this
* particular file as subject to the "Classpath" exception as provided
* by Oracle in the LICENSE file that accompanied this code.
*
* 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.
*/

package com.oracle.svm.core.genscavenge;

import com.oracle.svm.core.Uninterruptible;
import com.oracle.svm.core.heap.GCCause;
import com.oracle.svm.core.heap.Heap;
import com.oracle.svm.core.jfr.JfrEvent;
import jdk.jfr.Event;
import jdk.jfr.Name;
import jdk.jfr.Period;

@Name("EveryChunkPeriodicGCEvents")
@Period(value = "everyChunk")
public class EveryChunkNativeGCPeriodicEvents extends Event {

public static void emit() {
emitObjectCount();
}

private static void emitObjectCount() {
if (shouldEmitObjectCount()) {
Heap.getHeap().getGC().collectCompletely(GCCause.JfrObjectCount);
}
}

/**
* ShouldEmit will be checked again later. This is merely an optimization to avoid a potentially
* unnecessary GC.
*/
@Uninterruptible(reason = "Caller of JfrEvent#shouldEmit must be uninterruptible.")
private static boolean shouldEmitObjectCount() {
return JfrEvent.ObjectCount.shouldEmit();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
import com.oracle.svm.core.jfr.JfrGCWhen;
import com.oracle.svm.core.jfr.JfrTicks;
import com.oracle.svm.core.jfr.events.AllocationRequiringGCEvent;
import com.oracle.svm.core.jfr.events.ObjectCountEventSupport;
import com.oracle.svm.core.log.Log;
import com.oracle.svm.core.os.CommittedMemoryProvider;
import com.oracle.svm.core.snippets.ImplicitExceptions;
Expand Down Expand Up @@ -244,6 +245,7 @@ private boolean collectImpl(GCCause cause, long requestingNanoTime, boolean forc
}
} finally {
JfrGCEvents.emitGarbageCollectionEvent(getCollectionEpoch(), cause, startTicks);
ObjectCountEventSupport.emitEvents(getCollectionEpoch(), startTicks, cause);
}
return outOfMemory;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@
*/
package com.oracle.svm.core.genscavenge;

import jdk.jfr.FlightRecorder;
import org.graalvm.nativeimage.ImageSingletons;
import org.graalvm.nativeimage.StackValue;
import org.graalvm.word.UnsignedWord;

import com.oracle.svm.core.jdk.RuntimeSupport;
import com.oracle.svm.core.SubstrateOptions;
import com.oracle.svm.core.Uninterruptible;
import com.oracle.svm.core.feature.AutomaticallyRegisteredFeature;
Expand Down Expand Up @@ -136,6 +138,18 @@ private int popPhase() {
assert currentPhase > 0;
return --currentPhase;
}

public static RuntimeSupport.Hook startupHook() {
return isFirstIsolate -> {
FlightRecorder.addPeriodicEvent(EveryChunkNativeGCPeriodicEvents.class, EveryChunkNativeGCPeriodicEvents::emit);
};
}

public static RuntimeSupport.Hook shutdownHook() {
return isFirstIsolate -> {
FlightRecorder.removePeriodicEvent(EveryChunkNativeGCPeriodicEvents::emit);
};
}
}

@AutomaticallyRegisteredFeature
Expand All @@ -150,6 +164,10 @@ public void beforeAnalysis(BeforeAnalysisAccess access) {
if (HasJfrSupport.get()) {
JfrGCName name = JfrGCNames.singleton().addGCName("serial");
ImageSingletons.add(JfrGCEventSupport.class, new JfrGCEventSupport(name));

RuntimeSupport runtime = RuntimeSupport.getRuntimeSupport();
runtime.addStartupHook(JfrGCEventSupport.startupHook());
runtime.addShutdownHook(JfrGCEventSupport.shutdownHook());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ public class GCCause {
@DuplicatedInNativeCode public static final GCCause HintedGC = new GCCause("Hinted GC", 3);
@DuplicatedInNativeCode public static final GCCause JvmtiForceGC = new GCCause("JvmtiEnv ForceGarbageCollection", 4);
@DuplicatedInNativeCode public static final GCCause HeapDump = new GCCause("Heap Dump Initiated GC ", 5);
/**
* {@link GCCause#JfrObjectCount} is a GC cause hotspot does not have. It indicates the GC was
* invoked in order to emit JFR ObjectCount periodic events.
*/
@DuplicatedInNativeCode public static final GCCause JfrObjectCount = new GCCause("JFR object counting", 6);

@UnknownObjectField(availability = ReadyForCompilation.class) protected static GCCause[] GCCauses;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ public void setMaxTypeId(int maxTypeId) {
this.maxTypeId = maxTypeId;
}

@Platforms(Platform.HOSTED_ONLY.class)
public int getMaxTypeId() {
return maxTypeId;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ public final class JfrEvent {
public static final JfrEvent ThreadAllocationStatistics = create("jdk.ThreadAllocationStatistics", false);
public static final JfrEvent SystemGC = create("jdk.SystemGC", true);
public static final JfrEvent AllocationRequiringGC = create("jdk.AllocationRequiringGC", false);
public static final JfrEvent ObjectCount = create("jdk.ObjectCount", false);
public static final JfrEvent ObjectCountAfterGC = create("jdk.ObjectCountAfterGC", false);

private final long id;
private final String name;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import com.oracle.svm.core.c.struct.PinnedObjectField;
import com.oracle.svm.core.collections.AbstractUninterruptibleHashtable;
import com.oracle.svm.core.collections.UninterruptibleEntry;
import com.oracle.svm.core.heap.Heap;
import com.oracle.svm.core.jdk.UninterruptibleUtils;
import com.oracle.svm.core.jdk.UninterruptibleUtils.CharReplacer;
import com.oracle.svm.core.jfr.traceid.JfrTraceIdEpoch;
Expand Down Expand Up @@ -77,8 +76,6 @@ public long getSymbolId(String imageHeapString, boolean previousEpoch, boolean r
return 0;
}

assert Heap.getHeap().isInImageHeap(imageHeapString);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assertion was failing due to com.sun.proxy.jdk.proxy1. I think this assertion is incorrect because its possible we may sometimes need to serialize symbols not in the image heap. This was exposed now since the ObjectCount event causes many more symbols to be required. What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be bad, then we would need to change JfrSymbolRepository significantly. Do you have a stack trace?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the string is not in the image heap, would it be possible to derive the hash from the symbol string itself rather than from its memory address?

I added an assertion to JfrTypeRepository.writePackage. Then with mx native-unittest:

21) com.oracle.svm.test.jfr.TestVirtualThreadsJfrStreaming#test
java.lang.AssertionError: Not in image heapcom.sun.proxy.jdk.proxy1
        at org.graalvm.nativeimage.builder/com.oracle.svm.core.jfr.JfrTypeRepository.writePackage(JfrTypeRepository.java:179)
        at org.graalvm.nativeimage.builder/com.oracle.svm.core.jfr.JfrTypeRepository.writePackages(JfrTypeRepository.java:172)
        at org.graalvm.nativeimage.builder/com.oracle.svm.core.jfr.JfrTypeRepository.write(JfrTypeRepository.java:68)
        at org.graalvm.nativeimage.builder/com.oracle.svm.core.jfr.JfrChunkWriter.writeConstantPools(JfrChunkWriter.java:354)
        at org.graalvm.nativeimage.builder/com.oracle.svm.core.jfr.JfrChunkWriter.writeCheckpointEvent(JfrChunkWriter.java:324)
        at org.graalvm.nativeimage.builder/com.oracle.svm.core.jfr.JfrChunkWriter.writeFlushCheckpoint(JfrChunkWriter.java:296)
        at org.graalvm.nativeimage.builder/com.oracle.svm.core.jfr.JfrChunkWriter.closeFile(JfrChunkWriter.java:222)
        at org.graalvm.nativeimage.builder/com.oracle.svm.core.jfr.SubstrateJVM.setOutput(SubstrateJVM.java:363)
        at jdk.jfr@21/jdk.jfr.internal.JVM.setOutput(JVM.java:167)
        at jdk.jfr@21/jdk.jfr.internal.MetadataRepository.setOutput(MetadataRepository.java:295)
        at jdk.jfr@21/jdk.jfr.internal.PlatformRecorder.start(PlatformRecorder.java:267)
        at jdk.jfr@21/jdk.jfr.internal.PlatformRecording.start(PlatformRecording.java:123)
        at jdk.jfr@21/jdk.jfr.consumer.RecordingStream.startAsync(RecordingStream.java:379)
        at com.oracle.svm.test.jfr.JfrStreamingTest.startStream(JfrStreamingTest.java:87)
        at com.oracle.svm.test.jfr.JfrStreamingTest.startStream(JfrStreamingTest.java:73)
        at com.oracle.svm.test.jfr.TestVirtualThreadsJfrStreaming.test(TestVirtualThreadsJfrStreaming.java:68)
        at java.base@21/java.lang.reflect.Method.invoke(Method.java:580)
        at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
        at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
        at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
        at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
        at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
        at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
        at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
        at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
        at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
        at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
        at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
        at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
        at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
        at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
        at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
        at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
        at org.junit.runners.Suite.runChild(Suite.java:128)
        at org.junit.runners.Suite.runChild(Suite.java:27)
        at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
        at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
        at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
        at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
        at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
        at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
        at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
        at org.junit.runner.JUnitCore.run(JUnitCore.java:115)
        at com.oracle.mxtool.junit.MxJUnitWrapper.runRequest(MxJUnitWrapper.java:376)
        at org.graalvm.nativeimage.junitsupport/com.oracle.svm.junit.SVMJUnitRunner.run(SVMJUnitRunner.java:164)
        at org.graalvm.nativeimage.junitsupport/com.oracle.svm.junit.SVMJUnitRunner.main(SVMJUnitRunner.java:184)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@roberttoyonaga : please try to figure out where the problematic Package object is created, for which package.getName() returns an object that doesn't live in the image heap. I suppose that Package object also doesn't live in the image heap?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please try to figure out where the problematic Package object is created

It gets created the first time we try to get the package. In this case, that's in JfrTypeRepository while we're trying to write out the type data.
Class.getPackage() --> BootLoader.definePackage(Class<?> c) --> Target_jdk_internal_loader_BootLoader.getDefinedPackage --> Target_java_lang_Package()

I suppose that Package object also doesn't live in the image heap?

That's correct. It seems like when DynamicHubInitializer.registerPackage is called at build time, javaClass.getPackage() returns null for all packages with name com.sun.proxy.jdk.proxy1, preventing the package from being registered. Maybe this is why the package is not in the image heap.

Is something wrong with the behavior for handling proxy classes? Otherwise, should we modify the JfrSymbolRepository (I can do that in a separate PR)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds like a difference in behavior between Native Image and HotSpot. If HotSpot directly returns null for the package of proxy classes, then Native Image should do the same (i.e., it should not allocate a package at run-time in Target_jdk_internal_loader_BootLoader.getDefinedPackage).

Please try to implement a smaller reproducer that shows this difference in behavior and open a separate issue for this problem.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @christianhaeubl, I've opened an issue here: #8796
It includes a small reproducer too.


JfrSymbol symbol = StackValue.get(JfrSymbol.class);
symbol.setValue(imageHeapString);
symbol.setReplaceDotWithSlash(replaceDotWithSlash);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,14 @@ private TypeInfo collectTypeInfo(boolean flushpoint) {

private void visitClass(TypeInfo typeInfo, Class<?> clazz) {
if (clazz != null && addClass(typeInfo, clazz)) {
visitPackage(typeInfo, clazz.getPackage(), clazz.getModule());
visitPackage(typeInfo, clazz.getPackage(), clazz.getModule(), getPackageKey(clazz));
visitClass(typeInfo, clazz.getSuperclass());
visitClassLoader(typeInfo, clazz.getClassLoader());
}
}

private void visitPackage(TypeInfo typeInfo, Package pkg, Module module) {
if (pkg != null && addPackage(typeInfo, pkg, module)) {
private void visitPackage(TypeInfo typeInfo, Package pkg, Module module, String pkgKey) {
if (pkg != null && addPackage(typeInfo, pkg, module, pkgKey)) {
visitModule(typeInfo, module);
}
}
Expand Down Expand Up @@ -145,7 +146,7 @@ private void writeClass(TypeInfo typeInfo, JfrChunkWriter writer, Class<?> clazz
writer.writeCompressedLong(JfrTraceId.getTraceId(clazz));
writer.writeCompressedLong(getClassLoaderId(typeInfo, clazz.getClassLoader()));
writer.writeCompressedLong(getSymbolId(writer, clazz.getName(), flushpoint, true));
writer.writeCompressedLong(getPackageId(typeInfo, clazz.getPackage()));
writer.writeCompressedLong(getPackageId(typeInfo, clazz));
writer.writeCompressedLong(clazz.getModifiers());
writer.writeBoolean(clazz.isHidden());
}
Expand All @@ -168,14 +169,14 @@ private int writePackages(JfrChunkWriter writer, TypeInfo typeInfo, boolean flus
writer.writeCompressedInt(typeInfo.packages.size());

for (Map.Entry<String, PackageInfo> pkgInfo : typeInfo.packages.entrySet()) {
writePackage(typeInfo, writer, pkgInfo.getKey(), pkgInfo.getValue(), flushpoint);
writePackage(typeInfo, writer, pkgInfo.getValue(), flushpoint);
}
return NON_EMPTY;
}

private void writePackage(TypeInfo typeInfo, JfrChunkWriter writer, String pkgName, PackageInfo pkgInfo, boolean flushpoint) {
private void writePackage(TypeInfo typeInfo, JfrChunkWriter writer, PackageInfo pkgInfo, boolean flushpoint) {
writer.writeCompressedLong(pkgInfo.id); // id
writer.writeCompressedLong(getSymbolId(writer, pkgName, flushpoint, true));
writer.writeCompressedLong(getSymbolId(writer, pkgInfo.pkgName, flushpoint, true));
writer.writeCompressedLong(getModuleId(typeInfo, pkgInfo.module));
writer.writeBoolean(false); // exported
}
Expand Down Expand Up @@ -228,10 +229,12 @@ private static void writeClassLoader(JfrChunkWriter writer, ClassLoader cl, long
private static class PackageInfo {
private final long id;
private final Module module;
private final String pkgName;

PackageInfo(long id, Module module) {
PackageInfo(long id, Module module, String pkgName) {
this.id = id;
this.module = module;
this.pkgName = pkgName;
}
}

Expand All @@ -246,27 +249,47 @@ private boolean isClassVisited(TypeInfo typeInfo, Class<?> clazz) {
return typeInfo.classes.contains(clazz) || flushedClasses.contains(clazz);
}

private boolean addPackage(TypeInfo typeInfo, Package pkg, Module module) {
if (isPackageVisited(typeInfo, pkg)) {
assert module == (flushedPackages.containsKey(pkg.getName()) ? flushedPackages.get(pkg.getName()).module : typeInfo.packages.get(pkg.getName()).module);
/**
* It is possible that generated classes may have the same package as ordinary non-generated
* classes, however their module may be different (the unnamed module). We must deduplicate
* packages during serialization, but since a single package name may correspond to multiple
* modules, we make the deduplication key the concatenation of the package and module name. This
* allows each package module combination to be represented in the serialized data.
*/
private static String getPackageKey(Class<?> clazz) {
if (clazz.getPackage() == null) {
return null;
}
String packageKey = clazz.getPackage().getName();
if (clazz.getModule() != null && clazz.getModule().getName() != null) {
packageKey += clazz.getModule().getName();
}
return packageKey;
}

private boolean addPackage(TypeInfo typeInfo, Package pkg, Module module, String pkgKey) {

if (isPackageVisited(typeInfo, pkgKey)) {
assert module == (flushedPackages.containsKey(pkgKey) ? flushedPackages.get(pkgKey).module : typeInfo.packages.get(pkgKey).module);
return false;
}
// The empty package represented by "" is always traced with id 0
long id = pkg.getName().isEmpty() ? 0 : ++currentPackageId;
typeInfo.packages.put(pkg.getName(), new PackageInfo(id, module));
typeInfo.packages.put(pkgKey, new PackageInfo(id, module, pkg.getName()));
return true;
}

private boolean isPackageVisited(TypeInfo typeInfo, Package pkg) {
return flushedPackages.containsKey(pkg.getName()) || typeInfo.packages.containsKey(pkg.getName());
private boolean isPackageVisited(TypeInfo typeInfo, String packageKey) {
return flushedPackages.containsKey(packageKey) || typeInfo.packages.containsKey(packageKey);
}

private long getPackageId(TypeInfo typeInfo, Package pkg) {
if (pkg != null) {
if (flushedPackages.containsKey(pkg.getName())) {
return flushedPackages.get(pkg.getName()).id;
private long getPackageId(TypeInfo typeInfo, Class<?> clazz) {
String packageKey = getPackageKey(clazz);
if (packageKey != null) {
if (flushedPackages.containsKey(packageKey)) {
return flushedPackages.get(packageKey).id;
}
return typeInfo.packages.get(pkg.getName()).id;
return typeInfo.packages.get(packageKey).id;
} else {
return 0;
}
Expand Down
Loading