From 1118c13363c592d49e206c11242c4a626318d67a Mon Sep 17 00:00:00 2001 From: Roman Kennke Date: Fri, 7 Aug 2026 18:14:49 +0000 Subject: [PATCH] feat(nativemem): instrument remaining allocation sites (method map, liveness table, thread info, call-trace buffers) Extends the NM_* categorized native-memory accounting (#669) to eight previously-uninstrumented allocation sites: Recording::_method_map and LivenessTracker::_table get new NM_METHOD_MAP/NM_LIVENESS categories; UnwindFailures, ThreadInfo's two maps, Lookup::_vtable_receiver_cache, CallTraceStorage's two working buffers, and wall-clock's thread-reservoir vector are folded into existing categories. Adds a stateless CountingAllocator that wraps ::operator new/delete and reports exact per-node byte counts via STL's allocator-rebinding, used for the STL-container sites; raw-malloc sites (UnwindFailures, LivenessTracker) get direct record()/setLive() calls. Also fixes CodeCache::setDwarfTable() to shrink the DWARF/SFrame FrameDesc table to its exact length via realloc, correcting memoryUsage()'s length-based formula and eliminating the parser's capacity-doubling slack from real RSS. Test fixture fix: NativeMemTest's SetUp()/TearDown() used to zero all live gauges unconditionally, which underflowed UnwindFailures's destructor decrement (a real static-duration object accounted for outside the test) at process exit. Snapshot and restore the pre-test baseline instead. Co-Authored-By: Claude Sonnet 5 --- .../src/main/cpp/callTraceHashTable.cpp | 2 +- ddprof-lib/src/main/cpp/callTraceHashTable.h | 16 +++++- ddprof-lib/src/main/cpp/callTraceStorage.cpp | 2 +- ddprof-lib/src/main/cpp/callTraceStorage.h | 8 +-- ddprof-lib/src/main/cpp/codeCache.cpp | 10 ++++ ddprof-lib/src/main/cpp/countingAllocator.h | 55 +++++++++++++++++++ ddprof-lib/src/main/cpp/flightRecorder.cpp | 5 +- ddprof-lib/src/main/cpp/flightRecorder.h | 11 +++- ddprof-lib/src/main/cpp/livenessTracker.cpp | 10 +++- ddprof-lib/src/main/cpp/livenessTracker.h | 3 +- ddprof-lib/src/main/cpp/nativeMem.h | 2 + ddprof-lib/src/main/cpp/profiler.h | 4 +- ddprof-lib/src/main/cpp/threadInfo.cpp | 4 +- ddprof-lib/src/main/cpp/threadInfo.h | 13 ++++- ddprof-lib/src/main/cpp/unwindStats.h | 7 +++ ddprof-lib/src/main/cpp/wallClock.h | 2 + ddprof-lib/src/test/cpp/nativeMem_ut.cpp | 21 ++++++- .../src/test/cpp/stress_callTraceStorage.cpp | 26 ++++----- .../test/cpp/stress_threadLifecycle_ut.cpp | 2 +- .../src/test/cpp/test_callTraceStorage.cpp | 52 +++++++++--------- 20 files changed, 191 insertions(+), 64 deletions(-) create mode 100644 ddprof-lib/src/main/cpp/countingAllocator.h diff --git a/ddprof-lib/src/main/cpp/callTraceHashTable.cpp b/ddprof-lib/src/main/cpp/callTraceHashTable.cpp index ef27f7f738..1e2dbd9ad2 100644 --- a/ddprof-lib/src/main/cpp/callTraceHashTable.cpp +++ b/ddprof-lib/src/main/cpp/callTraceHashTable.cpp @@ -470,7 +470,7 @@ u64 CallTraceHashTable::put(int num_frames, ASGCT_CallFrame *frames, } } -void CallTraceHashTable::collect(std::unordered_set &traces, std::function trace_hook) { +void CallTraceHashTable::collect(CallTraceSet &traces, std::function trace_hook) { // Lock-free collection for read-only tables. // Use ACQUIRE to pair with the ACQ_REL CAS in put()'s expansion path and the // RELEASE store in clearTableOnly(); ensures we see the fully-initialised table diff --git a/ddprof-lib/src/main/cpp/callTraceHashTable.h b/ddprof-lib/src/main/cpp/callTraceHashTable.h index ae7286f2a2..9b387ca023 100644 --- a/ddprof-lib/src/main/cpp/callTraceHashTable.h +++ b/ddprof-lib/src/main/cpp/callTraceHashTable.h @@ -7,7 +7,9 @@ #define _CALLTRACEHASHTABLE_H #include "arch.h" +#include "countingAllocator.h" #include "linearAllocator.h" +#include "nativeMem.h" #include "vmEntry.h" #include #include @@ -21,11 +23,21 @@ struct CallTrace { u64 trace_id; // 64-bit for JFR constant pool compatibility ASGCT_CallFrame frames[1]; - CallTrace(bool truncated, int num_frames, u64 trace_id) + CallTrace(bool truncated, int num_frames, u64 trace_id) : truncated(truncated), num_frames(num_frames), trace_id(trace_id) { } }; +// Traces collected for JFR/liveness processing. Uses CountingAllocator so the +// real per-node heap cost is attributed to NM_CALLTRACE, the same category the +// rest of the call-trace arena already accounts into. +using CallTraceSet = + std::unordered_set, std::equal_to, + CountingAllocator>; +using CallTraceIdSet = + std::unordered_set, std::equal_to, + CountingAllocator>; + struct CallTraceSample { CallTrace *trace; @@ -122,7 +134,7 @@ class CallTraceHashTable { */ ChunkList clearTableOnly(); - void collect(std::unordered_set &traces, std::function trace_hook = nullptr); + void collect(CallTraceSet &traces, std::function trace_hook = nullptr); u64 put(int num_frames, ASGCT_CallFrame *frames, bool truncated, u64 weight); void putWithExistingId(CallTrace* trace, u64 weight); // For standby tables with no contention diff --git a/ddprof-lib/src/main/cpp/callTraceStorage.cpp b/ddprof-lib/src/main/cpp/callTraceStorage.cpp index b648367b6e..aa06e3ffd4 100644 --- a/ddprof-lib/src/main/cpp/callTraceStorage.cpp +++ b/ddprof-lib/src/main/cpp/callTraceStorage.cpp @@ -166,7 +166,7 @@ u64 CallTraceStorage::put(int num_frames, ASGCT_CallFrame* frames, bool truncate * This function is safe to call concurrently with put() operations. * It is not designed to be called concurrently with itself. */ -void CallTraceStorage::processTraces(std::function&)> processor) { +void CallTraceStorage::processTraces(std::function processor) { // PHASE 1: Collect liveness information with simple lock (rare operation) { SharedLockGuard lock(&_liveness_lock); diff --git a/ddprof-lib/src/main/cpp/callTraceStorage.h b/ddprof-lib/src/main/cpp/callTraceStorage.h index 5d1563e65b..e5cd24d527 100644 --- a/ddprof-lib/src/main/cpp/callTraceStorage.h +++ b/ddprof-lib/src/main/cpp/callTraceStorage.h @@ -26,7 +26,7 @@ class CallTraceHashTable; // Liveness checker function type // Fills the provided set with 64-bit call_trace_id values that should be preserved // Using reference parameter avoids malloc() for vector creation and copying -typedef std::function&)> LivenessChecker; +typedef std::function LivenessChecker; class CallTraceStorage { public: @@ -65,8 +65,8 @@ class CallTraceStorage { // Pre-allocated collections for processTraces (single-threaded operation) // These collections are reused to eliminate malloc/free cycles - std::unordered_set _traces_buffer; // All traces for JFR processing - std::unordered_set _preserve_set_buffer; // Preserve set for current cycle + CallTraceSet _traces_buffer; // All traces for JFR processing + CallTraceIdSet _preserve_set_buffer; // Preserve set for current cycle public: CallTraceStorage(); @@ -85,7 +85,7 @@ class CallTraceStorage { // Lock-free trace processing with RefCountGuard protection // The callback receives traces that are guaranteed to be valid during execution // Uses atomic table swapping with grace period for safe memory reclamation - void processTraces(std::function&)> processor); + void processTraces(std::function processor); // Enhanced clear with liveness preservation (rarely called - uses atomic operations) void clear(); diff --git a/ddprof-lib/src/main/cpp/codeCache.cpp b/ddprof-lib/src/main/cpp/codeCache.cpp index ab96ee112d..4563d91b83 100644 --- a/ddprof-lib/src/main/cpp/codeCache.cpp +++ b/ddprof-lib/src/main/cpp/codeCache.cpp @@ -474,6 +474,16 @@ void CodeCache::setDwarfTable(FrameDesc *table, int length, const FrameDesc &def // _dwarf_table_length lock-free at dump time, so this must not run afterwards. assert(!_published.load(std::memory_order_acquire) && "setDwarfTable() on a published CodeCache races memoryUsage()"); + // The parser (SFrameParser/DwarfParser) builds this table with capacity-doubling + // malloc/realloc, so the incoming allocation is typically larger than length + // entries. Shrink it to the exact size so memoryUsage()'s length-based formula + // isn't an undercount, and so the doubling slack doesn't linger in real RSS. + if (length > 0) { + FrameDesc *trimmed = (FrameDesc *)realloc(table, length * sizeof(FrameDesc)); + if (trimmed != NULL) { + table = trimmed; + } + } _dwarf_table = table; _dwarf_table_length = length; _default_frame = &default_frame; diff --git a/ddprof-lib/src/main/cpp/countingAllocator.h b/ddprof-lib/src/main/cpp/countingAllocator.h new file mode 100644 index 0000000000..e31aae3b85 --- /dev/null +++ b/ddprof-lib/src/main/cpp/countingAllocator.h @@ -0,0 +1,55 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ +#ifndef _COUNTINGALLOCATOR_H +#define _COUNTINGALLOCATOR_H + +#include "nativeMem.h" +#include +#include + +// A stateless, C++11-Allocator-conformant wrapper around ::operator new / +// ::operator delete that records every allocation/deallocation into the given +// NativeMem category. STL containers rebind the supplied Allocator +// to their actual node type before calling allocate(), so this yields the +// exact real per-node byte count the implementation uses -- not an estimate. +template +class CountingAllocator { +public: + using value_type = T; + + CountingAllocator() noexcept = default; + template + CountingAllocator(const CountingAllocator &) noexcept {} + + T *allocate(std::size_t n) { + T *p = static_cast(::operator new(n * sizeof(T))); + NativeMem::record(Cat, (long long)(n * sizeof(T))); + return p; + } + + void deallocate(T *p, std::size_t n) noexcept { + NativeMem::record(Cat, -(long long)(n * sizeof(T))); + ::operator delete(p); + } + + template + struct rebind { + using other = CountingAllocator; + }; +}; + +template +inline bool operator==(const CountingAllocator &, + const CountingAllocator &) { + return true; +} + +template +inline bool operator!=(const CountingAllocator &, + const CountingAllocator &) { + return false; +} + +#endif // _COUNTINGALLOCATOR_H diff --git a/ddprof-lib/src/main/cpp/flightRecorder.cpp b/ddprof-lib/src/main/cpp/flightRecorder.cpp index 95875ed016..73c7f4c474 100644 --- a/ddprof-lib/src/main/cpp/flightRecorder.cpp +++ b/ddprof-lib/src/main/cpp/flightRecorder.cpp @@ -1615,15 +1615,14 @@ int Recording::writeStackTraces(Buffer *buf, Lookup *lookup) { // via processCallTraces, but no T_STACK_TRACE section is emitted in that case. int trace_count = 0; // Use safe trace processing with guaranteed lifetime during callback execution - Profiler::instance()->processCallTraces([this, buf, lookup, &trace_count](const std::unordered_set& traces) { + Profiler::instance()->processCallTraces([this, buf, lookup, &trace_count](const CallTraceSet& traces) { if (traces.empty()) { return; } trace_count = (int)traces.size(); buf->putVar64(T_STACK_TRACE); buf->putVar64(traces.size()); - for (std::unordered_set::const_iterator it = traces.begin(); - it != traces.end(); ++it) { + for (auto it = traces.begin(); it != traces.end(); ++it) { CallTrace *trace = *it; buf->putVar64(trace->trace_id); if (trace->num_frames > 0) { diff --git a/ddprof-lib/src/main/cpp/flightRecorder.h b/ddprof-lib/src/main/cpp/flightRecorder.h index ee6929b4f1..71eb938ec6 100644 --- a/ddprof-lib/src/main/cpp/flightRecorder.h +++ b/ddprof-lib/src/main/cpp/flightRecorder.h @@ -7,6 +7,7 @@ #ifndef _FLIGHTRECORDER_H #define _FLIGHTRECORDER_H +#include #include #include #include @@ -18,6 +19,7 @@ #include "arch.h" #include "arguments.h" #include "buffers.h" +#include "countingAllocator.h" #include "counters.h" #include "dictionary.h" #include "stringDictionary.h" @@ -106,7 +108,10 @@ class MethodInfo { // 10 - void* address (native frame names) // 01 - RemoteFrameInfo (packed remote symbolication) // 11 - vtable_receiver class_id (BCI_VTABLE_RECEIVER frames) -class MethodMap : public std::map { +class MethodMap + : public std::map, + CountingAllocator, + NM_METHOD_MAP>> { public: static constexpr unsigned long ADDRESS_MARK = 0x8000000000000000ULL; static constexpr unsigned long REMOTE_FRAME_MARK = 0x4000000000000000ULL; @@ -352,7 +357,9 @@ class Lookup { // as the MethodMap key, so distinct Symbol* addresses for the same // class name (class unload/reload mid-chunk) collapse to a single // MethodInfo row. - std::unordered_map _vtable_receiver_cache; + std::unordered_map, std::equal_to, + CountingAllocator, NM_JFR_BUFFERS>> + _vtable_receiver_cache; Dictionary _packages; Dictionary _symbols; diff --git a/ddprof-lib/src/main/cpp/livenessTracker.cpp b/ddprof-lib/src/main/cpp/livenessTracker.cpp index 17dd48b691..0f5a84d4ea 100644 --- a/ddprof-lib/src/main/cpp/livenessTracker.cpp +++ b/ddprof-lib/src/main/cpp/livenessTracker.cpp @@ -16,6 +16,7 @@ #include "jniHelper.h" #include "livenessTracker.h" #include "log.h" +#include "nativeMem.h" #include "os.h" #include "profiler.h" #include "threadLocalData.h" @@ -193,7 +194,7 @@ Error LivenessTracker::start(Arguments &args) { } // Self-register with the profiler for liveness checking - Profiler::instance()->registerLivenessChecker([this](std::unordered_set& buffer) { + Profiler::instance()->registerLivenessChecker([this](CallTraceIdSet& buffer) { this->getLiveTraceIds(buffer); }); @@ -277,6 +278,9 @@ Error LivenessTracker::initialize(Arguments &args) { std::min(2048, _table_max_cap); // with default 512k sampling interval, it's // enough for 1G of heap _table = (TrackingEntry *)malloc(sizeof(TrackingEntry) * _table_cap); + if (_table != NULL) { + NativeMem::record(NM_LIVENESS, (long long)sizeof(TrackingEntry) * _table_cap); + } _gc_epoch = 0; _last_gc_epoch = 0; @@ -406,6 +410,8 @@ void LivenessTracker::track(JNIEnv *env, AllocEvent &event, jint tid, TrackingEntry *tmp = (TrackingEntry *)realloc( _table, sizeof(TrackingEntry) * newcap); if (tmp != nullptr) { + NativeMem::record(NM_LIVENESS, + (long long)sizeof(TrackingEntry) * (newcap - _table_cap)); _table = tmp; _table_cap = newcap; Log::debug( @@ -450,7 +456,7 @@ void LivenessTracker::onGC() { } } -void LivenessTracker::getLiveTraceIds(std::unordered_set& out_buffer) { +void LivenessTracker::getLiveTraceIds(CallTraceIdSet& out_buffer) { out_buffer.clear(); if (!_enabled || !_initialized) { diff --git a/ddprof-lib/src/main/cpp/livenessTracker.h b/ddprof-lib/src/main/cpp/livenessTracker.h index 622a03c810..d404d53ffb 100644 --- a/ddprof-lib/src/main/cpp/livenessTracker.h +++ b/ddprof-lib/src/main/cpp/livenessTracker.h @@ -7,6 +7,7 @@ #define _LIVENESSTRACKER_H #include "arch.h" +#include "callTraceHashTable.h" #include "context.h" #include "engine.h" #include "event.h" @@ -104,7 +105,7 @@ class alignas(alignof(SpinLock)) LivenessTracker { static void JNICALL GarbageCollectionFinish(jvmtiEnv *jvmti_env); private: - void getLiveTraceIds(std::unordered_set& out_buffer); + void getLiveTraceIds(CallTraceIdSet& out_buffer); }; #endif // _LIVENESSTRACKER_H diff --git a/ddprof-lib/src/main/cpp/nativeMem.h b/ddprof-lib/src/main/cpp/nativeMem.h index 61dfe7af9e..f74f2cd2ea 100644 --- a/ddprof-lib/src/main/cpp/nativeMem.h +++ b/ddprof-lib/src/main/cpp/nativeMem.h @@ -26,6 +26,8 @@ X(PERF, "perf") \ X(THREAD_LOCAL, "thread_local") \ X(JFR_BUFFERS, "jfr_buffers") \ + X(METHOD_MAP, "method_map") \ + X(LIVENESS, "liveness") \ X(MISC, "misc") #define X_NM_ENUM(a, b) NM_##a, diff --git a/ddprof-lib/src/main/cpp/profiler.h b/ddprof-lib/src/main/cpp/profiler.h index 3e4ffffced..5aa6bbebab 100644 --- a/ddprof-lib/src/main/cpp/profiler.h +++ b/ddprof-lib/src/main/cpp/profiler.h @@ -304,12 +304,12 @@ class alignas(alignof(SpinLock)) Profiler { const char* cstack() const; int lookupClass(const char *key, size_t length); - void processCallTraces(std::function&)> processor) { + void processCallTraces(std::function processor) { if (!_omit_stacktraces) { _call_trace_storage.processTraces(processor); } else { // If stack traces are omitted, call processor with empty set - static std::unordered_set empty_traces; + static CallTraceSet empty_traces; processor(empty_traces); } } diff --git a/ddprof-lib/src/main/cpp/threadInfo.cpp b/ddprof-lib/src/main/cpp/threadInfo.cpp index c2b80ca4c2..527ee4214a 100644 --- a/ddprof-lib/src/main/cpp/threadInfo.cpp +++ b/ddprof-lib/src/main/cpp/threadInfo.cpp @@ -42,7 +42,7 @@ void ThreadInfo::clearAll(std::set &live_thread_ids) { _thread_ids.clear(); } else { // we need to honor the thread referenced from the liveness tracker - std::map::iterator name_itr = _thread_names.begin(); + auto name_itr = _thread_names.begin(); while (name_itr != _thread_names.end()) { if (live_thread_ids.find(name_itr->first) == live_thread_ids.end()) { name_itr = _thread_names.erase(name_itr); @@ -50,7 +50,7 @@ void ThreadInfo::clearAll(std::set &live_thread_ids) { ++name_itr; } } - std::map::iterator id_itr = _thread_ids.begin(); + auto id_itr = _thread_ids.begin(); while (id_itr != _thread_ids.end()) { if (live_thread_ids.find(id_itr->first) == live_thread_ids.end()) { id_itr = _thread_ids.erase(id_itr); diff --git a/ddprof-lib/src/main/cpp/threadInfo.h b/ddprof-lib/src/main/cpp/threadInfo.h index 9fab8d2f5f..d60aae5332 100644 --- a/ddprof-lib/src/main/cpp/threadInfo.h +++ b/ddprof-lib/src/main/cpp/threadInfo.h @@ -1,4 +1,6 @@ +#include "countingAllocator.h" #include "mutex.h" +#include "nativeMem.h" #include "os.h" #include #include @@ -8,9 +10,16 @@ class ThreadInfo { private: + using ThreadNamesMap = + std::map, + CountingAllocator, NM_THREAD_LOCAL>>; + using ThreadIdsMap = + std::map, + CountingAllocator, NM_THREAD_LOCAL>>; + Mutex _ti_lock; - std::map _thread_names; - std::map _thread_ids; + ThreadNamesMap _thread_names; + ThreadIdsMap _thread_ids; public: // disallow copy and assign to avoid issues with the mutex diff --git a/ddprof-lib/src/main/cpp/unwindStats.h b/ddprof-lib/src/main/cpp/unwindStats.h index 1eb4eab297..114b934c2f 100644 --- a/ddprof-lib/src/main/cpp/unwindStats.h +++ b/ddprof-lib/src/main/cpp/unwindStats.h @@ -2,6 +2,7 @@ #define STUB_UNWIND_STATS_H #include "common.h" +#include "nativeMem.h" #include "spinLock.h" #include @@ -30,9 +31,15 @@ class UnwindFailures { _counters = new u64[MAX_UNWIND_FAILURE_NAMES][UNWIND_FAILURE_ANY + 1]; memset((void*)_names, 0, MAX_UNWIND_FAILURE_NAMES * MAX_NAME_LENGTH); memset((void*)_counters, 0, MAX_UNWIND_FAILURE_NAMES * (UNWIND_FAILURE_ANY + 1) * sizeof(u64)); + NativeMem::record(NM_THREAD_LOCAL, + (long long)(MAX_UNWIND_FAILURE_NAMES * MAX_NAME_LENGTH) + + (long long)(MAX_UNWIND_FAILURE_NAMES * (UNWIND_FAILURE_ANY + 1) * sizeof(u64))); } ~UnwindFailures() { + NativeMem::record(NM_THREAD_LOCAL, + -((long long)(MAX_UNWIND_FAILURE_NAMES * MAX_NAME_LENGTH) + + (long long)(MAX_UNWIND_FAILURE_NAMES * (UNWIND_FAILURE_ANY + 1) * sizeof(u64)))); delete[] _names; delete[] _counters; } diff --git a/ddprof-lib/src/main/cpp/wallClock.h b/ddprof-lib/src/main/cpp/wallClock.h index 14e3f88aa3..657384eb57 100644 --- a/ddprof-lib/src/main/cpp/wallClock.h +++ b/ddprof-lib/src/main/cpp/wallClock.h @@ -9,6 +9,7 @@ #include #include "engine.h" +#include "nativeMem.h" #include "os.h" #include "profiler.h" #include "reservoirSampler.h" @@ -79,6 +80,7 @@ class BaseWallClock : public Engine { while (_running.load(std::memory_order_relaxed)) { collectThreads(threads); + NativeMem::setLive(NM_MISC, (long long)threads.capacity() * sizeof(ThreadType)); int num_failures = 0; int threads_already_exited = 0; diff --git a/ddprof-lib/src/test/cpp/nativeMem_ut.cpp b/ddprof-lib/src/test/cpp/nativeMem_ut.cpp index d2f3bf1822..35cd3f89d9 100644 --- a/ddprof-lib/src/test/cpp/nativeMem_ut.cpp +++ b/ddprof-lib/src/test/cpp/nativeMem_ut.cpp @@ -9,8 +9,25 @@ class NativeMemTest : public ::testing::Test { protected: - void SetUp() override { NativeMem::reset(); } - void TearDown() override { NativeMem::reset(); } + long long _baseline[NM_NUM_CATEGORIES]; + + // Real static-duration objects elsewhere in the binary (e.g. + // UnwindStats::_unwind_failures) record their legitimate allocations into + // these same categories before main() runs. Snapshot that baseline and + // restore it in TearDown so their eventual destructors (which decrement + // what they incremented) don't underflow a category this fixture zeroed. + void SetUp() override { + for (int c = 0; c < NM_NUM_CATEGORIES; c++) { + _baseline[c] = NativeMem::live((NativeMemCategory)c); + } + NativeMem::reset(); + } + void TearDown() override { + NativeMem::reset(); + for (int c = 0; c < NM_NUM_CATEGORIES; c++) { + NativeMem::setLive((NativeMemCategory)c, _baseline[c]); + } + } }; // record() adds to and subtracts from the per-category live gauge, and the diff --git a/ddprof-lib/src/test/cpp/stress_callTraceStorage.cpp b/ddprof-lib/src/test/cpp/stress_callTraceStorage.cpp index a8706967d9..dda0b17deb 100644 --- a/ddprof-lib/src/test/cpp/stress_callTraceStorage.cpp +++ b/ddprof-lib/src/test/cpp/stress_callTraceStorage.cpp @@ -41,7 +41,7 @@ static constexpr const char STRESS_TEST_NAME[] = "StressCallTraceStorage"; static constexpr int CALLTRACE_EXPANSION_THRESHOLD = 65536 * 3 / 4; // 49152 // Helper function to find a CallTrace by trace_id in an unordered_set -CallTrace* findTraceById(const std::unordered_set& traces, u64 trace_id) { +CallTrace* findTraceById(const CallTraceSet& traces, u64 trace_id) { for (CallTrace* trace : traces) { if (trace && trace != CallTraceSample::PREPARING && trace->trace_id == trace_id) { return trace; @@ -51,7 +51,7 @@ CallTrace* findTraceById(const std::unordered_set& traces, u64 trace } // Optimized batch lookup for multiple trace IDs -void findMultipleTracesById(const std::unordered_set& traces, +void findMultipleTracesById(const CallTraceSet& traces, const std::vector& trace_ids, size_t& found_count) { // Create a lookup set for O(1) lookups instead of O(n) per trace @@ -250,7 +250,7 @@ TEST_F(StressTestSuite, SwapStormTest) { // Use mutex to ensure single-threaded processTraces access - matches production { std::lock_guard lock(process_traces_mutex); - storage->processTraces([](const std::unordered_set& traces) { + storage->processTraces([](const CallTraceSet& traces) { // Process traces (simulating JFR serialization) (void)traces.size(); }); @@ -655,7 +655,7 @@ TEST_F(StressTestSuite, LivenessPurityTest) { size_t preserve_count = trace_ids.size() / 2; std::vector to_preserve(trace_ids.begin(), trace_ids.begin() + preserve_count); - storage->registerLivenessChecker([to_preserve](std::unordered_set& buffer) { + storage->registerLivenessChecker([to_preserve](CallTraceIdSet& buffer) { // Pure callback - no side effects, deterministic output for (u64 trace_id : to_preserve) { buffer.insert(trace_id); @@ -668,7 +668,7 @@ TEST_F(StressTestSuite, LivenessPurityTest) { size_t actual_preserved = 0; { std::lock_guard lock(process_traces_mutex); - storage->processTraces([&](const std::unordered_set& traces) { + storage->processTraces([&](const CallTraceSet& traces) { findMultipleTracesById(traces, to_preserve, actual_preserved); }); } @@ -676,7 +676,7 @@ TEST_F(StressTestSuite, LivenessPurityTest) { preserved_traces.fetch_add(actual_preserved, std::memory_order_relaxed); // Verify deterministic behavior - re-register same callback - storage->registerLivenessChecker([to_preserve](std::unordered_set& buffer) { + storage->registerLivenessChecker([to_preserve](CallTraceIdSet& buffer) { for (u64 trace_id : to_preserve) { buffer.insert(trace_id); } @@ -686,7 +686,7 @@ TEST_F(StressTestSuite, LivenessPurityTest) { size_t second_preserved = 0; { std::lock_guard lock(process_traces_mutex); - storage->processTraces([&](const std::unordered_set& traces) { + storage->processTraces([&](const CallTraceSet& traces) { findMultipleTracesById(traces, to_preserve, second_preserved); }); } @@ -879,7 +879,7 @@ TEST_F(StressTestSuite, TLSOverrunCanaryTest) { try { { std::lock_guard lock(process_traces_mutex); - storage->processTraces([](const std::unordered_set& traces) { + storage->processTraces([](const CallTraceSet& traces) { // Aggressive processing to stress TLS during swaps volatile size_t count = traces.size(); (void)count; @@ -1375,7 +1375,7 @@ TEST_F(StressTestSuite, TeardownFuzzTest) { // Periodic cleanup of storage to simulate real usage patterns if (cycle % 10 == 0) { std::lock_guard lock(process_traces_mutex); - test_storage->processTraces([](const std::unordered_set& traces) { + test_storage->processTraces([](const CallTraceSet& traces) { // Simulate processing collected traces volatile size_t count = traces.size(); (void)count; @@ -1588,7 +1588,7 @@ static void realProfilerSignalStressImpl(int signal_barrage_count, int num_worke // Single-threaded processTraces call - matches production pattern { std::lock_guard lock(StressTestSuite::process_traces_mutex); - signal_storage->processTraces([](const std::unordered_set& traces) { + signal_storage->processTraces([](const CallTraceSet& traces) { volatile size_t count = traces.size(); (void)count; }); @@ -1751,7 +1751,7 @@ TEST_F(StressTestSuite, InstanceIdTraceIdStressTest) { if (op % 100 == 0 && t == 0) { // Only one thread does swaps for (int swap = 0; swap < 3; ++swap) { std::lock_guard lock(process_traces_mutex); - storage->processTraces([](const std::unordered_set& traces) { + storage->processTraces([](const CallTraceSet& traces) { volatile size_t count = traces.size(); (void)count; }); @@ -1778,7 +1778,7 @@ TEST_F(StressTestSuite, InstanceIdTraceIdStressTest) { // Use single shared storage instance for swap { std::lock_guard lock(process_traces_mutex); - shared_storage->processTraces([](const std::unordered_set& traces) { + shared_storage->processTraces([](const CallTraceSet& traces) { // Process traces - this triggers new instance ID assignment volatile size_t count = traces.size(); (void)count; @@ -2401,7 +2401,7 @@ TEST_F(StressTestSuite, ConcurrentExpansionAndCollectStressTest) { for (int cycle = 0; cycle < 3 && !phase2_failed.load(); cycle++) { { std::lock_guard lock(process_traces_mutex); - storage->processTraces([&](const std::unordered_set& traces) { + storage->processTraces([&](const CallTraceSet& traces) { // Sanity: first cycle must contain all traces from Phase 1. if (cycle == 0 && static_cast(traces.size()) < total_inserted.load()) { diff --git a/ddprof-lib/src/test/cpp/stress_threadLifecycle_ut.cpp b/ddprof-lib/src/test/cpp/stress_threadLifecycle_ut.cpp index 5615010e0a..c9dd89924d 100644 --- a/ddprof-lib/src/test/cpp/stress_threadLifecycle_ut.cpp +++ b/ddprof-lib/src/test/cpp/stress_threadLifecycle_ut.cpp @@ -141,7 +141,7 @@ static void churn_worker(ThreadFilter* filter, bool with_dump) { static void dump_thread() { while (g_run.load(std::memory_order_relaxed)) { lock_all(); - g_storage.processTraces([](const std::unordered_set& traces) { + g_storage.processTraces([](const CallTraceSet& traces) { volatile size_t n = 0; for (CallTrace* t : traces) { if (t && t != CallTraceSample::PREPARING) { diff --git a/ddprof-lib/src/test/cpp/test_callTraceStorage.cpp b/ddprof-lib/src/test/cpp/test_callTraceStorage.cpp index 3483461fda..c65fb3a30c 100644 --- a/ddprof-lib/src/test/cpp/test_callTraceStorage.cpp +++ b/ddprof-lib/src/test/cpp/test_callTraceStorage.cpp @@ -21,7 +21,7 @@ static constexpr char TEST_NAME[] = "CallTraceStorageTest"; // Helper function to find a CallTrace by trace_id in an unordered_set -CallTrace* findTraceById(const std::unordered_set& traces, u64 trace_id) { +CallTrace* findTraceById(const CallTraceSet& traces, u64 trace_id) { for (CallTrace* trace : traces) { if (trace && trace->trace_id == trace_id) { return trace; @@ -59,7 +59,7 @@ TEST_F(CallTraceStorageTest, BasicFunctionality) { // Process traces to verify storage bool found_traces = false; - storage->processTraces([&found_traces](const std::unordered_set& traces) { + storage->processTraces([&found_traces](const CallTraceSet& traces) { found_traces = traces.size() > 0; }); EXPECT_TRUE(found_traces); @@ -81,14 +81,14 @@ TEST_F(CallTraceStorageTest, LivenessCheckerRegistration) { // Register a liveness checker that preserves only trace_id2 and trace_id4 u64 preserved_trace_id2 = trace_id2; u64 preserved_trace_id4 = trace_id4; - storage->registerLivenessChecker([&preserved_trace_id2, &preserved_trace_id4](std::unordered_set& buffer) { + storage->registerLivenessChecker([&preserved_trace_id2, &preserved_trace_id4](CallTraceIdSet& buffer) { buffer.insert(preserved_trace_id2); buffer.insert(preserved_trace_id4); }); // processTraces should preserve trace_id2 and trace_id4 but not trace_id1 and trace_id3 size_t traces_collected = 0; - storage->processTraces([&traces_collected](const std::unordered_set& traces) { + storage->processTraces([&traces_collected](const CallTraceSet& traces) { // Should have all 4 traces from the collection plus the dropped trace traces_collected = traces.size(); EXPECT_EQ(traces.size(), 5); @@ -99,7 +99,7 @@ TEST_F(CallTraceStorageTest, LivenessCheckerRegistration) { CallTrace* found_trace2 = nullptr; CallTrace* found_trace4 = nullptr; - storage->processTraces([&](const std::unordered_set& traces) { + storage->processTraces([&](const CallTraceSet& traces) { traces_after_preserve = traces.size(); found_trace2 = findTraceById(traces, preserved_trace_id2); found_trace4 = findTraceById(traces, preserved_trace_id4); @@ -136,16 +136,16 @@ TEST_F(CallTraceStorageTest, MultipleLivenessCheckers) { u64 preserved_id4 = trace_id4; // Register two liveness checkers that preserve non-consecutive traces - storage->registerLivenessChecker([&preserved_id1](std::unordered_set& buffer) { + storage->registerLivenessChecker([&preserved_id1](CallTraceIdSet& buffer) { buffer.insert(preserved_id1); }); - storage->registerLivenessChecker([&preserved_id4](std::unordered_set& buffer) { + storage->registerLivenessChecker([&preserved_id4](CallTraceIdSet& buffer) { buffer.insert(preserved_id4); }); // processTraces should preserve specified traces and swap storages - storage->processTraces([](const std::unordered_set& traces) { + storage->processTraces([](const CallTraceSet& traces) { // Should have all 5 traces from the collection plus the dropped trace EXPECT_EQ(traces.size(), 6); }); @@ -155,7 +155,7 @@ TEST_F(CallTraceStorageTest, MultipleLivenessCheckers) { CallTrace* found_trace4 = nullptr; size_t preserved_count = 0; - storage->processTraces([&](const std::unordered_set& traces) { + storage->processTraces([&](const CallTraceSet& traces) { preserved_count = traces.size(); found_trace1 = findTraceById(traces, preserved_id1); found_trace4 = findTraceById(traces, preserved_id4); @@ -185,13 +185,13 @@ TEST_F(CallTraceStorageTest, TraceIdPreservation) { // Register liveness checker to preserve this trace u64 preserved_id = original_trace_id; - storage->registerLivenessChecker([&preserved_id](std::unordered_set& buffer) { + storage->registerLivenessChecker([&preserved_id](CallTraceIdSet& buffer) { buffer.insert(preserved_id); }); // First process should contain the original trace u64 first_trace_id = 0; - storage->processTraces([&](const std::unordered_set& traces) { + storage->processTraces([&](const CallTraceSet& traces) { EXPECT_EQ(traces.size(), 2); CallTrace* first_trace = findTraceById(traces, original_trace_id); EXPECT_NE(first_trace, nullptr); @@ -201,7 +201,7 @@ TEST_F(CallTraceStorageTest, TraceIdPreservation) { // Second process should still contain the preserved trace with SAME ID u64 preserved_trace_id = 0; - storage->processTraces([&](const std::unordered_set& traces) { + storage->processTraces([&](const CallTraceSet& traces) { EXPECT_EQ(traces.size(), 2); CallTrace* preserved_trace = findTraceById(traces, original_trace_id); EXPECT_NE(preserved_trace, nullptr); @@ -226,7 +226,7 @@ TEST_F(CallTraceStorageTest, ClearMethod) { // Register a liveness checker (should be ignored by clear()) u64 preserved_id = trace_id; - storage->registerLivenessChecker([&preserved_id](std::unordered_set& buffer) { + storage->registerLivenessChecker([&preserved_id](CallTraceIdSet& buffer) { buffer.insert(preserved_id); }); @@ -235,7 +235,7 @@ TEST_F(CallTraceStorageTest, ClearMethod) { // Should have no traces after clear, except for the dropped trace size_t traces_after_clear = 0; - storage->processTraces([&](const std::unordered_set& traces) { + storage->processTraces([&](const CallTraceSet& traces) { traces_after_clear = traces.size(); }); EXPECT_EQ(traces_after_clear, 1); @@ -261,7 +261,7 @@ TEST_F(CallTraceStorageTest, ConcurrentClearAndPut) { // Either way, it shouldn't crash // Verify system is still functional - storage->processTraces([](const std::unordered_set& traces) { + storage->processTraces([](const CallTraceSet& traces) { // No assertion on size since behavior during concurrent operations can vary // The key test is that we don't crash }); @@ -459,7 +459,7 @@ TEST_F(CallTraceStorageTest, RefCountGuardSynchronizationDuringSwap) { // Perform processTraces in separate thread to trigger the storage swap std::thread process_thread([&]() { // Start processing - this will swap storage - storage->processTraces([&](const std::unordered_set& traces) { + storage->processTraces([&](const CallTraceSet& traces) { collection_started = true; // Verify we have traces from the initial population @@ -527,7 +527,7 @@ TEST_F(CallTraceStorageTest, RefCountGuardSynchronizationDuringSwap) { // Final verification: ensure we can still process traces std::atomic final_trace_count{0}; - storage->processTraces([&](const std::unordered_set& traces) { + storage->processTraces([&](const CallTraceSet& traces) { final_trace_count = static_cast(traces.size()); }); @@ -586,7 +586,7 @@ TEST_F(CallTraceStorageTest, UseAfterFreeInProcessTraces) { // This causes traces to be copied to SCRATCH during processTraces(). // After rotation, SCRATCH becomes STANDBY, so the NEXT processTraces() // will have these traces in STANDBY where the bug manifests. - storage->registerLivenessChecker([&trace_ids](std::unordered_set& buffer) { + storage->registerLivenessChecker([&trace_ids](CallTraceIdSet& buffer) { for (u64 id : trace_ids) { buffer.insert(id); } @@ -595,7 +595,7 @@ TEST_F(CallTraceStorageTest, UseAfterFreeInProcessTraces) { // First processTraces: traces are in ACTIVE, get collected and preserved to SCRATCH. // After rotation: SCRATCH becomes STANDBY (now contains preserved traces) int first_count = 0; - storage->processTraces([&first_count](const std::unordered_set& traces) { + storage->processTraces([&first_count](const CallTraceSet& traces) { first_count = traces.size(); printf("First processTraces: %d traces collected\n", first_count); }); @@ -606,7 +606,7 @@ TEST_F(CallTraceStorageTest, UseAfterFreeInProcessTraces) { // 2. Standby traces are collected into _traces_buffer (raw pointers) // 3. original_standby->clear() - FREES the trace memory! // 4. processor(_traces_buffer) - accesses FREED memory (USE-AFTER-FREE!) - storage->processTraces([&](const std::unordered_set& traces) { + storage->processTraces([&](const CallTraceSet& traces) { int total_frames = 0; int total_bci_sum = 0; int trace_count = 0; @@ -648,7 +648,7 @@ TEST_F(CallTraceStorageTest, UseAfterFreeInProcessTraces) { // Third processTraces: traces should still be preserved (copied to new scratch) // This further exercises the use-after-free if the bug exists - storage->processTraces([&](const std::unordered_set& traces) { + storage->processTraces([&](const CallTraceSet& traces) { int trace_count = 0; for (CallTrace* trace : traces) { if (trace == nullptr) continue; @@ -759,7 +759,7 @@ TEST_F(CallTraceStorageTest, LivenessPreservationAcrossMultipleCycles) { } // Liveness checker marks every stored trace as live. - storage->registerLivenessChecker([&ids](std::unordered_set& buf) { + storage->registerLivenessChecker([&ids](CallTraceIdSet& buf) { for (u64 id : ids) buf.insert(id); }); @@ -767,7 +767,7 @@ TEST_F(CallTraceStorageTest, LivenessPreservationAcrossMultipleCycles) { for (int cycle = 0; cycle < CYCLES; cycle++) { std::atomic done{false}; std::thread t([&] { - storage->processTraces([&](const std::unordered_set& traces) { + storage->processTraces([&](const CallTraceSet& traces) { // All N preserved traces must be present, plus the dropped sentinel. EXPECT_GE(traces.size(), static_cast(N + 1)) << "cycle " << cycle << ": too few traces"; @@ -830,7 +830,7 @@ TEST_F(CallTraceStorageTest, ClearTableOnlyDisconnectsFullChain) { // processTraces() performs the rotation including clearTableOnly(); run it once // to expose defect C. int count = 0; - storage->processTraces([&](const std::unordered_set& traces) { + storage->processTraces([&](const CallTraceSet& traces) { count = static_cast(traces.size()); }); // At least NUM_TRACES + the static dropped-trace sentinel should be present. @@ -840,7 +840,7 @@ TEST_F(CallTraceStorageTest, ClearTableOnlyDisconnectsFullChain) { // This deterministically detects defect C — if clearTableOnly() left stale entries // in freed memory that somehow end up in the new table, count2 would be wrong. int count2 = 0; - storage->processTraces([&](const std::unordered_set& traces) { + storage->processTraces([&](const CallTraceSet& traces) { count2 = static_cast(traces.size()); }); EXPECT_EQ(count2, 1); // only the dropped-trace sentinel; no stale entries @@ -863,7 +863,7 @@ TEST_F(CallTraceStorageTest, CollectFindsAllTracesAcrossExpandedChain) { } std::unordered_set seen_ids; - storage->processTraces([&](const std::unordered_set& traces) { + storage->processTraces([&](const CallTraceSet& traces) { for (CallTrace* t : traces) { if (t) seen_ids.insert(t->trace_id); }