Skip to content

Commit c5ccb1e

Browse files
kaahoskaahos
committed
fix: address sphinx review
1 parent c64cee1 commit c5ccb1e

17 files changed

Lines changed: 499 additions & 147 deletions

File tree

‎build-logic/conventions/src/main/kotlin/com/datadoghq/native/config/ConfigurationPresets.kt‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,18 @@
1+
/*
2+
* Copyright 2026, Datadog, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
116

217
package com.datadoghq.native.config
318

@@ -149,7 +164,7 @@ object ConfigurationPresets {
149164
config.compilerArgs.set(
150165
listOf("-O0", "-g", "-DDEBUG") + commonLinuxCompilerArgs(version)
151166
)
152-
config.linkerArgs.set(commonLinuxLinkerArgs())
167+
config.linkerArgs.set(commonLinuxLinkerArgs() + listOf("-Wl,-z,nodelete"))
153168
}
154169
Platform.MACOS -> {
155170
config.compilerArgs.set(

‎ddprof-lib/src/main/cpp/frames.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2026 Datadog, Inc
2+
* Copyright 2026, Datadog, Inc.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.

‎ddprof-lib/src/main/cpp/javaApi.cpp‎

Lines changed: 22 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,20 @@ Java_com_datadoghq_profiler_JavaProfiler_init0(
7878
}
7979

8080
// JavaVM* has already been stored when the native library was loaded so we can pass nullptr here
81-
return VM::initProfilerBridge(nullptr, true, delegateMonitorWaitEvents);
81+
ProfilerBridgeInitResult result =
82+
VM::initProfilerBridge(nullptr, true, delegateMonitorWaitEvents);
83+
if (result == ProfilerBridgeInitResult::MONITOR_EVENTS_DELEGATION_CONFLICT) {
84+
throwNew(env, "java/lang/IllegalStateException",
85+
"Monitor-event ownership conflicts with the profiler's "
86+
"process-wide initialization");
87+
return JNI_FALSE;
88+
}
89+
if (result != ProfilerBridgeInitResult::SUCCESS) {
90+
throwNew(env, "java/lang/IllegalStateException",
91+
"Failed to initialize the profiler bridge");
92+
return JNI_FALSE;
93+
}
94+
return JNI_TRUE;
8295
}
8396

8497
extern "C" DLLEXPORT void JNICALL
@@ -144,32 +157,6 @@ Java_com_datadoghq_profiler_JavaProfiler_getSamples(JNIEnv *env,
144157
return (jlong)Profiler::instance()->total_samples();
145158
}
146159

147-
// some duplication between add and remove, though we want to avoid having an extra branch in the hot path
148-
149-
static ThreadFilter::SlotID ensureCurrentThreadFilterSlot(
150-
ThreadFilter *thread_filter, ProfiledThread *current) {
151-
int tid = current->tid();
152-
if (unlikely(tid < 0)) {
153-
return -1;
154-
}
155-
156-
ThreadFilter::SlotID slot_id = current->filterSlotId();
157-
if (likely(slot_id >= 0)) {
158-
if (likely(thread_filter->activeSlotForId(slot_id, tid) != nullptr)) {
159-
return slot_id;
160-
}
161-
current->setFilterSlotId(-1);
162-
}
163-
164-
// Startup can register this TID centrally, but it cannot update another
165-
// pthread's TLS. registerThread(tid) reuses that existing slot.
166-
slot_id = thread_filter->registerThread(tid);
167-
if (slot_id >= 0) {
168-
current->setFilterSlotId(slot_id);
169-
}
170-
return slot_id;
171-
}
172-
173160
// JavaCritical is faster JNI, but more restrictive - parameters and return value have to be
174161
// primitives or arrays of primitive types.
175162
// We direct corresponding JNI calls to JavaCritical to make sure the parameters/return value
@@ -191,7 +178,7 @@ JavaCritical_com_datadoghq_profiler_JavaProfiler_filterThreadAdd0() {
191178
return;
192179
}
193180

194-
int slot_id = ensureCurrentThreadFilterSlot(thread_filter, current);
181+
int slot_id = thread_filter->ensureCurrentThreadSlot(current);
195182
if (unlikely(slot_id < 0)) {
196183
return; // Failed to register thread
197184
}
@@ -385,7 +372,7 @@ Java_com_datadoghq_profiler_JavaProfiler_parkEnter0(
385372
ThreadFilter *tf = profiler->threadFilter();
386373
if (context.spanId == 0 && tf->registryActive() &&
387374
(profiler->taskBlockEnabled() || tf->enabled())) {
388-
ThreadFilter::SlotID slot_id = ensureCurrentThreadFilterSlot(tf, current);
375+
ThreadFilter::SlotID slot_id = tf->ensureCurrentThreadSlot(current);
389376
if (slot_id >= 0) {
390377
current->setParkBlockToken(tf->enterBlockedRun(
391378
slot_id, OSThreadState::CONDVAR_WAIT, BlockRunOwner::JAVA));
@@ -412,32 +399,10 @@ Java_com_datadoghq_profiler_JavaProfiler_parkExit0(
412399
return;
413400
}
414401
Profiler *profiler = Profiler::instance();
415-
bool recording_enabled = profiler->taskBlockEnabled();
416-
bool activity = profiler->tryEnterTaskBlockActivity();
417-
if (!activity) profiler->waitForTaskBlockRotation();
418-
419-
ThreadFilter *tf = profiler->threadFilter();
420-
ThreadFilter::SlotID slot_id = ThreadFilter::tokenSlotId(park_block_token);
421-
ThreadFilter::SlotID current_slot = current->filterSlotId();
422-
if (current_slot < 0) current_slot = tf->slotIdByTid(current->tid());
423-
BlockRunSnapshot snapshot{};
424-
bool exited = current_slot == slot_id &&
425-
tf->snapshotAndExitBlockedRun(
426-
slot_id, ThreadFilter::tokenGeneration(park_block_token), &snapshot);
427-
428-
if (!activity) {
429-
Counters::increment(TASK_BLOCK_DROPPED_ROTATION);
430-
return;
431-
}
432-
if (recording_enabled && exited && snapshot.context_eligible) {
433-
recordTaskBlockIfEligible(
434-
current->tid(), thread, 1, start_ticks, TSC::ticks(), context,
435-
static_cast<u64>(blocker), static_cast<u64>(unblockingSpanId),
436-
snapshot.active_state, true);
437-
} else if (recording_enabled && exited && !snapshot.context_eligible) {
438-
Counters::increment(TASK_BLOCK_SKIPPED_TRACE_CONTEXT);
439-
}
440-
profiler->leaveTaskBlockActivity();
402+
finishTaskBlockAtExit(
403+
current, profiler->threadFilter(), thread, 1, park_block_token,
404+
start_ticks, context, static_cast<u64>(blocker),
405+
static_cast<u64>(unblockingSpanId));
441406
}
442407

443408
static bool decodeJavaBlockState(jint state, OSThreadState &decoded) {
@@ -469,7 +434,7 @@ Java_com_datadoghq_profiler_JavaProfiler_blockEnter0(
469434
if (!profiler->taskBlockEnabled() && !tf->enabled()) {
470435
return 0;
471436
}
472-
ThreadFilter::SlotID slot_id = ensureCurrentThreadFilterSlot(tf, current);
437+
ThreadFilter::SlotID slot_id = tf->ensureCurrentThreadSlot(current);
473438
if (slot_id < 0) return 0;
474439
return static_cast<jlong>(tf->enterBlockedRun(slot_id, decoded));
475440
}
@@ -510,7 +475,7 @@ Java_com_datadoghq_profiler_JavaProfiler_beginTaskBlock0(
510475
}
511476
ThreadFilter *tf = profiler->threadFilter();
512477
if (!tf->unfilteredWallTrackingActive()) return 0;
513-
ThreadFilter::SlotID slot_id = ensureCurrentThreadFilterSlot(tf, current);
478+
ThreadFilter::SlotID slot_id = tf->ensureCurrentThreadSlot(current);
514479
if (slot_id < 0) return 0;
515480

516481
Context context = ContextApi::snapshot();

‎ddprof-lib/src/main/cpp/profiler.cpp‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,10 @@ void Profiler::onThreadStart(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread) {
8585

8686
current->setJavaThread(true);
8787
int tid = current->tid();
88-
// Preserve eager registration for context-filtered recordings. Unfiltered
89-
// wall prechecks register only from context and owned-block hooks.
90-
if (_thread_filter.enabled()) {
88+
// Java lifecycle callbacks own registry allocation. The wall timer only
89+
// looks up these entries and must never allocate slots for arbitrary OS
90+
// threads returned by OS::listThreads().
91+
if (_thread_filter.registryActive()) {
9192
int slot_id = _thread_filter.registerThread(tid);
9293
current->setFilterSlotId(slot_id);
9394
}
@@ -1553,8 +1554,8 @@ Error Profiler::start(Arguments &args, bool reset) {
15531554
_thread_filter.clearActive();
15541555
}
15551556

1556-
// Preserve the context-filter fast path. Unfiltered tracking remains empty
1557-
// until a context or owned-block hook registers the current thread.
1557+
// Preserve the context-filter fast path. Unfiltered Java threads are
1558+
// registered by ThreadStart callbacks after the engines are activated.
15581559
if (_thread_filter.enabled()) {
15591560
ProfiledThread *current = ProfiledThread::initCurrentThreadSignalSafe();
15601561
assert(current != nullptr);

‎ddprof-lib/src/main/cpp/taskBlockRecorder.cpp‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,27 @@ bool recordTaskBlockAtExit(ProfiledThread* current, ThreadFilter* thread_filter,
3434
return false;
3535
}
3636

37+
if (slot_id != ThreadFilter::tokenSlotId(block_token) ||
38+
generation != ThreadFilter::tokenGeneration(block_token)) {
39+
return false;
40+
}
41+
42+
return finishTaskBlockAtExit(
43+
current, thread_filter, thread, start_depth, block_token, start_ticks,
44+
context, blocker, unblocking_span_id);
45+
}
46+
47+
bool finishTaskBlockAtExit(ProfiledThread* current,
48+
ThreadFilter* thread_filter, jthread thread,
49+
int start_depth, u64 block_token, u64 start_ticks,
50+
const Context& context, u64 blocker,
51+
u64 unblocking_span_id) {
3752
Profiler* profiler = Profiler::instance();
3853
bool recording_enabled = profiler->taskBlockEnabled();
3954
bool activity = profiler->tryEnterTaskBlockActivity();
4055

56+
ThreadFilter::SlotID slot_id = ThreadFilter::tokenSlotId(block_token);
57+
u64 generation = ThreadFilter::tokenGeneration(block_token);
4158
ThreadFilter::SlotID current_slot = current->filterSlotId();
4259
if (current_slot < 0) {
4360
current_slot = thread_filter->slotIdByTid(current->tid());

‎ddprof-lib/src/main/cpp/taskBlockRecorder.h‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,17 @@ bool recordTaskBlockAtExit(ProfiledThread* current, ThreadFilter* thread_filter,
2020
ThreadFilter::SlotID slot_id, u64 generation,
2121
u64 blocker, u64 unblocking_span_id);
2222

23+
// Completes ThreadFilter lifecycle cleanup for an already-exited producer and
24+
// records its event only when dump/stop rotation admits the recording work.
25+
// Cleanup is deliberately performed even when admission is rejected so an
26+
// application thread never waits for rotation and suppression cannot be left
27+
// armed.
28+
bool finishTaskBlockAtExit(ProfiledThread* current,
29+
ThreadFilter* thread_filter, jthread thread,
30+
int start_depth, u64 block_token, u64 start_ticks,
31+
const Context& context, u64 blocker,
32+
u64 unblocking_span_id);
33+
2334
class TaskBlockActivity {
2435
private:
2536
Profiler* _profiler;

‎ddprof-lib/src/main/cpp/threadFilter.cpp‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ void ThreadFilter::initializeChunk(int chunk_idx) {
102102
slot.recording_epoch.store(0, std::memory_order_relaxed);
103103
slot.context_window_state.store(0, std::memory_order_relaxed);
104104
slot.active_block_state.store(OSThreadState::UNKNOWN, std::memory_order_relaxed);
105+
slot.unowned_blocked_fallback_enabled.store(1, std::memory_order_relaxed);
105106
}
106107

107108
// Try to install it atomically
@@ -140,6 +141,7 @@ ThreadFilter::SlotID ThreadFilter::registerThread(int tid) {
140141
slot->lifecycle_generation.fetch_add(1, std::memory_order_acq_rel);
141142
slot->recording_epoch.store(0, std::memory_order_relaxed);
142143
slot->context_window_state.store(0, std::memory_order_relaxed);
144+
slot->enableUnownedBlockedFallback();
143145
slot->clearActiveBlockRun(OSThreadState::UNKNOWN);
144146
slot->tid.store(tid, std::memory_order_release);
145147
if (tid >= 0 && !indexSlot(reused_slot, tid)) {
@@ -186,6 +188,7 @@ ThreadFilter::SlotID ThreadFilter::registerThread(int tid) {
186188
slot->lifecycle_generation.fetch_add(1, std::memory_order_acq_rel);
187189
slot->recording_epoch.store(0, std::memory_order_relaxed);
188190
slot->context_window_state.store(0, std::memory_order_relaxed);
191+
slot->enableUnownedBlockedFallback();
189192
slot->clearActiveBlockRun(OSThreadState::UNKNOWN);
190193
slot->tid.store(tid, std::memory_order_release);
191194
if (tid >= 0 && !indexSlot(index, tid)) {
@@ -210,6 +213,7 @@ void ThreadFilter::refreshSlotForRecording(Slot* slot, RecordingEpoch epoch) {
210213
// payload, then publish the new epoch only after the reset is complete.
211214
slot->recording_epoch.store(0, std::memory_order_release);
212215
slot->context_window_state.store(0, std::memory_order_relaxed);
216+
slot->enableUnownedBlockedFallback();
213217
slot->clearActiveBlockRun(OSThreadState::UNKNOWN);
214218
slot->recording_epoch.store(epoch, std::memory_order_release);
215219
}
@@ -301,6 +305,45 @@ ThreadFilter::Slot* ThreadFilter::activeSlotForId(SlotID slot_id,
301305
return slot;
302306
}
303307

308+
bool ThreadFilter::lookupThreadEntry(ThreadEntry& entry,
309+
RecordingEpoch epoch) const {
310+
Slot* slot = epoch != 0 ? lookupByTid(entry.tid, epoch)
311+
: lookupByTid(entry.tid);
312+
if (slot == nullptr) {
313+
return false;
314+
}
315+
entry.slot = slot;
316+
entry.lifecycle_generation = slot->lifecycleGeneration();
317+
entry.recording_epoch = slot->recordingEpoch();
318+
return true;
319+
}
320+
321+
ThreadFilter::SlotID ThreadFilter::ensureCurrentThreadSlot(ProfiledThread* current) {
322+
if (current == nullptr) {
323+
return -1;
324+
}
325+
int tid = current->tid();
326+
if (unlikely(tid < 0)) {
327+
return -1;
328+
}
329+
330+
SlotID slot_id = current->filterSlotId();
331+
if (likely(slot_id >= 0)) {
332+
if (likely(activeSlotForId(slot_id, tid) != nullptr)) {
333+
return slot_id;
334+
}
335+
current->setFilterSlotId(-1);
336+
}
337+
338+
// Startup can register this TID centrally, but it cannot update another
339+
// pthread's TLS. registerThread(tid) reuses that existing slot.
340+
slot_id = registerThread(tid);
341+
if (slot_id >= 0) {
342+
current->setFilterSlotId(slot_id);
343+
}
344+
return slot_id;
345+
}
346+
304347
void ThreadFilter::initFreeList() {
305348
// Initialize the free list storage
306349
for (int i = 0; i < kFreeListSize; ++i) {
@@ -395,6 +438,7 @@ void ThreadFilter::unregisterThreadLocked(SlotID slot_id, int expected_tid) {
395438
slot->recording_epoch.store(0, std::memory_order_release);
396439
slot->tid.store(-1, std::memory_order_release);
397440
slot->context_window_state.store(0, std::memory_order_release);
441+
slot->enableUnownedBlockedFallback();
398442
slot->clearActiveBlockRun(OSThreadState::UNKNOWN);
399443
pushToFreeList(slot_id);
400444
}
@@ -420,6 +464,7 @@ void ThreadFilter::resetRegistrationsLocked() {
420464
slot.recording_epoch.store(0, std::memory_order_release);
421465
slot.tid.store(-1, std::memory_order_release);
422466
slot.context_window_state.store(0, std::memory_order_release);
467+
slot.enableUnownedBlockedFallback();
423468
slot.clearActiveBlockRun(OSThreadState::UNKNOWN);
424469
}
425470
}
@@ -541,6 +586,7 @@ void ThreadFilter::clearActive() {
541586
for (int slot_idx = 0; slot_idx < kChunkSize; ++slot_idx) {
542587
Slot& slot = chunk->slots[slot_idx];
543588
slot.exitContextWindow();
589+
slot.enableUnownedBlockedFallback();
544590
slot.clearActiveBlockRun(OSThreadState::UNKNOWN);
545591
}
546592
}
@@ -554,6 +600,7 @@ void ThreadFilter::resetSlotRunState(SlotID slot_id) {
554600
if (chunk != nullptr) {
555601
// Clear stale suppression state so a new thread in this slot cannot
556602
// inherit its predecessor's active block.
603+
chunk->slots[slot_idx].enableUnownedBlockedFallback();
557604
chunk->slots[slot_idx].clearActiveBlockRun(OSThreadState::UNKNOWN);
558605
}
559606
}

0 commit comments

Comments
 (0)