Open roberttoyonaga opened 1 year ago
@christianhaeubl One problem that I've run into is that the sample buffers are not segmented the same way JfrBuffers are (they only have one position pointer). Which means a flushing thread can destroy in-progress writes by the thread that owns the buffer. This would happen when the flushing thread processes an active buffer and resets its position pointer. A simple solution to this could be to add a second "flushedPos" pointer similar to the JfrBuffers. Then it won't be necessary to clear the sample buffers when processing, and instead only advance the "flushedPos". However, that could mean that queuing buffers on the full queue might be a waste of space because a "full" buffer may actually be completely flushed already.
A second solution would be to force owning threads to lock their sample buffers before using them. But this approach results in lock acquisitions inside the signal handler code. What do you think is the best solution here?
I thought about that a bit and here is my opinion and a few invariants that I came up with:
SamplerBuffer
s must be flushed after we are done flushing the JfrBuffer
s. Unlike JfrBuffer
s, SamplerBuffer
s must not be skipped (otherwise, we would flush events but not have valid stack traces for them).JfrStackTraceRepository
, need to hold the JfrChunkWriter
lock.flushedPos
to SamplerBuffer
. However, unlike JfrBuffer
s, there is no need for any separate locking because the flushedPos
will only be modified by threads that hold the JfrChunkWriter
lock. Adding an already flushed buffer to the fullBuffers
shouldn't cause any issues: the JfrRecorderThread
will be notified and needs to realize that no flushing is necessary. So, it can either free or reuse the buffer right away.
Currently, stacktraces are disabled for flushing. This means that streamed events will not have stacktrace data. The problem is that we cannot safely access the sample buffers of other threads outside of safepoints. This is a problem because they must be accessed so they can be serialized and written to the chunk lazily by a thread performing the flush operation. One possible solution is to adapt a similar approach to how the JfrBuffers are iterated outside of safepoints (collecting them in a list).
Express whether you'd like to help contributing this feature I'll help with this feature.