Skip to content

[GR-67169] Address JFR follow-up review comments #13440

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
graalvmbot wants to merge 4 commits into
base: master
Choose a base branch
from
Open

[GR-67169] Address JFR follow-up review comments #13440

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5f6a485
Limit JFR OOME dumps to VM-exit paths
thomaswue Apr 29, 2026
cd95afb
Make JFR emergency checkpoints allocation-free
thomaswue Apr 29, 2026
a199a7f
Update JFR emergency dump tests
thomaswue Apr 29, 2026
17219a8
Address JFR review feedback
thomaswue Apr 30, 2026
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
5 changes: 3 additions & 2 deletions substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/GCCause.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import com.oracle.svm.shared.singletons.AutomaticallyRegisteredImageSingleton;
import com.oracle.svm.core.feature.InternalFeature;
import com.oracle.svm.core.imagelayer.ImageLayerBuildingSupport;
import com.oracle.svm.core.util.AbstractImageHeapList;
import com.oracle.svm.core.util.DuplicatedInNativeCode;
import com.oracle.svm.core.util.ImageHeapList;
import com.oracle.svm.shared.Uninterruptible;
Expand Down Expand Up @@ -89,7 +90,7 @@ public static GCCause fromId(int causeId) {
return getGCCauses().get(causeId);
}

public static List<GCCause> getGCCauses() {
public static AbstractImageHeapList<GCCause> getGCCauses() {
return ImageSingletons.lookup(GCCauseSupport.class).gcCauses;
}

Expand All @@ -102,7 +103,7 @@ public static void registerGCCause(GCCause cause) {
@AutomaticallyRegisteredImageSingleton
@SingletonTraits(access = AllAccess.class, layeredCallbacks = SingleLayer.class, layeredInstallationKind = InitialLayerOnly.class)
class GCCauseSupport {
final List<GCCause> gcCauses = ImageHeapList.create(GCCause.class, null);
final AbstractImageHeapList<GCCause> gcCauses = ImageHeapList.create(GCCause.class, null);

@Platforms(Platform.HOSTED_ONLY.class)
Object collectGCCauses(Object obj) {
Expand Down
14 changes: 10 additions & 4 deletions substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/OutOfMemoryUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,8 @@ private static void reportOutOfMemoryError0(OutOfMemoryError error) {
(!ImageLayerBuildingSupport.buildingImageLayer() || HeapDumpMetadata.isLayeredMetadataAvailable())) {
HeapDumping.singleton().dumpHeapOnOutOfMemoryError();
}
if (HasJfrSupport.get()) {
SubstrateJVM.get().vmOutOfMemoryErrorRotation();
}

if (SubstrateGCOptions.ExitOnOutOfMemoryError.getValue()) {
dumpJfrOnOutOfMemoryError();
if (LibC.isSupported()) {
Copy link
Copy Markdown
Collaborator

@roberttoyonaga roberttoyonaga Apr 30, 2026

Choose a reason for hiding this comment

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

Why remove dumpJfrOnOutOfMemoryError() from here now?

Copy link
Copy Markdown
Member

@thomaswue thomaswue May 1, 2026

Choose a reason for hiding this comment

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

This is to match HotSpot behavior. Our tests and research indicate that HotSpot does not do that dump when -XX:+ExitOnOutOfMemoryError is enabled. There is however indeed a general design question how close we want to match HotSpot's behavior. Or if we want to provide additional functionality.

Copy link
Copy Markdown
Collaborator

@roberttoyonaga roberttoyonaga May 1, 2026

Choose a reason for hiding this comment

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

I'm fine with matching hotspot as closely as possible. Hotspot also does a JFR emergency dump upon VM crashes in VMError::report_and_die. I wonder if it's possible to do an emergency dump from VMError.shouldNotReachHere to achieve something similar. Although, it may not be possible since emergency dumps require a safepoint and we don't know what state the VM is in.

On the other hand, if we are okay not exactly matching Hotspot, I do think it could be useful generating an emergency dump if ExitOnOutOfMemoryError or if an OOME error goes uncaught by the application code. The user has already included JFR in the image build and JFR would already need to be running anyway.

Copy link
Copy Markdown
Member

@thomaswue thomaswue May 1, 2026

Choose a reason for hiding this comment

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

Yes, I think we might want to offer more options than HotSpot, at least if it does not make the implementation overly complex. @christianhaeubl What is your opinion on the longer term plans?

Copy link
Copy Markdown
Member

@christianhaeubl christianhaeubl May 4, 2026

Choose a reason for hiding this comment

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

We can offer additional options. However by default, I think Native Image should behave in the same way as HotSpot as differences can cause problems for people that want to adopt Native Image.

I wonder if it's possible to do an emergency dump from VMError.shouldNotReachHere to achieve something similar

It should be possible, but it will definitely require some work as we would want to skip JFR emergency dumping in some cases (e.g., the crash corrupted some VM-internal data structure, a recursive crash happens during JFR emergency dumping, the process is out-of-memory on the OS-level). As HotSpot has pretty much the same problem, I assume that they already implemented some heuristic.

Copy link
Copy Markdown
Collaborator

@roberttoyonaga roberttoyonaga May 4, 2026

Choose a reason for hiding this comment

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

However by default, I think Native Image should behave in the same way as HotSpot as differences can cause problems for people that want to adopt Native Image

Okay, let's match Hotspot and not dump on ExitOnOutOfMemoryError or uncaught OOME.

As HotSpot has pretty much the same problem, I assume that they already implemented some heuristic.

Yes, that's right. Hotspot does a bunch of things to try to increase the likelihood of the dump succeeding on VM crash, but acknowledges it's still only a best effort. We can decide later whether to do this in a future enhancement.

Log.log().string("Terminating due to java.lang.OutOfMemoryError: ").string(JDKUtils.getRawMessage(error)).newline();
LibC.exit(3);
Expand All @@ -92,7 +89,16 @@ private static void reportOutOfMemoryError0(OutOfMemoryError error) {
}

if (SubstrateGCOptions.ReportFatalErrorOnOutOfMemoryError.getValue()) {
dumpJfrOnOutOfMemoryError();
throw VMError.shouldNotReachHere("reporting due to java.lang.OutOfMemoryError");
}
}

@Uninterruptible(reason = "Not uninterruptible but it doesn't matter for the callers.", calleeMustBe = false)
@RestrictHeapAccess(access = RestrictHeapAccess.Access.NO_ALLOCATION, reason = "Can't allocate while out of memory.")
private static void dumpJfrOnOutOfMemoryError() {
if (HasJfrSupport.get()) {
SubstrateJVM.get().dumpOnOutOfMemoryError();
}
}
}
140 changes: 72 additions & 68 deletions substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/UninterruptibleUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@
import org.graalvm.word.Pointer;
import org.graalvm.word.PointerBase;
import org.graalvm.word.UnsignedWord;
import org.graalvm.word.WordBase;
import org.graalvm.word.impl.Word;
import org.graalvm.word.WordBase;

import com.oracle.svm.shared.util.SubstrateUtil;
import com.oracle.svm.shared.Uninterruptible;
Expand Down Expand Up @@ -520,7 +520,35 @@ public static int compareUnsigned(int x, int y) {
}
}

public static class Character {
@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
public static boolean isHighSurrogate(char ch) {
return ch >= java.lang.Character.MIN_HIGH_SURROGATE && ch < (java.lang.Character.MAX_HIGH_SURROGATE + 1);
}

@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
public static boolean isLowSurrogate(char ch) {
return ch >= java.lang.Character.MIN_LOW_SURROGATE && ch < (java.lang.Character.MAX_LOW_SURROGATE + 1);
}

@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
public static boolean isSurrogate(char ch) {
return isHighSurrogate(ch) || isLowSurrogate(ch);
}

@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
public static int toCodePoint(char high, char low) {
return ((high << 10) + low) + (java.lang.Character.MIN_SUPPLEMENTARY_CODE_POINT - (java.lang.Character.MIN_HIGH_SURROGATE << 10) - java.lang.Character.MIN_LOW_SURROGATE);
}

@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
public static int charCount(int codePoint) {
return codePoint >= java.lang.Character.MIN_SUPPLEMENTARY_CODE_POINT ? 2 : 1;
}
}

public static class String {
private static final int MALFORMED_UTF8_REPLACEMENT = '?';

/**
* Gets the number of bytes for a char in modified UTF8 format.
Expand All @@ -531,7 +559,9 @@ private static int modifiedUTF8Length(char c) {
}

/**
* Gets the number of bytes for a char in UTF-8 format.
* Gets the number of bytes for a single UTF-16 code unit in UTF-8 format. This helper does
* not combine surrogate pairs; callers that need code point semantics must use
* {@link #utf8Length(int)} or one of the string-based overloads.
*/
@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
private static int utf8Length(char c) {
Expand All @@ -554,26 +584,6 @@ public static int utf8Length(int codePoint) {
}
}

@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
private static boolean isHighSurrogate(char ch) {
return ch >= Character.MIN_HIGH_SURROGATE && ch < (Character.MAX_HIGH_SURROGATE + 1);
}

@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
private static boolean isLowSurrogate(char ch) {
return ch >= Character.MIN_LOW_SURROGATE && ch < (Character.MAX_LOW_SURROGATE + 1);
}

@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
private static int toCodePoint(char high, char low) {
return ((high << 10) + low) + (Character.MIN_SUPPLEMENTARY_CODE_POINT - (Character.MIN_HIGH_SURROGATE << 10) - Character.MIN_LOW_SURROGATE);
}

@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
public static int charCount(int codePoint) {
return codePoint >= Character.MIN_SUPPLEMENTARY_CODE_POINT ? 2 : 1;
}

/**
* Write a char in modified UTF8 format into the buffer.
*/
Expand All @@ -596,14 +606,6 @@ private static Pointer writeModifiedUTF8(Pointer buffer, char c) {
return pos;
}

/**
* Write a char in UTF-8 format into the buffer.
*/
@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
private static Pointer writeUTF8(Pointer buffer, char c) {
return writeUTF8(buffer, (int) c);
}

/**
* Write a code point in UTF-8 format into the buffer.
*/
Expand Down Expand Up @@ -673,23 +675,9 @@ public static int utf8Length(java.lang.String string, CharReplacer replacer) {
public static int utf8Length(java.lang.String string, int stringLength, CharReplacer replacer) {
int result = 0;
for (int index = 0; index < stringLength;) {
char ch = charAt(string, index);
if (replacer != null) {
ch = replacer.replace(ch);
}
if (isHighSurrogate(ch) && index + 1 < stringLength) {
char low = charAt(string, index + 1);
if (replacer != null) {
low = replacer.replace(low);
}
if (isLowSurrogate(low)) {
result += utf8Length(toCodePoint(ch, low));
index += 2;
continue;
}
}
result += utf8Length(ch);
index++;
int codePoint = utf8CodePointAt(string, index, stringLength, replacer);
result += utf8Length(codePoint);
index += Character.charCount(codePoint);
}
return result;
}
Expand Down Expand Up @@ -749,41 +737,57 @@ public static Pointer toUTF8(java.lang.String string, Pointer buffer, Pointer bu
public static Pointer toUTF8(java.lang.String string, int stringLength, Pointer buffer, Pointer bufferEnd, CharReplacer replacer) {
Pointer pos = buffer;
for (int index = 0; index < stringLength;) {
char ch = charAt(string, index);
if (replacer != null) {
ch = replacer.replace(ch);
}
if (isHighSurrogate(ch) && index + 1 < stringLength) {
char low = charAt(string, index + 1);
if (replacer != null) {
low = replacer.replace(low);
}
if (isLowSurrogate(low)) {
pos = writeUTF8(pos, toCodePoint(ch, low));
index += 2;
continue;
}
}
pos = writeUTF8(pos, ch);
index++;
int codePoint = utf8CodePointAt(string, index, stringLength, replacer);
pos = writeUTF8(pos, codePoint);
index += Character.charCount(codePoint);
}
VMError.guarantee(pos.belowOrEqual(bufferEnd), "Must not write out of bounds.");
return pos;
}

@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
public static int toUTF8UntilLimit(java.lang.String string, Pointer buffer, Pointer bufferEnd, int maxBytes) {
Pointer pos = buffer;
int bytesWritten = 0;
for (int index = 0; index < string.length();) {
int codePoint = utf8CodePointAt(string, index, string.length(), null);
int byteLength = utf8Length(codePoint);
if (maxBytes - bytesWritten < byteLength) {
break;
}
pos = writeUTF8(pos, codePoint);
index += Character.charCount(codePoint);
bytesWritten += byteLength;
}
VMError.guarantee(pos.belowOrEqual(bufferEnd), "Must not write out of bounds.");
return bytesWritten;
}

/**
* Returns the Unicode code point at the given index in the string, combining surrogate
* pairs into a single code point when applicable.
* pairs into a single code point when applicable. Unpaired surrogates are returned as the
* malformed UTF-8 replacement used by the other UTF-8 helpers in this class.
*/
@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
public static int codePointAt(java.lang.String string, int index) {
return utf8CodePointAt(string, index, string.length(), null);
}

@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
private static int utf8CodePointAt(java.lang.String string, int index, int stringLength, CharReplacer replacer) {
char ch = charAt(string, index);
if (isHighSurrogate(ch) && index + 1 < string.length()) {
if (Character.isHighSurrogate(ch) && index + 1 < stringLength) {
char low = charAt(string, index + 1);
if (isLowSurrogate(low)) {
return toCodePoint(ch, low);
if (Character.isLowSurrogate(low)) {
return Character.toCodePoint(ch, low);
}
}
if (replacer != null) {
ch = replacer.replace(ch);
}
if (Character.isSurrogate(ch)) {
return MALFORMED_UTF8_REPLACEMENT;
}
return ch;
}

Expand Down
20 changes: 12 additions & 8 deletions substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/JfrChunkFileWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ public long getChunkStartNanos() {
}

@Override
public void setFilename(String fileToOpen) {
public void setFileToOpen(String fileToOpen) {
assert lock.isOwner();
this.fileToOpen = fileToOpen;
}
Expand Down Expand Up @@ -321,6 +321,7 @@ private byte getAndIncrementGeneration() {
}

private void writeFlushCheckpoint() {
assert lock.isOwner();
long start = beginCheckpointEvent(JfrCheckpointType.Flush);
long poolCountPos = writeCheckpointPoolCountPlaceholder();
int poolCount = newChunk ? writeSerializers() : 0;
Expand All @@ -334,18 +335,20 @@ private void writeFlushCheckpoint() {

@RestrictHeapAccess(access = RestrictHeapAccess.Access.NO_ALLOCATION, reason = "Used on OOME for emergency dumps")
private void writePreviousEpochFlushCheckpoint() {
assert lock.isOwner();
long start = beginCheckpointEvent(JfrCheckpointType.Flush);
long poolCountPos = writeCheckpointPoolCountPlaceholder();
int poolCount = newChunk ? writeSerializers() : 0;
poolCount += stackTraceRepo.write(this, false);
poolCount += methodRepo.write(this, false);
poolCount += oldObjectRepo.write(this, false);
poolCount += typeRepo.writePreviousEpoch(this);
poolCount += typeRepo.writeAndClearPreviousEpoch(this);
poolCount += symbolRepo.write(this, false);
endCheckpointEvent(start, poolCountPos, poolCount);
}

private void writeThreadCheckpoint() {
assert lock.isOwner();
/* The code below is only atomic enough because the epoch can't change while flushing. */
if (SubstrateJVM.getThreadRepo().hasUnflushedData()) {
long start = beginCheckpointEvent(JfrCheckpointType.Threads);
Expand All @@ -357,10 +360,11 @@ private void writeThreadCheckpoint() {

@RestrictHeapAccess(access = RestrictHeapAccess.Access.NO_ALLOCATION, reason = "Used on OOME for emergency dumps")
private void writePreviousEpochThreadCheckpoint() {
assert lock.isOwner();
if (threadRepo.hasUnflushedPreviousEpochData()) {
long start = beginCheckpointEvent(JfrCheckpointType.Threads);
long poolCountPos = writeCheckpointPoolCountPlaceholder();
int poolCount = threadRepo.write(this, false);
int poolCount = threadRepo.writePreviousEpoch(this);
endCheckpointEvent(start, poolCountPos, poolCount);
} else {
threadRepo.clearPreviousEpoch();
Expand Down Expand Up @@ -570,20 +574,20 @@ public void writeString(String str) {
int bufferSize = 64;
Pointer buffer = StackValue.get(bufferSize);
Pointer bufferEnd = buffer.add(bufferSize);
int charsWritten = 0;
int charsProcessed = 0;
UnsignedWord totalBytesWritten = Word.unsigned(0);
while (charsWritten < str.length()) {
while (charsProcessed < str.length()) {
// Fill up the buffer as much as possible
Pointer pos = buffer;
while (charsWritten < str.length()) {
int codePoint = UninterruptibleUtils.String.codePointAt(str, charsWritten);
while (charsProcessed < str.length()) {
int codePoint = UninterruptibleUtils.String.codePointAt(str, charsProcessed);
int nextCharSize = UninterruptibleUtils.String.utf8Length(codePoint);
if (pos.add(nextCharSize).aboveThan(bufferEnd)) {
// buffer is too full to add the next char
break;
}
pos = UninterruptibleUtils.String.writeUTF8(pos, codePoint);
charsWritten += UninterruptibleUtils.String.charCount(codePoint);
charsProcessed += UninterruptibleUtils.Character.charCount(codePoint);
}
// Write the contents of the buffer to disk
UnsignedWord bytesToDisk = pos.subtract(buffer);
Expand Down
2 changes: 1 addition & 1 deletion substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/JfrChunkNoWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public long getChunkStartNanos() {
}

@Override
public void setFilename(String filename) {
public void setFileToOpen(String filename) {
/* Nothing to do. */
}

Expand Down
2 changes: 1 addition & 1 deletion substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/JfrChunkWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public interface JfrChunkWriter extends JfrUnlockedChunkWriter {

long getChunkStartNanos();

void setFilename(String filename);
void setFileToOpen(String filename);

void maybeOpenFile();

Expand Down
6 changes: 3 additions & 3 deletions substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/JfrEmergencyDumpSupport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@

/**
* JFR emergency dumps are snapshots generated when the VM shuts down due to unexpected
* circumstances such as OOME or VM crash. Currently, only dumping on OOME is supported. Emergency
* dumps are a best effort attempt to persist in-flight data and consolidate data in the on-disk JFR
* chunk repository into a snapshot. This process is allocation free.
* circumstances such as OOME or VM crash. Currently, only dumping on OOME-triggered VM shutdown is
* supported. Emergency dumps are a best effort attempt to persist in-flight data and consolidate
* data in the on-disk JFR chunk repository into a snapshot. This process is allocation free.
*/
public interface JfrEmergencyDumpSupport {
@Fold
Expand Down
3 changes: 3 additions & 0 deletions substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/JfrFrameTypeSerializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
import org.graalvm.nativeimage.Platform;
import org.graalvm.nativeimage.Platforms;

import com.oracle.svm.core.heap.RestrictHeapAccess;

/**
* Used to serialize all predefined frame types into the chunk.
*/
Expand All @@ -39,6 +41,7 @@ public JfrFrameTypeSerializer() {
}

@Override
@RestrictHeapAccess(access = RestrictHeapAccess.Access.NO_ALLOCATION, reason = "Used on OOME for emergency dumps")
public void write(JfrChunkWriter writer) {
writer.writeCompressedLong(JfrType.FrameType.getId());
writer.writeCompressedLong(frameTypes.length);
Expand Down
Loading