Skip to content

Commit b7af1f9

Browse files
jovanstevanovicjovanstevanovic
committed
Changes according to comments.
1 parent fd23532 commit b7af1f9

File tree

8 files changed

+70
-70
lines changed

8 files changed

+70
-70
lines changed

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerBuffer.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,13 @@
3939
interface SamplerBuffer extends PointerBase {
4040

4141
/**
42-
* Returns a buffer that is next in the {@link SamplerBufferStack}, otherwise null.
42+
* Returns the buffer that is next in the {@link SamplerBufferStack}, otherwise null.
4343
*/
4444
@RawField
4545
SamplerBuffer getNext();
4646

4747
/**
48-
* Sets a buffer as a new head in the {@link SamplerBufferStack}.
48+
* Sets the successor to this node in the {@link SamplerBufferStack}.
4949
*/
5050
@RawField
5151
void setNext(SamplerBuffer buffer);

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerBufferPool.java‎

Lines changed: 40 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -41,90 +41,89 @@ class SamplerBufferPool {
4141

4242
private static final long THREAD_BUFFER_SIZE = Options.getThreadBufferSize();
4343

44-
private static final VMMutex mutex = new VMMutex("profilerBufferUtils");
44+
private static final VMMutex mutex = new VMMutex("SamplerBufferPool");
4545

4646
private static long bufferCount;
4747

4848
@Uninterruptible(reason = "Locking without transition requires that the whole critical section is uninterruptible.", mayBeInlined = true)
49-
public static void adjustBufferCount(SamplerBuffer threadLocalBuffer) {
50-
mutex.lockNoTransition();
51-
try {
52-
releaseThreadLocalBuffer(threadLocalBuffer);
53-
adjustBufferCount0();
54-
} finally {
55-
mutex.unlock();
56-
}
49+
public static void releaseBufferAndAdjustCount(SamplerBuffer threadLocalBuffer) {
50+
adjustBufferCount0(threadLocalBuffer);
5751
}
5852

5953
@Uninterruptible(reason = "Locking without transition requires that the whole critical section is uninterruptible.", mayBeInlined = true)
6054
public static void adjustBufferCount() {
61-
mutex.lockNoTransition();
62-
try {
63-
adjustBufferCount0();
64-
} finally {
65-
mutex.unlock();
66-
}
55+
adjustBufferCount0(WordFactory.nullPointer());
6756
}
6857

6958
@Uninterruptible(reason = "Locking without transition requires that the whole critical section is uninterruptible.", mayBeInlined = true)
70-
public static void adjustBufferCount0() {
71-
long diff = diff();
72-
if (diff > 0) {
73-
for (int i = 0; i < diff; i++) {
74-
allocateAndPush();
75-
}
76-
} else {
77-
for (long i = diff; i < 0; i++) {
78-
if (!popAndFree()) {
79-
break;
59+
private static void adjustBufferCount0(SamplerBuffer threadLocalBuffer) {
60+
mutex.lockNoTransition();
61+
try {
62+
releaseThreadLocalBuffer(threadLocalBuffer);
63+
long diff = diff();
64+
if (diff > 0) {
65+
for (int i = 0; i < diff; i++) {
66+
allocateAndPush();
67+
}
68+
} else {
69+
for (long i = diff; i < 0; i++) {
70+
if (!popAndFree()) {
71+
break;
72+
}
8073
}
8174
}
75+
} finally {
76+
mutex.unlock();
8277
}
8378
}
8479

85-
@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
86-
private static void allocateAndPush() {
87-
VMError.guarantee(bufferCount >= 0);
88-
SamplerBuffer buffer = SamplerBufferAccess.allocate(WordFactory.unsigned(THREAD_BUFFER_SIZE));
89-
if (buffer.isNull()) {
90-
return;
91-
}
92-
SubstrateSigprofHandler.availableBuffers().pushBuffer(buffer);
93-
bufferCount++;
94-
}
95-
9680
@Uninterruptible(reason = "Locking without transition requires that the whole critical section is uninterruptible.", mayBeInlined = true)
9781
private static void releaseThreadLocalBuffer(SamplerBuffer buffer) {
98-
/* buffer will be null if no stack walk were performed. */
82+
/*
83+
* buffer is null if the thread is not running yet, or we did not perform the stack walk for
84+
* this thread during the run.
85+
*/
9986
if (buffer.isNonNull()) {
10087
if (SamplerBufferAccess.isEmpty(buffer)) {
10188
/* We can free it right away. */
10289
SamplerBufferAccess.free(buffer);
10390
} else {
10491
/* Put it in the stack with other unprocessed buffers. */
10592
buffer.setFreeable(true);
106-
SubstrateSigprofHandler.fullBuffers().pushBuffer(buffer);
93+
SubstrateSigprofHandler.singleton().fullBuffers().pushBuffer(buffer);
10794
}
10895
VMError.guarantee(bufferCount > 0);
10996
bufferCount--;
11097
}
11198
}
11299

100+
@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
101+
private static void allocateAndPush() {
102+
VMError.guarantee(bufferCount >= 0);
103+
SamplerBuffer buffer = SamplerBufferAccess.allocate(WordFactory.unsigned(THREAD_BUFFER_SIZE));
104+
if (buffer.isNull()) {
105+
return;
106+
}
107+
SubstrateSigprofHandler.singleton().availableBuffers().pushBuffer(buffer);
108+
bufferCount++;
109+
}
110+
113111
@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
114112
private static boolean popAndFree() {
115113
VMError.guarantee(bufferCount > 0);
116-
SamplerBuffer buffer = SubstrateSigprofHandler.availableBuffers().popBuffer();
114+
SamplerBuffer buffer = SubstrateSigprofHandler.singleton().availableBuffers().popBuffer();
117115
if (buffer.isNonNull()) {
118116
SamplerBufferAccess.free(buffer);
119117
bufferCount--;
118+
return true;
119+
} else {
120120
return false;
121121
}
122-
return true;
123122
}
124123

125124
@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
126125
private static long diff() {
127-
double diffD = SubstrateSigprofHandler.getSubstrateThreadMXBean().getThreadCount() * 1.5 - bufferCount;
126+
double diffD = SubstrateSigprofHandler.singleton().substrateThreadMXBean().getThreadCount() * 1.5 - bufferCount;
128127
return (long) (diffD + 0.5);
129128
}
130129
}

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerBufferStack.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ class SamplerBufferStack {
5050
}
5151

5252
/**
53-
* Push a profiler buffer into the global linked list.
53+
* Push the buffer into the linked-list.
5454
*/
5555
@Uninterruptible(reason = "Locking without transition requires that the whole critical section is uninterruptible.")
5656
public void pushBuffer(SamplerBuffer buffer) {
@@ -64,7 +64,7 @@ public void pushBuffer(SamplerBuffer buffer) {
6464
}
6565

6666
/**
67-
* Pop a profiler buffer from the global linked list. Returns {@code null} if the list is empty.
67+
* Pop the buffer from the linked-list. Returns {@code null} if the list is empty.
6868
*/
6969
@Uninterruptible(reason = "Locking without transition requires that the whole critical section is uninterruptible.")
7070
public SamplerBuffer popBuffer() {
@@ -81,9 +81,6 @@ public SamplerBuffer popBuffer() {
8181
}
8282
}
8383

84-
/**
85-
* Returns the lock that this stack is using.
86-
*/
8784
@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
8885
public boolean isLockedByCurrentThread() {
8986
return spinLock.isOwner();

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerIsolateLocal.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,12 @@ public void afterCreateIsolate(Isolate isolate) {
5858
@Override
5959
@Uninterruptible(reason = "The isolate teardown is in progress.")
6060
public void onIsolateTeardown() {
61-
if (SubstrateSigprofHandler.isProfilingEnabled() && isKeySet()) {
61+
if (SubstrateSigprofHandler.singleton().isProfilingEnabled() && isKeySet()) {
6262
/* Invalidate the isolate-specific key. */
6363
UnsignedWord oldKey = key;
6464
key = WordFactory.unsigned(0);
6565

66-
/* We need to manually call teardown because isKeySet will now return false. */
66+
/* Manually disable sampling for the current thread (no other threads are remaining). */
6767
SamplerThreadLocal.teardown(CurrentIsolate.getCurrentThread());
6868

6969
/* Now, it's safe to delete the isolate-specific key. */

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerSampleWriter.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,14 @@
3232
import com.oracle.svm.core.annotate.Uninterruptible;
3333
import com.oracle.svm.core.util.VMError;
3434

35-
class SamplerSampleWriter {
35+
final class SamplerSampleWriter {
3636

3737
private static final int END_MARKER_SIZE = Long.BYTES;
3838
private static final long END_MARKER = -1;
3939

40+
private SamplerSampleWriter() {
41+
}
42+
4043
@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
4144
public static boolean putLong(SamplerSampleWriterData data, long value) {
4245
if (ensureSize(data, Long.BYTES)) {
@@ -86,7 +89,7 @@ private static boolean accommodate(SamplerSampleWriterData data, UnsignedWord un
8689
}
8790

8891
/* Pop first free buffer from the pool. */
89-
SamplerBuffer newBuffer = SubstrateSigprofHandler.availableBuffers().popBuffer();
92+
SamplerBuffer newBuffer = SubstrateSigprofHandler.singleton().availableBuffers().popBuffer();
9093
if (newBuffer.isNull()) {
9194
/* No available buffers on the pool. Fallback! */
9295
SamplerThreadLocal.increaseMissedSamples();
@@ -99,7 +102,7 @@ private static boolean accommodate(SamplerSampleWriterData data, UnsignedWord un
99102

100103
/* Put in the stack with other unprocessed buffers. */
101104
SamplerBuffer oldBuffer = data.getSamplerBuffer();
102-
SubstrateSigprofHandler.fullBuffers().pushBuffer(oldBuffer);
105+
SubstrateSigprofHandler.singleton().fullBuffers().pushBuffer(oldBuffer);
103106

104107
/* Reinitialize data structure. */
105108
data.setSamplerBuffer(newBuffer);

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerSpinLock.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525

2626
package com.oracle.svm.core.sampler;
2727

28-
import com.oracle.svm.core.util.VMError;
2928
import org.graalvm.compiler.nodes.PauseNode;
3029
import org.graalvm.nativeimage.CurrentIsolate;
3130
import org.graalvm.nativeimage.IsolateThread;
@@ -35,13 +34,15 @@
3534

3635
import com.oracle.svm.core.annotate.Uninterruptible;
3736
import com.oracle.svm.core.jdk.UninterruptibleUtils;
37+
import com.oracle.svm.core.util.VMError;
3838

3939
/**
4040
* The custom implementation of spin lock that is async signal safe.
4141
*
4242
* In some specific situations, the signal handler can interrupt execution while the same thread
43-
* already has the lock. Other spin lock implementations can deadlock in such a case. So it is
44-
* essential to check if the current thread is the owner of the lock, before acquiring it.
43+
* already has the lock. This implementation will check and fatally fail while other spin locks
44+
* implementations can deadlock in this case. So it is essential to check if the current thread is
45+
* the owner of the lock, before acquiring it.
4546
*/
4647
class SamplerSpinLock {
4748
private final UninterruptibleUtils.AtomicPointer<IsolateThread> owner;

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerThreadLocal.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ class SamplerThreadLocal implements ThreadListener {
5050
@Override
5151
@Uninterruptible(reason = "Only uninterruptible code may be executed before Thread.run.")
5252
public void beforeThreadRun(IsolateThread isolateThread, Thread javaThread) {
53-
if (SubstrateSigprofHandler.isProfilingEnabled()) {
53+
if (SubstrateSigprofHandler.singleton().isProfilingEnabled()) {
5454
initialize(isolateThread);
5555
}
5656
}
@@ -75,7 +75,7 @@ static void initialize(IsolateThread isolateThread) {
7575
@Override
7676
@Uninterruptible(reason = "Only uninterruptible code may be executed after Thread.exit.")
7777
public void afterThreadExit(IsolateThread isolateThread, Thread javaThread) {
78-
if (SubstrateSigprofHandler.isProfilingEnabled() && SamplerIsolateLocal.isKeySet()) {
78+
if (SubstrateSigprofHandler.singleton().isProfilingEnabled() && SamplerIsolateLocal.isKeySet()) {
7979
teardown(isolateThread);
8080
}
8181
}
@@ -94,7 +94,7 @@ static void teardown(IsolateThread isolateThread) {
9494

9595
/* Adjust the number of buffers (including the thread-local buffer). */
9696
SamplerBuffer threadLocalBuffer = localBuffer.get(isolateThread);
97-
SamplerBufferPool.adjustBufferCount(threadLocalBuffer);
97+
SamplerBufferPool.releaseBufferAndAdjustCount(threadLocalBuffer);
9898
localBuffer.set(isolateThread, WordFactory.nullPointer());
9999
}
100100

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SubstrateSigprofHandler.java‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -185,23 +185,23 @@ static boolean isProfilingSupported() {
185185
}
186186

187187
@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
188-
static boolean isProfilingEnabled() {
189-
return singleton().enabled;
188+
boolean isProfilingEnabled() {
189+
return enabled;
190190
}
191191

192192
@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
193-
static SamplerBufferStack availableBuffers() {
194-
return singleton().availableBuffers;
193+
SamplerBufferStack availableBuffers() {
194+
return availableBuffers;
195195
}
196196

197197
@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
198-
static SamplerBufferStack fullBuffers() {
199-
return singleton().fullBuffers;
198+
SamplerBufferStack fullBuffers() {
199+
return fullBuffers;
200200
}
201201

202202
@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
203-
static SubstrateThreadMXBean getSubstrateThreadMXBean() {
204-
return singleton().threadMXBean;
203+
SubstrateThreadMXBean substrateThreadMXBean() {
204+
return threadMXBean;
205205
}
206206

207207
/**
@@ -287,7 +287,7 @@ private static void doUninterruptibleStackWalk(RegisterDumper.Context uContext,
287287

288288
@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
289289
private static boolean prepareStackWalk(SamplerSampleWriterData data) {
290-
if (availableBuffers().isLockedByCurrentThread() || fullBuffers().isLockedByCurrentThread()) {
290+
if (singleton().availableBuffers().isLockedByCurrentThread() || singleton().fullBuffers().isLockedByCurrentThread()) {
291291
/*
292292
* The current thread already holds the stack lock, so we can't access it. It's way
293293
* better to lose one sample, then potentially the whole buffer.
@@ -299,7 +299,7 @@ private static boolean prepareStackWalk(SamplerSampleWriterData data) {
299299
SamplerBuffer buffer = SamplerThreadLocal.getThreadLocalBuffer();
300300
if (buffer.isNull()) {
301301
/* Pop first free buffer from the pool. */
302-
buffer = availableBuffers().popBuffer();
302+
buffer = singleton().availableBuffers().popBuffer();
303303
if (buffer.isNull()) {
304304
/* No available buffers on the pool. Fallback! */
305305
SamplerThreadLocal.increaseMissedSamples();
@@ -369,13 +369,13 @@ protected void operate() {
369369
*/
370370
@Uninterruptible(reason = "Prevent pollution of the current thread's thread local JFR buffer.")
371371
private void initialize() {
372-
enabled = true;
373372
/*
374373
* Iterate all over all thread and initialize the thread-local storage of each thread.
375374
*/
376375
for (IsolateThread thread = VMThreads.firstThread(); thread.isNonNull(); thread = VMThreads.nextThread(thread)) {
377376
SamplerThreadLocal.initialize(thread);
378377
}
378+
enabled = true;
379379
}
380380
}
381381
}

0 commit comments

Comments
 (0)