Skip to content

Commit 8283ea1

Browse files
authored
Fix wallclock sample collapse for blocking operations (#322)
Compare execution state (PC+SP) and trace context (spanId+rootSpanId) instead of requiring CPU samples between wallclock samples. Prevents misattribution when code does mostly blocking I/O and thread moves between different syscalls or when trace context changes.
1 parent f0ae94b commit 8283ea1

2 files changed

Lines changed: 26 additions & 7 deletions

File tree

ddprof-lib/src/main/cpp/thread.h

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,9 @@ class ProfiledThread : public ThreadLocalData {
5252
static void pushFreeSlot(int slot_index);
5353

5454
u64 _pc;
55+
u64 _sp;
5556
u64 _span_id;
57+
u64 _root_span_id;
5658
volatile u32 _crash_depth;
5759
int _buffer_pos;
5860
int _tid;
@@ -66,7 +68,7 @@ class ProfiledThread : public ThreadLocalData {
6668
Context* _ctx_tls_ptr;
6769

6870
ProfiledThread(int buffer_pos, int tid)
69-
: ThreadLocalData(), _pc(0), _span_id(0), _crash_depth(0), _buffer_pos(buffer_pos), _tid(tid), _cpu_epoch(0),
71+
: ThreadLocalData(), _pc(0), _sp(0), _span_id(0), _root_span_id(0), _crash_depth(0), _buffer_pos(buffer_pos), _tid(tid), _cpu_epoch(0),
7072
_wall_epoch(0), _call_trace_id(0), _recording_epoch(0), _filter_slot_id(-1), _ctx_tls_initialized(false), _ctx_tls_ptr(nullptr) {};
7173

7274
void releaseFromBuffer();
@@ -99,13 +101,29 @@ class ProfiledThread : public ThreadLocalData {
99101
return ++_cpu_epoch;
100102
}
101103

102-
u64 lookupWallclockCallTraceId(u64 pc, u32 recording_epoch, u64 span_id) {
103-
if (_wall_epoch == _cpu_epoch && _pc == pc && _span_id == span_id &&
104-
_recording_epoch == recording_epoch && _call_trace_id != 0) {
104+
/**
105+
* Attempts to reuse a cached call trace ID for wallclock sample collapsing.
106+
* Collapsing is allowed only when the execution state (PC, SP) and trace
107+
* context (spanId, rootSpanId) are identical to the previous sample.
108+
*
109+
* @param pc Program counter from ucontext
110+
* @param sp Stack pointer from ucontext
111+
* @param recording_epoch Current profiling session epoch
112+
* @param span_id Current trace span ID
113+
* @param root_span_id Current trace root span ID
114+
* @return Cached call_trace_id if collapsing is allowed, 0 otherwise
115+
*/
116+
u64 lookupWallclockCallTraceId(u64 pc, u64 sp, u32 recording_epoch,
117+
u64 span_id, u64 root_span_id) {
118+
if (_pc == pc && _sp == sp && _span_id == span_id &&
119+
_root_span_id == root_span_id && _recording_epoch == recording_epoch &&
120+
_call_trace_id != 0) {
105121
return _call_trace_id;
106122
}
107-
_wall_epoch = _cpu_epoch;
108123
_pc = pc;
124+
_sp = sp;
125+
_span_id = span_id;
126+
_root_span_id = root_span_id;
109127
_recording_epoch = recording_epoch;
110128
return 0;
111129
}

ddprof-lib/src/main/cpp/wallClock.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,9 @@ void WallClockASGCT::signalHandler(int signo, siginfo_t *siginfo, void *ucontext
7070
StackFrame frame(ucontext);
7171
Context &context = Contexts::get();
7272
call_trace_id = current->lookupWallclockCallTraceId(
73-
(u64)frame.pc(), Profiler::instance()->recordingEpoch(),
74-
context.spanId);
73+
(u64)frame.pc(), (u64)frame.sp(),
74+
Profiler::instance()->recordingEpoch(),
75+
context.spanId, context.rootSpanId);
7576
if (call_trace_id != 0) {
7677
Counters::increment(SKIPPED_WALLCLOCK_UNWINDS);
7778
}

0 commit comments

Comments
 (0)