From 4ae514e036569773a5f3b9fc8f3be349e9c41b0f Mon Sep 17 00:00:00 2001 From: jovanstevanovic Date: Sun, 30 Oct 2022 13:40:41 +0100 Subject: [PATCH] JFR and Sampler code improvements and bug fixes. --- .../posix/PosixSubstrateSigprofHandler.java | 2 +- .../oracle/svm/core/code/CodeInfoAccess.java | 11 ++-- .../svm/core/code/FrameInfoQueryResult.java | 1 + .../svm/core/jfr/JfrStackTraceRepository.java | 51 +++++++++++++------ .../com/oracle/svm/core/jfr/SubstrateJVM.java | 2 + .../core/jfr/events/ExecutionSampleEvent.java | 4 +- .../core/sampler/SamplerBuffersAccess.java | 31 ++++++++--- .../svm/core/sampler/SamplerSampleWriter.java | 44 +++++++++------- .../core/sampler/SamplerStackWalkVisitor.java | 10 ++-- .../core/sampler/SubstrateSigprofHandler.java | 2 +- .../svm/core/thread/PlatformThreads.java | 14 ++++- .../com/oracle/svm/core/thread/VMThreads.java | 23 +++++---- 12 files changed, 127 insertions(+), 68 deletions(-) diff --git a/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/PosixSubstrateSigprofHandler.java b/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/PosixSubstrateSigprofHandler.java index 103787588b26..688aedc41bdc 100644 --- a/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/PosixSubstrateSigprofHandler.java +++ b/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/PosixSubstrateSigprofHandler.java @@ -78,7 +78,7 @@ private static void registerSigprofSignal() { LibC.memset(structSigAction, WordFactory.signed(0), WordFactory.unsigned(structSigActionSize)); /* Register sa_sigaction signal handler */ - structSigAction.sa_flags(Signal.SA_SIGINFO() | Signal.SA_NODEFER()); + structSigAction.sa_flags(Signal.SA_SIGINFO() | Signal.SA_NODEFER() | Signal.SA_RESTART()); structSigAction.sa_sigaction(advancedSignalDispatcher.getFunctionPointer()); Signal.sigaction(Signal.SignalEnum.SIGPROF.getCValue(), structSigAction, WordFactory.nullPointer()); } diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/code/CodeInfoAccess.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/code/CodeInfoAccess.java index 1838b639819a..dc97ffa63669 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/code/CodeInfoAccess.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/code/CodeInfoAccess.java @@ -41,7 +41,6 @@ import com.oracle.svm.core.deopt.SubstrateInstalledCode; import com.oracle.svm.core.graal.stackvalue.UnsafeStackValue; import com.oracle.svm.core.heap.Heap; -import com.oracle.svm.core.heap.RestrictHeapAccess; import com.oracle.svm.core.log.Log; import com.oracle.svm.core.thread.VMOperation; import com.oracle.svm.core.util.VMError; @@ -485,8 +484,8 @@ public SingleShotFrameInfoQueryResultAllocator reload() { return this; } - @RestrictHeapAccess(access = RestrictHeapAccess.Access.NO_ALLOCATION, reason = "Provide allocation-free StackFrameVisitor") @Override + @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) public FrameInfoQueryResult newFrameInfoQueryResult() { if (fired) { return null; @@ -498,26 +497,26 @@ public FrameInfoQueryResult newFrameInfoQueryResult() { } public static class DummyValueInfoAllocator implements ValueInfoAllocator { - @RestrictHeapAccess(access = RestrictHeapAccess.Access.NO_ALLOCATION, reason = "Provide allocation-free StackFrameVisitor") @Override + @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) public FrameInfoQueryResult.ValueInfo newValueInfo() { return null; } - @RestrictHeapAccess(access = RestrictHeapAccess.Access.NO_ALLOCATION, reason = "Provide allocation-free StackFrameVisitor") @Override + @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) public FrameInfoQueryResult.ValueInfo[] newValueInfoArray(int len) { return null; } - @RestrictHeapAccess(access = RestrictHeapAccess.Access.NO_ALLOCATION, reason = "Provide allocation-free StackFrameVisitor") @Override + @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) public FrameInfoQueryResult.ValueInfo[][] newValueInfoArrayArray(int len) { return null; } - @RestrictHeapAccess(access = RestrictHeapAccess.Access.NO_ALLOCATION, reason = "Provide allocation-free StackFrameVisitor") @Override + @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) public void decodeConstant(FrameInfoQueryResult.ValueInfo valueInfo, NonmovableObjectArray frameInfoObjectConstants) { } } diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/code/FrameInfoQueryResult.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/code/FrameInfoQueryResult.java index e394e7ffbb65..bb78af328b8a 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/code/FrameInfoQueryResult.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/code/FrameInfoQueryResult.java @@ -179,6 +179,7 @@ public FrameInfoQueryResult() { init(); } + @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) public void init() { caller = null; deoptMethod = null; diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/JfrStackTraceRepository.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/JfrStackTraceRepository.java index 27725464a2ba..abfd188fa4c6 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/JfrStackTraceRepository.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/JfrStackTraceRepository.java @@ -24,6 +24,7 @@ */ package com.oracle.svm.core.jfr; +import org.graalvm.nativeimage.ImageSingletons; import org.graalvm.nativeimage.Platform; import org.graalvm.nativeimage.Platforms; import org.graalvm.nativeimage.StackValue; @@ -32,6 +33,7 @@ import org.graalvm.nativeimage.c.struct.RawStructure; import org.graalvm.nativeimage.c.struct.SizeOf; import org.graalvm.nativeimage.c.type.CIntPointer; +import org.graalvm.nativeimage.impl.UnmanagedMemorySupport; import org.graalvm.word.Pointer; import org.graalvm.word.UnsignedWord; import org.graalvm.word.WordFactory; @@ -48,8 +50,6 @@ import com.oracle.svm.core.jfr.traceid.JfrTraceIdEpoch; import com.oracle.svm.core.jfr.utils.JfrVisited; import com.oracle.svm.core.locks.VMMutex; -import com.oracle.svm.core.sampler.SamplerBuffer; -import com.oracle.svm.core.sampler.SamplerBufferAccess; import com.oracle.svm.core.sampler.SamplerSampleWriter; import com.oracle.svm.core.sampler.SamplerSampleWriterData; import com.oracle.svm.core.sampler.SamplerSampleWriterDataAccess; @@ -119,7 +119,7 @@ public long getStackTraceId(int skipCount) { Pointer start = data.getStartPos().add(SamplerSampleWriter.getHeaderSize()); stackTraceId = getStackTraceId(start, data.getCurrentPos(), data.getHashCode(), status, false); if (JfrStackTraceTableEntryStatus.get(status, JfrStackTraceTableEntryStatus.NEW)) { - SamplerSampleWriter.end(data); + SamplerSampleWriter.end(data, SamplerSampleWriter.JFR_STACK_TRACE_END); } } finally { releaseLock(); @@ -138,6 +138,7 @@ public long getStackTraceId(Pointer start, Pointer end, int hashCode, CIntPointe UnsignedWord size = end.subtract(start); entry.setHash(hashCode); entry.setSize((int) size.rawValue()); + entry.setSerialized(false); /* Do not copy stacktrace into new entry unless it is necessary. */ entry.setStackTrace(start); @@ -147,15 +148,20 @@ public long getStackTraceId(Pointer start, Pointer end, int hashCode, CIntPointe return result.getId(); } else { /* Replace the previous pointer with new one (entry size and hash remains the same). */ - SamplerBuffer buffer = SamplerBufferAccess.allocate(size); - Pointer to = SamplerBufferAccess.getDataStart(buffer); - entry.setStackTrace(to); - /* Copy the stacktrace into separate native memory entry in hashtable. */ - UnmanagedMemoryUtil.copy(start, to, size); - - JfrStackTraceTableEntry newEntry = (JfrStackTraceTableEntry) epochData.visitedStackTraces.getOrPut(entry); - JfrStackTraceTableEntryStatus.update(newEntry, status, true, false, isSerializationInProgress); - return newEntry.getId(); + Pointer to = ImageSingletons.lookup(UnmanagedMemorySupport.class).malloc(size); + if (to.isNull()) { + /* There is not enough space to allocate a new buffer. */ + JfrStackTraceTableEntryStatus.failStatus(status); + return 0; + } else { + entry.setStackTrace(to); + /* Copy the stacktrace into separate native memory entry in hashtable. */ + UnmanagedMemoryUtil.copy(start, to, size); + + JfrStackTraceTableEntry newEntry = (JfrStackTraceTableEntry) epochData.visitedStackTraces.getOrPut(entry); + JfrStackTraceTableEntryStatus.update(newEntry, status, true, false, isSerializationInProgress); + return newEntry.getId(); + } } } @@ -294,20 +300,35 @@ protected UninterruptibleEntry copyToHeap(UninterruptibleEntry valueOnStack) { result.setId(++nextId); return result; } + + @Override + @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) + protected void free(UninterruptibleEntry entry) { + JfrStackTraceTableEntry stackTraceEntry = (JfrStackTraceTableEntry) entry; + /* The base method will free only the entry itself, not the pointer with stacktrace. */ + ImageSingletons.lookup(UnmanagedMemorySupport.class).free(stackTraceEntry.getStackTrace()); + super.free(entry); + } } public static class JfrStackTraceTableEntryStatus { public static final int NEW = 1; public static final int SHOULD_SERIALIZE = NEW << 1; public static final int SERIALIZED = SHOULD_SERIALIZE << 1; + public static final int FAILED = SERIALIZED << 1; + + @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) + public static void failStatus(CIntPointer status) { + status.write(FAILED); + } @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) public static void update(JfrStackTraceTableEntry entry, CIntPointer status, boolean setNew, boolean isAlreadySerialized, boolean isSerializationInProgress) { - int isRecorded = setNew ? NEW : 0; + int isNew = setNew ? NEW : 0; int shouldSerialize = !isAlreadySerialized ? SHOULD_SERIALIZE : 0; int isSerialized = isAlreadySerialized ? SERIALIZED : 0; - status.write(isRecorded | shouldSerialize | isSerialized); - if (!entry.getSerialized() && isSerializationInProgress) { + status.write(isNew | shouldSerialize | isSerialized); + if (!isAlreadySerialized && isSerializationInProgress) { entry.setSerialized(true); } } diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/SubstrateJVM.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/SubstrateJVM.java index d1ebc70da7fb..ffae9cade3b6 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/SubstrateJVM.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/SubstrateJVM.java @@ -385,6 +385,8 @@ public void setMethodSamplingInterval(long type, long intervalMillis) { if (millis > 0) { SubstrateJVM.get().setEnabled(type, true); + /* Stacktrace walk is disabled by default for ExecutionSample. */ + SubstrateJVM.get().setStackTraceEnabled(type, true); } else { millis = 0; } diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/events/ExecutionSampleEvent.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/events/ExecutionSampleEvent.java index cddcb8748cab..6ee9ea35a2e8 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/events/ExecutionSampleEvent.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/events/ExecutionSampleEvent.java @@ -50,7 +50,7 @@ public final class ExecutionSampleEvent { @Uninterruptible(reason = "Called from uninterruptible code.", calleeMustBe = false) public static void tryToRegisterExecutionSampleEventCallback() { - if (SubstrateJVM.get().isEnabled(JfrEvent.ExecutionSample) && intervalMillis > 0) { + if (intervalMillis > 0) { ImageSingletons.lookup(ThreadingSupport.class).registerRecurringCallback(intervalMillis, TimeUnit.MILLISECONDS, CALLBACK); } } @@ -83,7 +83,7 @@ public void run(Threading.RecurringCallbackAccess access) { ExecutionSampleEvent.writeExecutionSample( JfrTicks.elapsedTicks(), SubstrateJVM.getThreadId(isolateThread), - SubstrateJVM.get().getStackTraceId(JfrEvent.ExecutionSample.getId(), 0), + SubstrateJVM.get().getStackTraceId(JfrEvent.ExecutionSample, 0), JfrThreadState.getId(javaThread.getState())); } } diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerBuffersAccess.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerBuffersAccess.java index 4f271398f32b..9d91109d0a08 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerBuffersAccess.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerBuffersAccess.java @@ -25,6 +25,10 @@ package com.oracle.svm.core.sampler; +import static com.oracle.svm.core.jfr.JfrStackTraceRepository.JfrStackTraceTableEntryStatus.FAILED; +import static com.oracle.svm.core.jfr.JfrStackTraceRepository.JfrStackTraceTableEntryStatus.SERIALIZED; +import static com.oracle.svm.core.jfr.JfrStackTraceRepository.JfrStackTraceTableEntryStatus.SHOULD_SERIALIZE; + import org.graalvm.nativeimage.StackValue; import org.graalvm.nativeimage.c.function.CodePointer; import org.graalvm.nativeimage.c.type.CIntPointer; @@ -119,18 +123,30 @@ public static void processSamplerBuffer(SamplerBuffer buffer) { stackTraceRepo.acquireLock(); try { long stackTraceId = stackTraceRepo.getStackTraceId(current, current.add(sampleSize), sampleHash, status, true); - if (JfrStackTraceRepository.JfrStackTraceTableEntryStatus.get(status, JfrStackTraceRepository.JfrStackTraceTableEntryStatus.SERIALIZED)) { - ExecutionSampleEvent.writeExecutionSample(sampleTick, buffer.getOwner(), stackTraceId, threadState); - /* Sample is already there, skip the rest of sample plus END_MARK symbol. */ - current = current.add(sampleSize).add(SamplerSampleWriter.END_MARKER_SIZE); + boolean serialized = JfrStackTraceRepository.JfrStackTraceTableEntryStatus.get(status, SERIALIZED); + boolean failed = JfrStackTraceRepository.JfrStackTraceTableEntryStatus.get(status, FAILED); + if (serialized || failed) { + /* + * Sample/Stack is already there or there is not enough memory to operate, skip + * the rest of the data. + */ + current = current.add(sampleSize); + long endMarker = current.readLong(0); + if (endMarker == SamplerSampleWriter.SAMPLE_EVENT_DATA_END && serialized) { + ExecutionSampleEvent.writeExecutionSample(sampleTick, buffer.getOwner(), stackTraceId, threadState); + } + current = current.add(SamplerSampleWriter.END_MARKER_SIZE); } else { - assert JfrStackTraceRepository.JfrStackTraceTableEntryStatus.get(status, JfrStackTraceRepository.JfrStackTraceTableEntryStatus.SHOULD_SERIALIZE); + assert JfrStackTraceRepository.JfrStackTraceTableEntryStatus.get(status, SHOULD_SERIALIZE); /* Sample is not there. Start walking a stacktrace. */ stackTraceRepo.serializeStackTraceHeader(stackTraceId, isTruncated, sampleSize / SamplerSampleWriter.IP_SIZE); while (current.belowThan(end)) { long ip = current.readLong(0); - if (ip == SamplerSampleWriter.END_MARKER) { - ExecutionSampleEvent.writeExecutionSample(sampleTick, buffer.getOwner(), stackTraceId, threadState); + /* Check if we hit any of the end markers. */ + if (ip == SamplerSampleWriter.JFR_STACK_TRACE_END || ip == SamplerSampleWriter.SAMPLE_EVENT_DATA_END) { + if (ip == SamplerSampleWriter.SAMPLE_EVENT_DATA_END) { + ExecutionSampleEvent.writeExecutionSample(sampleTick, buffer.getOwner(), stackTraceId, threadState); + } current = current.add(SamplerSampleWriter.END_MARKER_SIZE); break; } else { @@ -143,6 +159,7 @@ public static void processSamplerBuffer(SamplerBuffer buffer) { stackTraceRepo.releaseLock(); } } + SamplerBufferAccess.reinitialize(buffer); } @Uninterruptible(reason = "The handle should only be accessed from uninterruptible code to prevent that the GC frees the CodeInfo.", callerMustBe = true) diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerSampleWriter.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerSampleWriter.java index fa508c5fbd57..019385dd28f5 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerSampleWriter.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerSampleWriter.java @@ -38,7 +38,8 @@ public final class SamplerSampleWriter { - public static final long END_MARKER = -1; + public static final long JFR_STACK_TRACE_END = -1; + public static final long SAMPLE_EVENT_DATA_END = -2; public static final int IP_SIZE = Long.BYTES; public static final int END_MARKER_SIZE = Long.BYTES; @@ -51,7 +52,7 @@ public static int getHeaderSize() { return Integer.BYTES + Integer.BYTES + Integer.BYTES + Long.BYTES + Long.BYTES; } - @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) + @Uninterruptible(reason = "Accesses a sampler buffer.", callerMustBe = true) public static void begin(SamplerSampleWriterData data) { assert isValid(data); assert getUncommittedSize(data).equal(0); @@ -68,13 +69,18 @@ public static void begin(SamplerSampleWriterData data) { SamplerSampleWriter.putLong(data, JfrThreadState.getId(Thread.State.RUNNABLE)); } - @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) - public static void end(SamplerSampleWriterData data) { + @Uninterruptible(reason = "Accesses a sampler buffer.", callerMustBe = true) + public static void end(SamplerSampleWriterData data, long endMarker) { assert isValid(data); UnsignedWord sampleSize = getSampleSize(data); - /* Put end marker. */ - putUncheckedLong(data, END_MARKER); + /* + * Put end marker. + * + * We need to distinguish between end of a sample and end of a regular stack walk (see + * com.oracle.svm.core.sampler.SamplerBuffersAccess.processSamplerBuffer) + */ + putUncheckedLong(data, endMarker); Pointer currentPos = data.getCurrentPos(); data.setCurrentPos(data.getStartPos()); @@ -89,7 +95,7 @@ public static void end(SamplerSampleWriterData data) { commit(data); } - @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) + @Uninterruptible(reason = "Accesses a sampler buffer.", callerMustBe = true) public static boolean putLong(SamplerSampleWriterData data, long value) { if (ensureSize(data, Long.BYTES)) { putUncheckedLong(data, value); @@ -99,7 +105,7 @@ public static boolean putLong(SamplerSampleWriterData data, long value) { } } - @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) + @Uninterruptible(reason = "Accesses a sampler buffer.", callerMustBe = true) private static void putUncheckedLong(SamplerSampleWriterData data, long value) { /* This method is only called if ensureSize() succeeded earlier. */ assert getAvailableSize(data).aboveOrEqual(Long.BYTES); @@ -107,7 +113,7 @@ private static void putUncheckedLong(SamplerSampleWriterData data, long value) { increaseCurrentPos(data, WordFactory.unsigned(Long.BYTES)); } - @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) + @Uninterruptible(reason = "Accesses a sampler buffer.", callerMustBe = true) public static boolean putInt(SamplerSampleWriterData data, int value) { if (ensureSize(data, Integer.BYTES)) { putUncheckedInt(data, value); @@ -117,7 +123,7 @@ public static boolean putInt(SamplerSampleWriterData data, int value) { } } - @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) + @Uninterruptible(reason = "Accesses a sampler buffer.", callerMustBe = true) private static void putUncheckedInt(SamplerSampleWriterData data, int value) { /* This method is only called if ensureSize() succeeded earlier. */ assert getAvailableSize(data).aboveOrEqual(Integer.BYTES); @@ -125,7 +131,7 @@ private static void putUncheckedInt(SamplerSampleWriterData data, int value) { increaseCurrentPos(data, WordFactory.unsigned(Integer.BYTES)); } - @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) + @Uninterruptible(reason = "Accesses a sampler buffer.", callerMustBe = true) private static void commit(SamplerSampleWriterData data) { SamplerBuffer buffer = data.getSamplerBuffer(); assert isValid(data); @@ -135,7 +141,7 @@ private static void commit(SamplerSampleWriterData data) { buffer.setPos(data.getCurrentPos()); } - @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) + @Uninterruptible(reason = "Accesses a sampler buffer.", callerMustBe = true) private static boolean ensureSize(SamplerSampleWriterData data, int requested) { assert requested > 0; if (!isValid(data)) { @@ -153,7 +159,7 @@ private static boolean ensureSize(SamplerSampleWriterData data, int requested) { return true; } - @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) + @Uninterruptible(reason = "Accesses a sampler buffer.", callerMustBe = true) private static boolean accommodate(SamplerSampleWriterData data, UnsignedWord uncommitted) { if (SamplerBufferAccess.isEmpty(data.getSamplerBuffer())) { /* @@ -188,7 +194,7 @@ private static boolean accommodate(SamplerSampleWriterData data, UnsignedWord un return true; } - @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) + @Uninterruptible(reason = "Accesses a sampler buffer.", callerMustBe = true) private static void reset(SamplerSampleWriterData data) { SamplerBuffer buffer = data.getSamplerBuffer(); data.setStartPos(buffer.getPos()); @@ -196,27 +202,27 @@ private static void reset(SamplerSampleWriterData data) { data.setEndPos(SamplerBufferAccess.getDataEnd(buffer)); } - @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) + @Uninterruptible(reason = "Accesses a sampler buffer.", callerMustBe = true) private static boolean isValid(SamplerSampleWriterData data) { return data.getEndPos().isNonNull(); } - @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) + @Uninterruptible(reason = "Accesses a sampler buffer.", callerMustBe = true) private static UnsignedWord getAvailableSize(SamplerSampleWriterData data) { return data.getEndPos().subtract(data.getCurrentPos()); } - @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) + @Uninterruptible(reason = "Accesses a sampler buffer.", callerMustBe = true) private static UnsignedWord getUncommittedSize(SamplerSampleWriterData data) { return data.getCurrentPos().subtract(data.getStartPos()); } - @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) + @Uninterruptible(reason = "Accesses a sampler buffer.", callerMustBe = true) private static UnsignedWord getSampleSize(SamplerSampleWriterData data) { return getUncommittedSize(data).subtract(getHeaderSize()); } - @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) + @Uninterruptible(reason = "Accesses a sampler buffer.", callerMustBe = true) private static void increaseCurrentPos(SamplerSampleWriterData data, UnsignedWord delta) { data.setCurrentPos(data.getCurrentPos().add(delta)); } diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerStackWalkVisitor.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerStackWalkVisitor.java index 9d94b4252868..14847cda349a 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerStackWalkVisitor.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerStackWalkVisitor.java @@ -33,10 +33,10 @@ import com.oracle.svm.core.deopt.DeoptimizedFrame; import com.oracle.svm.core.stack.ParameterizedStackFrameVisitor; -final class SamplerStackWalkVisitor extends ParameterizedStackFrameVisitor { +final class SamplerStackWalkVisitor extends ParameterizedStackFrameVisitor { @Override - @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) - protected boolean visitFrame(Pointer sp, CodePointer ip, CodeInfo codeInfo, DeoptimizedFrame deoptimizedFrame, Void voidData) { + @Uninterruptible(reason = "The method executes during signal handling.", callerMustBe = true) + protected boolean visitFrame(Pointer sp, CodePointer ip, CodeInfo codeInfo, DeoptimizedFrame deoptimizedFrame, Object data) { SamplerSampleWriterData writerData = SamplerThreadLocal.getWriterData(); boolean shouldSkipFrame = shouldSkipFrame(writerData); boolean shouldContinueWalk = shouldContinueWalk(writerData); @@ -71,8 +71,8 @@ private static int computeHash(int oldHash, long ip) { } @Override - @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) - protected boolean unknownFrame(Pointer sp, CodePointer ip, DeoptimizedFrame deoptimizedFrame, Void data) { + @Uninterruptible(reason = "The method executes during signal handling.", callerMustBe = true) + protected boolean unknownFrame(Pointer sp, CodePointer ip, DeoptimizedFrame deoptimizedFrame, Object data) { SamplerThreadLocal.increaseUnparseableStacks(); return false; } diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SubstrateSigprofHandler.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SubstrateSigprofHandler.java index 9bc7891a2724..a46afbbb5452 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SubstrateSigprofHandler.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SubstrateSigprofHandler.java @@ -344,7 +344,7 @@ protected static void doUninterruptibleStackWalk(RegisterDumper.Context uContext * walk was interrupted because stack size exceeded given depth. */ if (JavaStackWalker.walkCurrentThread(sp, ip, visitor()) || data.getTruncated()) { - SamplerSampleWriter.end(data); + SamplerSampleWriter.end(data, SamplerSampleWriter.SAMPLE_EVENT_DATA_END); } } } diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/PlatformThreads.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/PlatformThreads.java index 490d3dcbde49..eca2163afefb 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/PlatformThreads.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/PlatformThreads.java @@ -300,13 +300,22 @@ public static IsolateThread getIsolateThread(Thread t) { } /** Before detaching a thread, run any Java cleanup code. */ - static void cleanupBeforeDetach(IsolateThread thread) { + static void threadExit(IsolateThread thread) { VMError.guarantee(thread.equal(CurrentIsolate.getCurrentThread()), "Cleanup must execute in detaching thread"); Thread javaThread = currentThread.get(thread); if (javaThread != null) { toTarget(javaThread).exit(); - ThreadListenerSupport.get().afterThreadExit(CurrentIsolate.getCurrentThread(), javaThread); + } + } + + @Uninterruptible(reason = "Only uninterruptible code may be executed after Thread.exit.") + static void afterThreadExit(IsolateThread thread) { + VMError.guarantee(thread.equal(CurrentIsolate.getCurrentThread()), "Cleanup must execute in detaching thread"); + + Thread javaThread = currentThread.get(thread); + if (javaThread != null) { + ThreadListenerSupport.get().afterThreadExit(thread, javaThread); } } @@ -678,6 +687,7 @@ private static boolean isApplicationThread(IsolateThread isolateThread) { return !VMOperationControl.isDedicatedVMOperationThread(isolateThread); } + /** This method should only be used to exit the main thread. */ @SuppressFBWarnings(value = "NN", justification = "notifyAll is necessary for Java semantics, no shared state needs to be modified beforehand") public static void exit(Thread thread) { /* diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/VMThreads.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/VMThreads.java index 6f147454dd7a..dc33f44de177 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/VMThreads.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/VMThreads.java @@ -42,8 +42,8 @@ import org.graalvm.word.WordFactory; import com.oracle.svm.core.IsolateListenerSupport; -import com.oracle.svm.core.SubstrateOptions; import com.oracle.svm.core.NeverInline; +import com.oracle.svm.core.SubstrateOptions; import com.oracle.svm.core.Uninterruptible; import com.oracle.svm.core.c.function.CEntryPointErrors; import com.oracle.svm.core.c.function.CFunctionOptions; @@ -338,11 +338,14 @@ public void detachThread(IsolateThread thread) { nextOsThreadToCleanup = OSThreadHandleTL.get(thread); } - cleanupBeforeDetach(thread); + exit(thread); + /* Only uninterruptible code may be executed from now on. */ + PlatformThreads.afterThreadExit(thread); - // From this point on, all code must be fully uninterruptible because this thread either - // holds the THREAD_MUTEX (see the JavaDoc on THREAD_MUTEX) or because the IsolateThread was - // already freed. + /* + * Here, all code is uninterruptible because this thread either holds the THREAD_MUTEX (see + * the JavaDoc on THREAD_MUTEX) or because the IsolateThread was already freed. + */ lockThreadMutexInNativeCode(); OSThreadHandle threadToCleanup; try { @@ -466,17 +469,17 @@ private void waitUntilLastOsThreadExited() { } @Uninterruptible(reason = "Called from uninterruptible code, but still safe at this point.", calleeMustBe = false) - private static void cleanupBeforeDetach(IsolateThread thread) { - PlatformThreads.cleanupBeforeDetach(thread); + private static void exit(IsolateThread thread) { + PlatformThreads.threadExit(thread); } /** * Detaches all manually attached native threads, but not those threads that were launched from * Java, which must be notified to individually exit in the immediately following tear-down. * - * We cannot {@linkplain #cleanupBeforeDetach clean up} the threads we detach here because - * cleanup code needs to run in the detaching thread itself. We assume that this is tolerable - * considering the immediately following tear-down. + * We cannot clean up the threads we detach here because cleanup code needs to run in the + * detaching thread itself. We assume that this is tolerable considering the immediately + * following tear-down. */ public static void detachAllThreadsExceptCurrentWithoutCleanupForTearDown() { DetachAllThreadsExceptCurrentOperation vmOp = new DetachAllThreadsExceptCurrentOperation();