Serialize JFR stack traces during flushpoints #6365

roberttoyonaga · 2023-04-04T16:50:57Z

Summary of Changes
This PR adds serialization of JFR stacktraces at flushpoints. It uses a similar approach to how JFR local buffers are processed at flushpoints.

Since the SamplerBuffers and JfrBuffers are handled similarly, I've also introduced Buffer, BufferNode, and BufferList classes and done some refactoring to try and reduce code duplication.
I've also moved most of the stacktrace serialization code into the JfrStacktraceRepository. This makes is simpler to make the processing atomic.
serializedPos pointer was introduced in SamplerBuffer to avoid races between processing the active buffers and writing to them.
Doing safepoint checks was removed from SamplerBuffersAccess.processFullBuffers() because now the JfrChunkwriter lock must be held while doing the processing.

Please see related issue here: #6226

remove clear repository methods. More refactoring more simplification BufferList. Update javadoc clean up

gate fixes style

oubidar-Abderrahim · 2023-04-11T12:03:00Z

Hi @roberttoyonaga, Thank you for opening this PR
@christianhaeubl could you please review this PR or assign someone to do it? Thank you.

christianhaeubl

Thanks for your work, I added a few comments.

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/BufferNodeAccess.java

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/JfrBufferList.java

christianhaeubl · 2023-04-13T14:32:10Z

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/JfrChunkWriter.java

+     */
+    @Uninterruptible(reason = "Prevent JFR recording.")
+    private static void processSamplerBuffers(boolean flushpoint) {
+        SamplerBuffersAccess.processActiveBuffers(flushpoint);


You removed the calls to JfrExecutionSampler.singleton().disallowThreadsInSamplerCode() and JfrExecutionSampler.singleton().allowThreadsInSamplerCode(). So, the SIGPROF-based sampler can interrupt processActiveBuffers and processFullBuffers at any time. When the signal handler modifies the stacks of available and full buffers, this can cause issues that look like races (even though everything happens in the same thread).

See the JavaDoc on processSamplerBuffers().

Oh right, the sampler could also affect the buffer stacks. I've added JfrExecutionSampler.singleton().disallowThreadsInSamplerCode() and JfrExecutionSampler.singleton().allowThreadsInSamplerCode() back in, and I've corrected the Javadoc as well.

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerBuffersAccess.java

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerBufferList.java

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/BufferNodeAccess.java

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/JfrStackTraceRepository.java

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/JfrThreadLocal.java

christianhaeubl · 2023-04-24T15:45:30Z

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

+        BufferNode oldNode = oldBuffer.getNode();
+
+        // Lock here to avoid race with flushing thread which might be processing
+        BufferNodeAccess.lockNoTransition(oldNode);


Actually, I don't think that we need any BufferNode-based locking for SamplerBuffers:

the buffers have a fixed size and are never resized

in-use/full buffers are only processed by threads that hold the JfrChunkWriter lock

only the SamplerBufferPool frees buffers

So, the following should be safe:

a flushing thread can access the data below the committed position without any locking

SamplerSampleWriter.accommodate(...) may access the uncommitted data without any locking

Ok I agree. I've removed the node-level locking for the SamplerBuffers

christianhaeubl · 2023-04-24T15:56:04Z

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/BufferNodeAccess.java

+    }
+
+    @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
+    public static SamplerBuffer getSamplerBuffer(BufferNode node) {


Just as a note: in the long run, we should probably try to get rid of this special buffer. This should be possible if we improve the JfrBuffer/JfrBufferNode/JfrBufferList infrastructure a bit.

…treaming-follow-up

christianhaeubl · 2023-04-27T09:22:56Z

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/sampler/SamplerBufferList.java

+    @Override
+    @Uninterruptible(reason = "Locking without transition requires that the whole critical section is uninterruptible.")
+    public BufferNode addNode(com.oracle.svm.core.jfr.Buffer buffer) {
+        assert !AbstractJfrExecutionSampler.isExecutionSamplingAllowedInCurrentThread();


Thanks for doing the changes.

In the last review, I missed that this is called from the signal handler. This is not allowed because in the signal handler, we can't allocate any memory (i.e., not even C memory).

Oh ok I see. I've changed things now so that the BufferNode allocations are done by SamplerBufferPool at the same time SamplerBuffer allocations are done. Now, buffers acquired from the "available" stack already have corresponding nodes, which are ready to be linked onto SamplerBufferList when the buffers become active. When full buffers are released after being serialized, new BufferNodes are allocated for them before they go back into circulation on the available list.

roberttoyonaga · 2023-06-26T17:25:46Z

Hi @christianhaeubl, when you have time, can you please have another look at this?

roberttoyonaga · 2023-08-22T20:20:34Z

Hi @christianhaeubl, just commenting here to keep this on your radar.

…uffer

…treaming-follow-up

roberttoyonaga · 2024-01-04T16:32:39Z

hi @christianhaeubl can you please have another look at this one, when you get the chance?

roberttoyonaga added 6 commits March 20, 2023 14:11

wip add flushedPos pointer

a5c8246

wip made stacktrace serialization operations atomic

6c57fcf

wip

be06436

working. Added tests.

6fd1829

clean up

3f5fe3a

remove clear repository methods. More refactoring more simplification BufferList. Update javadoc clean up

fix conflicts

0e79f79

oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Apr 4, 2023

gate fixes and remove safepoint check

cf8c486

gate fixes style

roberttoyonaga force-pushed the event-streaming-follow-up branch from 60123e0 to cf8c486 Compare April 4, 2023 18:35

oubidar-Abderrahim self-assigned this Apr 11, 2023

oubidar-Abderrahim requested a review from christianhaeubl April 11, 2023 12:03

christianhaeubl reviewed Apr 24, 2023

View reviewed changes

roberttoyonaga added 4 commits April 25, 2023 11:31

Merge branch 'master' of github.com:roberttoyonaga/graal into event-s…

72cb128

…treaming-follow-up

apply review feedback

5628d66

remove locking of sampler nodes. Add deadlock checks

ac169f9

gate fixes

c32f3e5

christianhaeubl reviewed Apr 27, 2023

View reviewed changes

avoid allocations in signal handler code

6a6f787

roberttoyonaga requested a review from christianhaeubl April 27, 2023 19:07

javadoc fixes

562148e

roberttoyonaga mentioned this pull request Apr 28, 2023

Fix virtual thread IDs in JFR events #6464

Merged

merge master

736f930

roberttoyonaga force-pushed the event-streaming-follow-up branch from 2a5d0d8 to 038668b Compare June 22, 2023 18:00

style

f94a07a

roberttoyonaga force-pushed the event-streaming-follow-up branch from 038668b to f94a07a Compare June 26, 2023 15:19

avoid possible NPE

17d46ce

roberttoyonaga added 2 commits August 22, 2023 14:56

fix conflicts

cd18858

static method gate fix

52309c1

roberttoyonaga added 2 commits November 1, 2023 14:01

fix conflicts

058b793

use jdk.graal.compiler

2f7610a

roberttoyonaga force-pushed the event-streaming-follow-up branch from 919a533 to 2f7610a Compare November 1, 2023 20:37

roberttoyonaga added 4 commits November 3, 2023 14:30

prevent race between recorder thread and assertion verifying SamplerB…

b9198ca

…uffer

Merge branch 'master' of github.com:roberttoyonaga/graal into event-s…

1c04bc7

…treaming-follow-up

fix conflicts

52ec699

fix conflicts

95cfc2e

oubidar-Abderrahim assigned christianhaeubl and unassigned oubidar-Abderrahim Feb 13, 2024

roberttoyonaga mentioned this pull request Jul 25, 2025

[GR-67169] Support JFR emergency dumps on out of memory #11530

Open

Serialize JFR stack traces during flushpoints #6365

Are you sure you want to change the base?

Serialize JFR stack traces during flushpoints #6365

Uh oh!

Conversation

roberttoyonaga commented Apr 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oubidar-Abderrahim commented Apr 11, 2023

Uh oh!

christianhaeubl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

christianhaeubl Apr 13, 2023

Choose a reason for hiding this comment

Uh oh!

roberttoyonaga Apr 26, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

christianhaeubl Apr 24, 2023

Choose a reason for hiding this comment

Uh oh!

roberttoyonaga Apr 26, 2023

Choose a reason for hiding this comment

Uh oh!

christianhaeubl Apr 24, 2023

Choose a reason for hiding this comment

Uh oh!

christianhaeubl Apr 27, 2023

Choose a reason for hiding this comment

Uh oh!

roberttoyonaga Apr 27, 2023

Choose a reason for hiding this comment

Uh oh!

roberttoyonaga commented Jun 26, 2023

Uh oh!

roberttoyonaga commented Aug 22, 2023

Uh oh!

roberttoyonaga commented Jan 4, 2024

Uh oh!

Uh oh!

roberttoyonaga commented Apr 4, 2023 •

edited

Loading