Skip to content

Commit 7f03ffc

Browse files
jbachorikclaude
andcommitted
Add likely/unlikely hints to calltrace hot-path guards
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
1 parent 80a8977 commit 7f03ffc

2 files changed

Lines changed: 15 additions & 15 deletions

File tree

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

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ class LongHashTable {
4848

4949
static LongHashTable *allocate(LongHashTable *prev, u32 capacity, LinearAllocator* allocator) {
5050
void *memory = allocator->alloc(getSize(capacity));
51-
if (memory != nullptr) {
51+
if (likely(memory != nullptr)) {
5252
// Use placement new to invoke constructor in-place with parameters
5353
// LinearAllocator doesn't zero memory like OS::safeAlloc with anon mmap
5454
// so we need to explicitly clear the keys and values (should_clean = true)
@@ -174,7 +174,7 @@ CallTrace *CallTraceHashTable::storeCallTrace(int num_frames,
174174
const size_t total_size = header_size + num_frames * sizeof(ASGCT_CallFrame);
175175
void *memory = _allocator.alloc(total_size);
176176
CallTrace *buf = nullptr;
177-
if (memory != nullptr) {
177+
if (likely(memory != nullptr)) {
178178
// Use placement new to invoke constructor in-place
179179
buf = new (memory) CallTrace(truncated, num_frames, trace_id);
180180
// Do not use memcpy inside signal handler
@@ -199,7 +199,7 @@ CallTrace *CallTraceHashTable::findCallTrace(LongHashTable *table, u64 hash) {
199199
if (keys[slot] == 0) {
200200
return nullptr;
201201
}
202-
if (!probe.hasNext()) {
202+
if (unlikely(!probe.hasNext())) {
203203
break;
204204
}
205205
slot = probe.next();
@@ -213,7 +213,7 @@ u64 CallTraceHashTable::put(int num_frames, ASGCT_CallFrame *frames,
213213
u64 hash = calcHash(num_frames, frames, truncated);
214214

215215
LongHashTable *table = _table;
216-
if (table == nullptr) {
216+
if (unlikely(table == nullptr)) {
217217
// Table allocation failed or was cleared - drop sample
218218
Counters::increment(CALLTRACE_STORAGE_DROPPED);
219219
return CallTraceStorage::DROPPED_TRACE_ID;
@@ -230,7 +230,7 @@ u64 CallTraceHashTable::put(int num_frames, ASGCT_CallFrame *frames,
230230
CallTrace* current_trace = table->values()[slot].acquireTrace();
231231

232232
// If another thread is preparing this slot, wait for completion
233-
if (current_trace == CallTraceSample::PREPARING) {
233+
if (unlikely(current_trace == CallTraceSample::PREPARING)) {
234234
// Wait for the preparing thread to complete, with timeout
235235
int wait_cycles = 0;
236236
const int MAX_WAIT_CYCLES = 1000; // ~1000 cycles should be enough for allocation
@@ -252,13 +252,13 @@ u64 CallTraceHashTable::put(int num_frames, ASGCT_CallFrame *frames,
252252
} while (current_trace == CallTraceSample::PREPARING && wait_cycles < MAX_WAIT_CYCLES);
253253

254254
// If still preparing after timeout, something is wrong - continue search
255-
if (current_trace == CallTraceSample::PREPARING) {
255+
if (unlikely(current_trace == CallTraceSample::PREPARING)) {
256256
continue;
257257
}
258258
}
259259

260260
// Check final state after waiting
261-
if (current_trace != nullptr && current_trace != CallTraceSample::PREPARING) {
261+
if (likely(current_trace != nullptr && current_trace != CallTraceSample::PREPARING)) {
262262
// Trace is ready, use it
263263
return current_trace->trace_id;
264264
} else {
@@ -279,7 +279,7 @@ u64 CallTraceHashTable::put(int num_frames, ASGCT_CallFrame *frames,
279279
}
280280

281281
// Mark the slot as being prepared so other threads know to wait
282-
if (!table->values()[slot].markPreparing()) {
282+
if (unlikely(!table->values()[slot].markPreparing())) {
283283
// Failed to mark as preparing (shouldn't happen), clear key with full barrier and retry
284284
__atomic_thread_fence(__ATOMIC_SEQ_CST);
285285
__atomic_store_n(&keys[slot], 0, __ATOMIC_RELEASE);
@@ -293,10 +293,10 @@ u64 CallTraceHashTable::put(int num_frames, ASGCT_CallFrame *frames,
293293
probe.updateCapacity(new_size);
294294

295295
// EXPANSION LOGIC: Check if 75% capacity reached after incrementing size
296-
if (new_size == capacity * 3 / 4) {
296+
if (unlikely(new_size == capacity * 3 / 4)) {
297297
// Allocate new table with double capacity using LinearAllocator
298298
LongHashTable* new_table = LongHashTable::allocate(table, capacity * 2, &_allocator);
299-
if (new_table != nullptr) {
299+
if (likely(new_table != nullptr)) {
300300
// Atomic table swap - only one thread succeeds
301301
__atomic_compare_exchange_n(&_table, &table, new_table, false, __ATOMIC_ACQ_REL, __ATOMIC_RELAXED);
302302
}
@@ -315,7 +315,7 @@ u64 CallTraceHashTable::put(int num_frames, ASGCT_CallFrame *frames,
315315
u64 instance_id = _instance_id.load(std::memory_order_acquire);
316316
u64 trace_id = (instance_id << 32) | slot;
317317
trace = storeCallTrace(num_frames, frames, truncated, trace_id);
318-
if (trace == nullptr) {
318+
if (unlikely(trace == nullptr)) {
319319
// Allocation failure - reset trace first, then clear key
320320
table->values()[slot].setTrace(nullptr);
321321
__atomic_thread_fence(__ATOMIC_SEQ_CST);
@@ -330,7 +330,7 @@ u64 CallTraceHashTable::put(int num_frames, ASGCT_CallFrame *frames,
330330
return trace->trace_id;
331331
}
332332

333-
if (!probe.hasNext()) {
333+
if (unlikely(!probe.hasNext())) {
334334
// Table overflow - very unlikely with expansion logic
335335
atomicIncRelaxed(_overflow);
336336
return OVERFLOW_TRACE_ID;

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ u64 CallTraceStorage::put(int num_frames, ASGCT_CallFrame* frames, bool truncate
345345
CallTraceHashTable* active = const_cast<CallTraceHashTable*>(__atomic_load_n(&_active_storage, __ATOMIC_ACQUIRE));
346346

347347
// Safety check - if null, system is shutting down
348-
if (active == nullptr) {
348+
if (unlikely(active == nullptr)) {
349349
Counters::increment(CALLTRACE_STORAGE_DROPPED);
350350
return DROPPED_TRACE_ID;
351351
}
@@ -355,7 +355,7 @@ u64 CallTraceStorage::put(int num_frames, ASGCT_CallFrame* frames, bool truncate
355355
RefCountGuard guard(active);
356356

357357
// Check if refcount guard allocation failed (slot exhaustion)
358-
if (!guard.isActive()) {
358+
if (unlikely(!guard.isActive())) {
359359
// No refcount protection available - return dropped trace ID
360360
Counters::increment(CALLTRACE_STORAGE_DROPPED);
361361
return DROPPED_TRACE_ID;
@@ -373,7 +373,7 @@ u64 CallTraceStorage::put(int num_frames, ASGCT_CallFrame* frames, bool truncate
373373
//
374374
// Memory ordering: ACQUIRE load ensures we see scanner's ACQ_REL exchange to nullptr
375375
CallTraceHashTable* original_active = const_cast<CallTraceHashTable*>(__atomic_load_n(&_active_storage, __ATOMIC_ACQUIRE));
376-
if (original_active != active || original_active == nullptr) {
376+
if (unlikely(original_active != active || original_active == nullptr)) {
377377
// Storage was swapped or nullified during guard construction
378378
// SAFE: We detected the race, drop this trace, never use the table pointer
379379
Counters::increment(CALLTRACE_STORAGE_DROPPED);

0 commit comments

Comments
 (0)