From 30e822bebc25f3547ad4b2048057dcbda9f860d8 Mon Sep 17 00:00:00 2001 From: r1viollet Date: Wed, 22 Mar 2023 22:09:46 +0100 Subject: [PATCH 01/21] Keep track of live allocated memory - Ensure we instrument frees - Introduce a live allocation object --- include/ddprof_context.hpp | 1 + include/ddprof_input.hpp | 4 +- include/ddprof_perf_event.hpp | 27 ++++ include/ddprof_worker_context.hpp | 2 + include/ipc.hpp | 2 + include/lib/allocation_tracker.hpp | 30 +++- include/live_allocation.hpp | 43 ++++++ include/unwind_output.hpp | 14 +- include/unwind_state.hpp | 3 +- src/ddprof_context_lib.cc | 2 + src/ddprof_input.cc | 2 + src/ddprof_worker.cc | 128 +++++++++++++++-- src/exe/main.cc | 4 + src/ipc.cc | 5 +- src/lib/allocation_tracker.cc | 118 +++++++++++++-- src/lib/dd_profiling.cc | 7 + src/lib/malloc_wrapper.cc | 222 +++++++++++++++++++++++++++++ src/pprof/ddprof_pprof.cc | 4 +- src/unwind.cc | 9 +- src/unwind_dwfl.cc | 18 +-- src/unwind_helpers.cc | 30 ++-- src/unwind_output.cc | 19 --- test/CMakeLists.txt | 5 +- test/allocation_tracker-ut.cc | 91 ++++++++---- test/savecontext-ut.cc | 12 +- test/self_unwind/self_unwind.cc | 2 +- test/simple_malloc-ut.sh | 3 + test/simple_malloc.cc | 40 +++++- test/unwind_output_mock.hpp | 8 +- 29 files changed, 720 insertions(+), 135 deletions(-) create mode 100644 include/ddprof_perf_event.hpp create mode 100644 include/live_allocation.hpp create mode 100644 src/lib/malloc_wrapper.cc delete mode 100644 src/unwind_output.cc diff --git a/include/ddprof_context.hpp b/include/ddprof_context.hpp index e73edb716..426d8cc30 100644 --- a/include/ddprof_context.hpp +++ b/include/ddprof_context.hpp @@ -32,6 +32,7 @@ typedef struct DDProfContext { int sockfd; bool wait_on_socket; bool show_samples; + bool live_allocations; // for now this overrides cpu_set_t cpu_affinity; const char *switch_user; const char *internal_stats; diff --git a/include/ddprof_input.hpp b/include/ddprof_input.hpp index 5a0e8a8f7..8cf3b0141 100644 --- a/include/ddprof_input.hpp +++ b/include/ddprof_input.hpp @@ -37,6 +37,7 @@ typedef struct DDProfInput { char *socket; char *preset; char *switch_user; + char *live_allocations; // Watcher presets PerfWatcher watchers[MAX_TYPE_WATCHER]; int num_watchers; @@ -100,7 +101,8 @@ typedef struct DDProfInput { XX(DD_PROFILING_NATIVE_PRESET, preset, D, 'D', 1, input, NULL, "", ) \ XX(DD_PROFILING_NATIVE_SHOW_SAMPLES, show_samples, y, 'y', 0, input, NULL, "", ) \ XX(DD_PROFILING_NATIVE_CPU_AFFINITY, affinity, a, 'a', 1, input, NULL, "", ) \ - XX(DD_PROFILING_NATIVE_SWITCH_USER, switch_user, W, 'W', 1, input, NULL, "", ) + XX(DD_PROFILING_NATIVE_SWITCH_USER, switch_user, W, 'W', 1, input, NULL, "", ) \ + XX(DD_PROFILING_NATIVE_LIVE_ALLOC, live_allocations, k, 'k', 1, input, NULL, "no", ) // clang-format on #define X_ENUM(a, b, c, d, e, f, g, h, i) a, diff --git a/include/ddprof_perf_event.hpp b/include/ddprof_perf_event.hpp new file mode 100644 index 000000000..d64c6b88a --- /dev/null +++ b/include/ddprof_perf_event.hpp @@ -0,0 +1,27 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. This product includes software +// developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present +// Datadog, Inc. + +#pragma once + +#include +#include + +// Extend the perf event types +// There are <30 different perf events (starting at 1000 seems safe) +constexpr uint32_t PERF_CUSTOM_EVENT_DEALLOCATION = 1000; + +static_assert(PERF_CUSTOM_EVENT_DEALLOCATION > PERF_RECORD_MAX, + "Error from PERF_CUSTOM_EVENT_DEALLOCATION definition"); + +namespace ddprof { + +// Custom sample type +struct DeallocationEvent { + perf_event_header hdr; + struct sample_id sample_id; + uintptr_t ptr; +}; + +} // namespace ddprof diff --git a/include/ddprof_worker_context.hpp b/include/ddprof_worker_context.hpp index de05c3dca..f7c969ace 100644 --- a/include/ddprof_worker_context.hpp +++ b/include/ddprof_worker_context.hpp @@ -5,6 +5,7 @@ #pragma once +#include "live_allocation.hpp" #include "pevent.hpp" #include "proc_status.hpp" @@ -36,4 +37,5 @@ struct DDProfWorkerContext { int64_t send_nanos; // Last time an export was sent uint32_t count_worker; // exports since last cache clear std::array lost_events_per_watcher; + ddprof::LiveAllocation live_allocation; }; diff --git a/include/ipc.hpp b/include/ipc.hpp index 47ed8c507..330aa717e 100644 --- a/include/ipc.hpp +++ b/include/ipc.hpp @@ -94,6 +94,7 @@ struct RingBufferInfo { }; struct ReplyMessage { + enum { kLiveAllocation = 0 }; // reply with the request flags from the request uint32_t request = 0; // profiler pid @@ -102,6 +103,7 @@ struct ReplyMessage { // RingBufferInfo is returned if request & kRingBuffer // cppcheck-suppress unusedStructMember RingBufferInfo ring_buffer; + int32_t allocation_flags = 0; }; class Client { diff --git a/include/lib/allocation_tracker.hpp b/include/lib/allocation_tracker.hpp index 7da487222..0e29deaf2 100644 --- a/include/lib/allocation_tracker.hpp +++ b/include/lib/allocation_tracker.hpp @@ -15,6 +15,7 @@ #include #include #include +#include namespace ddprof { @@ -38,6 +39,8 @@ class AllocationTracker { AllocationTracker(const AllocationTracker &) = delete; AllocationTracker &operator=(const AllocationTracker &) = delete; + ~AllocationTracker() { free(); } + enum AllocationTrackingFlags { kTrackDeallocations = 0x1, kDeterministicSampling = 0x2 @@ -60,6 +63,8 @@ class AllocationTracker { static inline bool is_active(); private: + using AdressSet = std::unordered_set; + struct TrackerState { std::mutex mutex; std::atomic track_allocations = false; @@ -82,16 +87,23 @@ class AllocationTracker { TrackerThreadLocalState &tl_state); void track_deallocation(uintptr_t addr, TrackerThreadLocalState &tl_state); - DDRes push_sample(uint64_t allocated_size, TrackerThreadLocalState &tl_state); + DDRes push_sample(uintptr_t addr, uint64_t allocated_size, + TrackerThreadLocalState &tl_state); // Return true if consumer should be notified DDRes push_lost_sample(MPSCRingBufferWriter &writer, bool ¬ify_needed); + // Return true if consumer should be notified + DDRes push_dealloc_sample(uintptr_t addr, TrackerThreadLocalState &tl_state); + + void free_on_consecutive_failures(bool success); + TrackerState _state; uint64_t _sampling_interval; std::mt19937 _gen; PEvent _pevent; bool _deterministic_sampling; + AdressSet _address_set; static thread_local TrackerThreadLocalState _tl_state; static AllocationTracker *_instance; @@ -128,7 +140,21 @@ void AllocationTracker::track_allocation(uintptr_t addr, size_t size) { } } -void AllocationTracker::track_deallocation(uintptr_t) {} +void AllocationTracker::track_deallocation(uintptr_t addr) { + // same pattern as track_allocation + AllocationTracker *instance = _instance; + + if (!instance) { + return; + } + TrackerThreadLocalState &tl_state = _tl_state; + + if (instance->_state.track_deallocations.load(std::memory_order_relaxed)) { + // not cool as we are always calling this (high overhead). Can we do better + // ? + instance->track_deallocation(addr, tl_state); + } +} bool AllocationTracker::is_active() { auto instance = _instance; diff --git a/include/live_allocation.hpp b/include/live_allocation.hpp new file mode 100644 index 000000000..821545c27 --- /dev/null +++ b/include/live_allocation.hpp @@ -0,0 +1,43 @@ +#pragma once + +#include "ddprof_defs.hpp" +#include "logger.hpp" +#include "unwind_output.hpp" + +#include + +namespace ddprof { + +class LiveAllocation { +public: + static constexpr auto kMaxTracked = 200000; + void register_allocation(const UnwindOutput &stack, uintptr_t addr, + size_t size, int watcher_pos, pid_t pid) { + StackMap &stack_map = _pid_map[pid]; + stack_map[addr] = AllocationInfo{ + ._stack = stack, ._size = size, ._watcher_pos = watcher_pos}; + } + + void register_deallocation(uintptr_t addr, pid_t pid) { + StackMap &stack_map = _pid_map[pid]; + if (!stack_map.erase(addr)) { + LG_DBG("Unmatched deallocation at %lx of PID%d", addr, pid); + } + } + + struct AllocationInfo { + UnwindOutput _stack; + size_t _size; + // Should the watcher be part of the key ? + // In theory we could watch allocations with different rules (per watcher) + // for now I'll leave it here + int _watcher_pos; + }; + + using StackMap = std::unordered_map; + using PidMap = std::unordered_map; + + PidMap _pid_map; +}; + +} // namespace ddprof \ No newline at end of file diff --git a/include/unwind_output.hpp b/include/unwind_output.hpp index 72e8e7bb6..c408432c7 100644 --- a/include/unwind_output.hpp +++ b/include/unwind_output.hpp @@ -8,6 +8,7 @@ #pragma once #include +#include #include "ddprof_defs.hpp" #include "string_view.hpp" @@ -18,12 +19,13 @@ typedef struct FunLoc { MapInfoIdx_t _map_info_idx; } FunLoc; -typedef struct UnwindOutput { - FunLoc locs[DD_MAX_STACK_DEPTH]; - uint64_t nb_locs; +struct UnwindOutput { + void clear() { + locs.clear(); + is_incomplete = true; + } + std::vector locs; int pid; int tid; bool is_incomplete; -} UnwindOutput; - -void uw_output_clear(UnwindOutput *); +}; diff --git a/include/unwind_state.hpp b/include/unwind_state.hpp index f1bc7626a..57b415991 100644 --- a/include/unwind_state.hpp +++ b/include/unwind_state.hpp @@ -39,7 +39,8 @@ struct UnwindState { explicit UnwindState(int dd_profiling_fd = -1) : _dwfl_wrapper(nullptr), dso_hdr("", dd_profiling_fd), pid(-1), stack(nullptr), stack_sz(0), current_ip(0) { - uw_output_clear(&output); + output.clear(); + output.locs.reserve(DD_MAX_STACK_DEPTH); } ddprof::DwflHdr dwfl_hdr; diff --git a/src/ddprof_context_lib.cc b/src/ddprof_context_lib.cc index 697be345d..78a18c6dd 100644 --- a/src/ddprof_context_lib.cc +++ b/src/ddprof_context_lib.cc @@ -313,6 +313,8 @@ DDRes ddprof_context_set(DDProfInput *input, DDProfContext *ctx) { } ctx->params.show_samples = input->show_samples != nullptr; + ctx->params.live_allocations = + arg_yesno(input->live_allocations, 1); // default no if (input->switch_user) { ctx->params.switch_user = strdup(input->switch_user); diff --git a/src/ddprof_input.cc b/src/ddprof_input.cc index 4aefcf698..6ac6c870c 100644 --- a/src/ddprof_input.cc +++ b/src/ddprof_input.cc @@ -147,6 +147,8 @@ const char* help_str[DD_KLEN] = { [DD_PROFILING_NATIVE_CPU_AFFINITY] = STR_UNDF, [DD_PROFILING_NATIVE_SWITCH_USER] = " Run the target process under the given user.\n", + [DD_PROFILING_NATIVE_LIVE_ALLOC] = + " Report only allocations that were not matched with a free.\n", }; // clang-format on diff --git a/src/ddprof_worker.cc b/src/ddprof_worker.cc index 497411bf7..6f3d47304 100644 --- a/src/ddprof_worker.cc +++ b/src/ddprof_worker.cc @@ -6,6 +6,7 @@ #include "ddprof_worker.hpp" #include "ddprof_context.hpp" +#include "ddprof_perf_event.hpp" #include "ddprof_stats.hpp" #include "dso_hdr.hpp" #include "dwfl_hdr.hpp" @@ -60,7 +61,7 @@ static DDRes report_lost_events(DDProfContext *ctx) { if (ctx->worker_ctx.lost_events_per_watcher[watcher_idx] > 0) { PerfWatcher *watcher = &ctx->watchers[watcher_idx]; UnwindState *us = ctx->worker_ctx.us; - uw_output_clear(&us->output); + us->output.clear(); add_common_frame(us, SymbolErrors::lost_event); LG_WRN("Reporting #%lu -> [%lu] lost samples for watcher #%d", ctx->worker_ctx.lost_events_per_watcher[watcher_idx], @@ -221,19 +222,14 @@ static DDRes worker_update_stats(ProcStatus *procstat, const UnwindState &us, return ddres_init(); } -/************************* perf_event_open() helpers **************************/ -/// Entry point for sample aggregation -DDRes ddprof_pr_sample(DDProfContext *ctx, perf_event_sample *sample, - int watcher_pos) { - if (!sample) - return ddres_warn(DD_WHAT_PERFSAMP); +static DDRes ddprof_unwind_sample(DDProfContext *ctx, perf_event_sample *sample, + int watcher_pos) { struct UnwindState *us = ctx->worker_ctx.us; PerfWatcher *watcher = &ctx->watchers[watcher_pos]; ddprof_stats_add(STATS_SAMPLE_COUNT, 1, NULL); ddprof_stats_add(STATS_UNWIND_AVG_STACK_SIZE, sample->size_stack, nullptr); - auto ticks0 = ddprof::get_tsc_cycles(); // copy the sample context into the unwind structure unwind_init_sample(us, sample->regs, sample->pid, sample->size_stack, sample->data_stack); @@ -275,12 +271,32 @@ DDRes ddprof_pr_sample(DDProfContext *ctx, perf_event_sample *sample, ddprof_stats_add(STATS_UNWIND_TRUNCATED_INPUT, 1, nullptr); } + return res; +} + +/************************* perf_event_open() helpers **************************/ +/// Entry point for sample aggregation +DDRes ddprof_pr_sample(DDProfContext *ctx, perf_event_sample *sample, + int watcher_pos) { + if (!sample) + return ddres_warn(DD_WHAT_PERFSAMP); + + // If this is a SW_TASK_CLOCK-type event, then aggregate the time + if (ctx->watchers[watcher_pos].config == PERF_COUNT_SW_TASK_CLOCK) + ddprof_stats_add(STATS_TARGET_CPU_USAGE, sample->period, NULL); + + auto ticks0 = ddprof::get_tsc_cycles(); + DDRes res = ddprof_unwind_sample(ctx, sample, watcher_pos); auto unwind_ticks = ddprof::get_tsc_cycles(); - DDRES_CHECK_FWD( - ddprof_stats_add(STATS_UNWIND_AVG_TIME, unwind_ticks - ticks0, NULL)); + ddprof_stats_add(STATS_UNWIND_AVG_TIME, unwind_ticks - ticks0, NULL); + + // Usually we want to send the sample_val, but sometimes we need to process + // the event to get the desired value + PerfWatcher *watcher = &ctx->watchers[watcher_pos]; // Aggregate if unwinding went well (todo : fatal error propagation) if (!IsDDResFatal(res) && EventConfMode::kCallgraph <= watcher->output_mode) { + struct UnwindState *us = ctx->worker_ctx.us; #ifndef DDPROF_NATIVE_LIB // Depending on the type of watcher, compute a value for sample uint64_t sample_val = perf_value_from_sample(watcher, sample); @@ -306,13 +322,38 @@ DDRes ddprof_pr_sample(DDProfContext *ctx, perf_event_sample *sample, #endif } - DDRES_CHECK_FWD(ddprof_stats_add(STATS_AGGREGATION_AVG_TIME, - ddprof::get_tsc_cycles() - unwind_ticks, - NULL)); + ddprof_stats_add(STATS_AGGREGATION_AVG_TIME, + ddprof::get_tsc_cycles() - unwind_ticks, NULL); return {}; } +DDRes ddprof_pr_allocation_tracking(DDProfContext *ctx, + perf_event_sample *sample, + int watcher_pos) { + if (!sample) + return ddres_warn(DD_WHAT_PERFSAMP); + + // If this is a SW_TASK_CLOCK-type event, then aggregate the time + if (ctx->watchers[watcher_pos].config == PERF_COUNT_SW_TASK_CLOCK) + ddprof_stats_add(STATS_TARGET_CPU_USAGE, sample->period, NULL); + + auto ticks0 = ddprof::get_tsc_cycles(); + DDRes res = ddprof_unwind_sample(ctx, sample, watcher_pos); + auto unwind_ticks = ddprof::get_tsc_cycles(); + ddprof_stats_add(STATS_UNWIND_AVG_TIME, unwind_ticks - ticks0, NULL); + + // Aggregate if unwinding went well (todo : fatal error propagation) + if (!IsDDResFatal(res)) { + struct UnwindState *us = ctx->worker_ctx.us; + ctx->worker_ctx.live_allocation.register_allocation( + us->output, sample->addr, sample->period, watcher_pos, sample->pid); + } + + // TODO: propagate fatal + return ddres_init(); +} + static void ddprof_reset_worker_stats() { for (unsigned i = 0; i < std::size(s_cycled_stats); ++i) { ddprof_stats_clear(s_cycled_stats[i]); @@ -340,10 +381,50 @@ void *ddprof_worker_export_thread(void *arg) { } #endif +#ifndef DDPROF_NATIVE_LIB +static DDRes aggregate_stack(const LiveAllocation::AllocationInfo &alloc_info, + DDProfContext *ctx) { + struct UnwindState *us = ctx->worker_ctx.us; + int watcher_pos = alloc_info._watcher_pos; + PerfWatcher *watcher = &ctx->watchers[watcher_pos]; + int i_export = ctx->worker_ctx.i_current_pprof; + DDProfPProf *pprof = ctx->worker_ctx.pprof[i_export]; + DDRES_CHECK_FWD(pprof_aggregate(&alloc_info._stack, &us->symbol_hdr, + alloc_info._size, 1, watcher, pprof)); + if (ctx->params.show_samples) { + ddprof_print_sample(alloc_info._stack, us->symbol_hdr, alloc_info._size, + *watcher); + } + return ddres_init(); +} + +static DDRes aggregate_live_allocations(DDProfContext *ctx) { + // this would be more efficient if we could reuse the same stacks in + // libdatadog + LiveAllocation &live_allocations = ctx->worker_ctx.live_allocation; + for (auto &stack_map : live_allocations._pid_map) { + for (const auto &alloc_info_pair : stack_map.second) { + DDRES_CHECK_FWD(aggregate_stack(alloc_info_pair.second, ctx)); + } + LG_NTC("Number of Live allocations for PID%d = %lu ", stack_map.first, + stack_map.second.size()); + // Safety to avoid spending all the time reporting allocations + if (stack_map.second.size() >= LiveAllocation::kMaxTracked) { + stack_map.second.clear(); + } + } + return ddres_init(); +} +#endif + /// Cycle operations : export, sync metrics, update counters DDRes ddprof_worker_cycle(DDProfContext *ctx, int64_t now, [[maybe_unused]] bool synchronous_export) { + #ifndef DDPROF_NATIVE_LIB + // TODO: lib mode (unhandled for now) + DDRES_CHECK_FWD(aggregate_live_allocations(ctx)); + // Take the current pprof contents and ship them to the backend. This also // clears the pprof for reuse // Dispatch happens in a thread, with the underlying data structure for @@ -482,6 +563,12 @@ void ddprof_pr_exit(DDProfContext *ctx, const perf_event_exit *ext, } } +void ddprof_pr_deallocation(DDProfContext *ctx, + const DeallocationEvent *event) { + ctx->worker_ctx.live_allocation.register_deallocation(event->ptr, + event->sample_id.pid); +} + /********************************** callbacks *********************************/ DDRes ddprof_worker_maybe_export(DDProfContext *ctx, int64_t now_ns) { try { @@ -589,9 +676,19 @@ DDRes ddprof_worker_process_event(const perf_event_header *hdr, int watcher_pos, case PERF_RECORD_SAMPLE: if (wpid->pid) { uint64_t mask = ctx->watchers[watcher_pos].sample_type; + bool is_allocation = + (ctx->watchers[watcher_pos].type == kDDPROF_TYPE_CUSTOM && + ctx->watchers[watcher_pos].config == kDDPROF_COUNT_ALLOCATIONS); + if (is_allocation) // temp hack + mask |= PERF_SAMPLE_ADDR; perf_event_sample *sample = hdr2samp(hdr, mask); if (sample) { - DDRES_CHECK_FWD(ddprof_pr_sample(ctx, sample, watcher_pos)); + if (is_allocation && ctx->params.live_allocations) { + DDRES_CHECK_FWD( + ddprof_pr_allocation_tracking(ctx, sample, watcher_pos)); + } else { + DDRES_CHECK_FWD(ddprof_pr_sample(ctx, sample, watcher_pos)); + } } } break; @@ -621,6 +718,9 @@ DDRes ddprof_worker_process_event(const perf_event_header *hdr, int watcher_pos, ddprof_pr_lost(ctx, reinterpret_cast(hdr), watcher_pos); break; + case PERF_CUSTOM_EVENT_DEALLOCATION: + ddprof_pr_deallocation(ctx, + reinterpret_cast(hdr)); default: break; } diff --git a/src/exe/main.cc b/src/exe/main.cc index 828b0f766..346adb9fb 100644 --- a/src/exe/main.cc +++ b/src/exe/main.cc @@ -356,6 +356,10 @@ static int start_profiler_internal(DDProfContext *ctx, bool &is_profiler) { static_cast(event_it->ring_buffer_type); reply.allocation_profiling_rate = ctx->watchers[alloc_watcher_idx].sample_period; + if (ctx->params.live_allocations) { + reply.allocation_flags |= + (1 << ddprof::ReplyMessage::kLiveAllocation); + } } } diff --git a/src/ipc.cc b/src/ipc.cc index 10425bbcb..5bf0199b2 100644 --- a/src/ipc.cc +++ b/src/ipc.cc @@ -24,6 +24,7 @@ struct InternalResponseMessage { int64_t mem_size; int64_t allocation_profiling_rate; int32_t ring_buffer_type; + int32_t allocation_flags; }; struct timeval to_timeval(std::chrono::microseconds duration) noexcept { @@ -257,7 +258,8 @@ DDRes send(UnixSocket &socket, const ReplyMessage &msg) { .pid = msg.pid, .mem_size = msg.ring_buffer.mem_size, .allocation_profiling_rate = msg.allocation_profiling_rate, - .ring_buffer_type = msg.ring_buffer.ring_buffer_type}; + .ring_buffer_type = msg.ring_buffer.ring_buffer_type, + .allocation_flags = msg.allocation_flags}; socket.send(to_byte_span(&data), fd_span, ec); DDRES_CHECK_ERRORCODE(ec, DD_WHAT_SOCKET, "Unable to send response message"); return {}; @@ -291,6 +293,7 @@ DDRes receive(UnixSocket &socket, ReplyMessage &msg) { msg.ring_buffer.ring_buffer_type = data.ring_buffer_type; msg.ring_buffer.ring_fd = fds[0]; msg.ring_buffer.event_fd = fds[1]; + msg.allocation_flags = data.allocation_flags; return {}; } diff --git a/src/lib/allocation_tracker.cc b/src/lib/allocation_tracker.cc index ea4532f5d..4e16e7424 100644 --- a/src/lib/allocation_tracker.cc +++ b/src/lib/allocation_tracker.cc @@ -5,6 +5,7 @@ #include "allocation_tracker.hpp" +#include "ddprof_perf_event.hpp" #include "ddres.hpp" #include "defer.hpp" #include "ipc.hpp" @@ -27,10 +28,10 @@ namespace ddprof { struct AllocationEvent { perf_event_header hdr; struct sample_id sample_id; + uint64_t addr; /* if PERF_SAMPLE_ADDR */ uint64_t period; - uint64_t abi; /* if PERF_SAMPLE_REGS_USER */ - uint64_t regs[PERF_REGS_COUNT]; - /* if PERF_SAMPLE_REGS_USER */ + uint64_t abi; /* if PERF_SAMPLE_REGS_USER */ + uint64_t regs[PERF_REGS_COUNT]; /* if PERF_SAMPLE_REGS_USER */ uint64_t size; /* if PERF_SAMPLE_STACK_USER */ std::byte data[PERF_SAMPLE_STACK_SIZE]; /* if PERF_SAMPLE_STACK_USER */ uint64_t dyn_size; /* if PERF_SAMPLE_STACK_USER && @@ -139,7 +140,21 @@ void AllocationTracker::allocation_tracking_free() { instance->free(); } -void AllocationTracker::track_allocation(uintptr_t, size_t size, +void AllocationTracker::free_on_consecutive_failures(bool success) { + if (!success) { + ++_state.failure_count; + if (_state.failure_count >= k_max_consecutive_failures) { + // Too many errors during ring buffer operation: stop allocation profiling + free(); + } + } else { + if (_state.failure_count.load(std::memory_order_relaxed) > 0) { + _state.failure_count = 0; + } + } +} + +void AllocationTracker::track_allocation(uintptr_t addr, size_t size, TrackerThreadLocalState &tl_state) { // Prevent reentrancy to avoid dead lock on mutex ReentryGuard guard(&tl_state.reentry_guard); @@ -184,16 +199,34 @@ void AllocationTracker::track_allocation(uintptr_t, size_t size, tl_state.remaining_bytes = remaining_bytes; uint64_t total_size = nsamples * sampling_interval; - if (!IsDDResOK(push_sample(total_size, tl_state))) { - ++_state.failure_count; - if (_state.failure_count >= k_max_consecutive_failures) { - // Too many errors during ring buffer operation: stop allocation profiling - free(); - } - } else { - if (_state.failure_count.load(std::memory_order_relaxed) > 0) { - _state.failure_count = 0; - } + bool success = IsDDResOK(push_sample(addr, total_size, tl_state)); + free_on_consecutive_failures(success); + + if (success) { // ensure we track this dealloc if it occurs + _address_set.insert(addr); + } +} + +void AllocationTracker::track_deallocation(uintptr_t addr, + TrackerThreadLocalState &tl_state) { + // Prevent reentrancy to avoid dead lock on mutex + ReentryGuard guard(&tl_state.reentry_guard); + + if (!guard) { + // This is an internal dealloc, so we don't need to keep track of this + return; + } + std::lock_guard lock{_state.mutex}; + + // recheck if profiling is enabled + if (!_state.track_deallocations) { + return; + } + + // Inserting / Erasing addresses is done within the lock + if (_address_set.erase(addr)) { + bool success = IsDDResOK(push_dealloc_sample(addr, tl_state)); + free_on_consecutive_failures(success); } } @@ -225,7 +258,61 @@ DDRes AllocationTracker::push_lost_sample(MPSCRingBufferWriter &writer, return {}; } -DDRes AllocationTracker::push_sample(uint64_t allocated_size, +// Return true if consumer should be notified +DDRes AllocationTracker::push_dealloc_sample( + uintptr_t addr, TrackerThreadLocalState &tl_state) { + MPSCRingBufferWriter writer{_pevent.rb}; + bool notify_consumer{false}; + + bool timeout = false; + if (unlikely(_state.lost_count.load(std::memory_order_relaxed))) { + DDRES_CHECK_FWD(push_lost_sample(writer, notify_consumer)); + } + + auto buffer = writer.reserve(sizeof(DeallocationEvent), &timeout); + if (buffer.empty()) { + // ring buffer is full, increase lost count + _state.lost_count.fetch_add(1, std::memory_order_acq_rel); + + if (timeout) { + DDRES_RETURN_ERROR_LOG(DD_WHAT_PERFRB, + "Unable to get write lock on ring buffer"); + } + // not an error + return {}; + } + + DeallocationEvent *event = + reinterpret_cast(buffer.data()); + event->hdr.misc = 0; + event->hdr.size = sizeof(DeallocationEvent); + event->hdr.type = PERF_CUSTOM_EVENT_DEALLOCATION; + event->sample_id.time = 0; + + if (_state.pid == 0) { + _state.pid = getpid(); + } + if (tl_state.tid == 0) { + tl_state.tid = ddprof::gettid(); + } + event->sample_id.pid = _state.pid; + event->sample_id.tid = tl_state.tid; + + // address of dealloc + event->ptr = addr; + + if (writer.commit(buffer) || notify_consumer) { + uint64_t count = 1; + if (write(_pevent.fd, &count, sizeof(count)) != sizeof(count)) { + DDRES_RETURN_ERROR_LOG(DD_WHAT_PERFRB, + "Error writing to memory allocation eventfd (%s)", + strerror(errno)); + } + } + return {}; +} + +DDRes AllocationTracker::push_sample(uintptr_t addr, uint64_t allocated_size, TrackerThreadLocalState &tl_state) { MPSCRingBufferWriter writer{_pevent.rb}; bool notify_consumer{false}; @@ -255,6 +342,7 @@ DDRes AllocationTracker::push_sample(uint64_t allocated_size, event->hdr.type = PERF_RECORD_SAMPLE; event->abi = PERF_SAMPLE_REGS_ABI_64; event->sample_id.time = 0; + event->addr = addr; if (_state.pid == 0) { _state.pid = getpid(); diff --git a/src/lib/dd_profiling.cc b/src/lib/dd_profiling.cc index cb5493444..457002a22 100644 --- a/src/lib/dd_profiling.cc +++ b/src/lib/dd_profiling.cc @@ -249,6 +249,13 @@ int ddprof_start_profiling_internal() { flags |= ddprof::AllocationTracker::kDeterministicSampling; info.allocation_profiling_rate = -info.allocation_profiling_rate; } + + if (info.allocation_flags & + (1 << ddprof::ReplyMessage::kLiveAllocation)) { + // tracking deallocations to allow a live view + flags |= ddprof::AllocationTracker::kTrackDeallocations; + } + if (IsDDResOK(ddprof::AllocationTracker::allocation_tracking_init( info.allocation_profiling_rate, flags, info.ring_buffer))) { // \fixme{nsavoire} pthread_create should probably be overridden diff --git a/src/lib/malloc_wrapper.cc b/src/lib/malloc_wrapper.cc new file mode 100644 index 000000000..e1797106b --- /dev/null +++ b/src/lib/malloc_wrapper.cc @@ -0,0 +1,222 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. This product includes software +// developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present +// Datadog, Inc. + +#include "allocation_tracker.hpp" +#include "ddprof_base.hpp" +#include "unlikely.hpp" + +#include +#include +#include +#include +#include +#include + +// Declaration of reallocarray is only available starting from glibc 2.28 +extern "C" { +#ifdef __llvm__ +void *reallocarray(void *ptr, size_t nmemb, size_t size) noexcept; +#else +void *reallocarray(void *ptr, size_t nmemb, size_t size); +#endif +void *pvalloc(size_t size) noexcept; + +void *temp_malloc(size_t size) noexcept; +void temp_free(void *ptr) noexcept; +void *temp_calloc(size_t nmemb, size_t size) noexcept; +void *temp_realloc(void *ptr, size_t size) noexcept; +int temp_posix_memalign(void **memptr, size_t alignment, size_t size) noexcept; +void *temp_aligned_alloc(size_t alignment, size_t size) noexcept; +void *temp_memalign(size_t alignment, size_t size) noexcept; +void *temp_pvalloc(size_t size) noexcept; +void *temp_valloc(size_t size) noexcept; +void *temp_reallocarray(void *ptr, size_t nmemb, size_t size) noexcept; +} + +#define ORIGINAL_FUNC(name) get_next(#name) +#define DECLARE_FUNC(name) decltype(&::name) s_##name = &temp_##name; + +template F get_next(const char *name) { + auto *func = reinterpret_cast(dlsym(RTLD_NEXT, name)); + return func; +} + +DECLARE_FUNC(malloc); +DECLARE_FUNC(calloc); +DECLARE_FUNC(realloc); +DECLARE_FUNC(free); +DECLARE_FUNC(posix_memalign); +DECLARE_FUNC(aligned_alloc); +DECLARE_FUNC(reallocarray); +// obsolete allocation functions +DECLARE_FUNC(memalign); +DECLARE_FUNC(pvalloc); +DECLARE_FUNC(valloc); + +namespace { +DDPROF_NOINLINE void init(); + +// calloc is invoked by dlsym, returning a null value in this case is well +// handled by glibc +void *temp_calloc2(size_t, size_t) noexcept { return nullptr; } + +inline DDPROF_NO_SANITIZER_ADDRESS void check_init() { + [[maybe_unused]] static bool init_once = []() { + init(); + return true; + }(); +} + +void init() { + s_calloc = &temp_calloc2; + + s_calloc = ORIGINAL_FUNC(calloc); + s_malloc = ORIGINAL_FUNC(malloc); + s_free = ORIGINAL_FUNC(free); + s_realloc = ORIGINAL_FUNC(realloc); + s_posix_memalign = ORIGINAL_FUNC(posix_memalign); + s_aligned_alloc = ORIGINAL_FUNC(aligned_alloc); + s_memalign = ORIGINAL_FUNC(memalign); + s_pvalloc = ORIGINAL_FUNC(pvalloc); + s_valloc = ORIGINAL_FUNC(valloc); + s_reallocarray = ORIGINAL_FUNC(reallocarray); +} + +} // namespace + +void *malloc(size_t size) { + void *ptr = s_malloc(size); + ddprof::AllocationTracker::track_allocation(reinterpret_cast(ptr), + size); + return ptr; +} + +void *temp_malloc(size_t size) noexcept { + check_init(); + return s_malloc(size); +} + +void free(void *ptr) { + if (ptr == nullptr) { + return; + } + ddprof::AllocationTracker::track_deallocation( + reinterpret_cast(ptr)); + s_free(ptr); +} + +void temp_free(void *ptr) noexcept { + check_init(); + return s_free(ptr); +} + +void *calloc(size_t nmemb, size_t size) { + void *ptr = s_calloc(nmemb, size); + ddprof::AllocationTracker::track_allocation(reinterpret_cast(ptr), + size * nmemb); + return ptr; +} + +void *temp_calloc(size_t nmemb, size_t size) noexcept { + check_init(); + return s_calloc(nmemb, size); +} + +void *realloc(void *ptr, size_t size) { + if (ptr) { + ddprof::AllocationTracker::track_deallocation( + reinterpret_cast(ptr)); + } + void *newptr = s_realloc(ptr, size); + ddprof::AllocationTracker::track_allocation( + reinterpret_cast(newptr), size); + return newptr; +} + +void *temp_realloc(void *ptr, size_t size) noexcept { + check_init(); + return s_realloc(ptr, size); +} + +int posix_memalign(void **memptr, size_t alignment, size_t size) { + int ret = s_posix_memalign(memptr, alignment, size); + if (likely(!ret)) { + ddprof::AllocationTracker::track_allocation( + reinterpret_cast(*memptr), size); + } + return ret; +} + +int temp_posix_memalign(void **memptr, size_t alignment, size_t size) noexcept { + check_init(); + return s_posix_memalign(memptr, alignment, size); +} + +void *aligned_alloc(size_t alignment, size_t size) { + void *ptr = s_aligned_alloc(alignment, size); + ddprof::AllocationTracker::track_allocation(reinterpret_cast(ptr), + size); + return ptr; +} + +void *temp_aligned_alloc(size_t alignment, size_t size) noexcept { + check_init(); + return s_aligned_alloc(alignment, size); +} + +void *memalign(size_t alignment, size_t size) { + void *ptr = s_memalign(alignment, size); + ddprof::AllocationTracker::track_allocation(reinterpret_cast(ptr), + size); + return ptr; +} +void *temp_memalign(size_t alignment, size_t size) noexcept { + check_init(); + return s_memalign(alignment, size); +} + +void *pvalloc(size_t size) noexcept { + void *ptr = s_pvalloc(size); + ddprof::AllocationTracker::track_allocation(reinterpret_cast(ptr), + size); + return ptr; +} + +void *temp_pvalloc(size_t size) noexcept { + check_init(); + return s_pvalloc(size); +} + +void *valloc(size_t size) { + void *ptr = s_valloc(size); + ddprof::AllocationTracker::track_allocation(reinterpret_cast(ptr), + size); + return ptr; +} + +void *temp_valloc(size_t size) noexcept { + check_init(); + return s_valloc(size); +} + +#ifdef __llvm__ +void *reallocarray(void *ptr, size_t nmemb, size_t size) noexcept { +#else +void *reallocarray(void *ptr, size_t nmemb, size_t size) { +#endif + if (ptr) { + ddprof::AllocationTracker::track_deallocation( + reinterpret_cast(ptr)); + } + void *newptr = s_reallocarray(ptr, nmemb, size); + ddprof::AllocationTracker::track_allocation( + reinterpret_cast(newptr), size * nmemb); + return newptr; +} + +void *temp_reallocarray(void *ptr, size_t nmemb, size_t size) noexcept { + check_init(); + return s_reallocarray(ptr, nmemb, size); +} diff --git a/src/pprof/ddprof_pprof.cc b/src/pprof/ddprof_pprof.cc index d2306f3eb..fbba23ce5 100644 --- a/src/pprof/ddprof_pprof.cc +++ b/src/pprof/ddprof_pprof.cc @@ -187,7 +187,7 @@ DDRes pprof_aggregate(const UnwindOutput *uw_output, // assumption of single line per loc for now ddog_prof_Line line_buff[DD_MAX_STACK_DEPTH]; - ddprof::span locs{uw_output->locs, uw_output->nb_locs}; + ddprof::span locs{uw_output->locs}; if (watcher->options.nb_frames_to_skip < locs.size()) { locs = locs.subspan(watcher->options.nb_frames_to_skip); @@ -275,7 +275,7 @@ void ddprof_print_sample(const UnwindOutput &uw_output, const PerfWatcher &watcher) { auto &symbol_table = symbol_hdr._symbol_table; - ddprof::span locs{uw_output.locs, uw_output.nb_locs}; + ddprof::span locs{uw_output.locs}; const char *sample_name = sample_type_name_from_idx( sample_type_id_to_count_sample_type_id(watcher.sample_type_id)); diff --git a/src/unwind.cc b/src/unwind.cc index 2fd058bca..d7c7e12b3 100644 --- a/src/unwind.cc +++ b/src/unwind.cc @@ -36,7 +36,7 @@ static void find_dso_add_error_frame(UnwindState *us) { void unwind_init_sample(UnwindState *us, uint64_t *sample_regs, pid_t sample_pid, uint64_t sample_size_stack, char *sample_data_stack) { - uw_output_clear(&us->output); + us->output.clear(); memcpy(&us->initial_regs.regs[0], sample_regs, K_NB_REGS_UNWIND * sizeof(uint64_t)); us->current_ip = us->initial_regs.regs[REGNAME(PC)]; @@ -56,11 +56,11 @@ static bool is_stack_complete(UnwindState *us) { static constexpr std::array s_expected_root_frames{"_start"sv, "__clone"sv, "_exit"sv}; - if (us->output.nb_locs == 0) { + if (us->output.locs.size() == 0) { return false; } - const auto &root_loc = us->output.locs[us->output.nb_locs - 1]; + const auto &root_loc = us->output.locs.back(); const auto &root_mapping = us->symbol_hdr._mapinfo_table[root_loc._map_info_idx]; @@ -96,7 +96,8 @@ DDRes unwindstate__unwind(UnwindState *us) { } else { us->output.is_incomplete = false; } - ddprof_stats_add(STATS_UNWIND_AVG_STACK_DEPTH, us->output.nb_locs, nullptr); + ddprof_stats_add(STATS_UNWIND_AVG_STACK_DEPTH, us->output.locs.size(), + nullptr); // Add a frame that identifies executable to which these belong add_virtual_base_frame(us); diff --git a/src/unwind_dwfl.cc b/src/unwind_dwfl.cc index 4a176194a..8e27283ae 100644 --- a/src/unwind_dwfl.cc +++ b/src/unwind_dwfl.cc @@ -102,7 +102,7 @@ static DDRes add_runtime_symbol_frame(UnwindState *us, const Dso &dso, static DDRes add_symbol(Dwfl_Frame *dwfl_frame, UnwindState *us) { if (is_max_stack_depth_reached(*us)) { add_common_frame(us, SymbolErrors::truncated_stack); - LG_DBG("Max stack depth reached (depth#%lu)", us->output.nb_locs); + LG_DBG("Max stack depth reached (depth#%lu)", us->output.locs.size()); ddprof_stats_add(STATS_UNWIND_TRUNCATED_OUTPUT, 1, nullptr); return ddres_warn(DD_WHAT_UW_MAX_DEPTH); } @@ -110,7 +110,7 @@ static DDRes add_symbol(Dwfl_Frame *dwfl_frame, UnwindState *us) { Dwarf_Addr pc = 0; if (!dwfl_frame_pc(dwfl_frame, &pc, nullptr)) { LG_DBG("Failure to compute frame PC: %s (depth#%lu)", dwfl_errmsg(-1), - us->output.nb_locs); + us->output.locs.size()); add_error_frame(nullptr, us, pc, SymbolErrors::dwfl_frame); return ddres_init(); // invalid pc : do not add frame } @@ -127,7 +127,7 @@ static DDRes add_symbol(Dwfl_Frame *dwfl_frame, UnwindState *us) { if (!find_res.second) { // no matching file was found LG_DBG("[UW] (PID%d) DSO not found at 0x%lx (depth#%lu)", us->pid, pc, - us->output.nb_locs); + us->output.locs.size()); add_error_frame(nullptr, us, pc, SymbolErrors::unknown_dso); return ddres_init(); } @@ -173,7 +173,7 @@ static DDRes add_symbol(Dwfl_Frame *dwfl_frame, UnwindState *us) { if (!dwfl_frame_pc(dwfl_frame, &pc, &isactivation)) { LG_DBG("Failure to compute frame PC: %s (depth#%lu)", dwfl_errmsg(-1), - us->output.nb_locs); + us->output.locs.size()); add_error_frame(nullptr, us, pc, SymbolErrors::dwfl_frame); return ddres_init(); // invalid pc : do not add frame } @@ -190,7 +190,7 @@ static DDRes add_symbol(Dwfl_Frame *dwfl_frame, UnwindState *us) { bool is_infinite_loop(UnwindState *us) { UnwindOutput &output = us->output; - uint64_t nb_locs = output.nb_locs; + uint64_t nb_locs = output.locs.size(); unsigned nb_frames_to_check = 3; if (nb_locs <= nb_frames_to_check) { return false; @@ -209,7 +209,7 @@ bool is_infinite_loop(UnwindState *us) { static int frame_cb(Dwfl_Frame *dwfl_frame, void *arg) { UnwindState *us = (UnwindState *)arg; #ifdef DEBUG - LG_NFO("Beging depth %lu", us->output.nb_locs); + LG_NFO("Beging depth %lu", us->output.locs.size()); #endif int dwfl_error_value = dwfl_errno(); if (dwfl_error_value) { @@ -222,7 +222,7 @@ static int frame_cb(Dwfl_Frame *dwfl_frame, void *arg) { #ifdef DEBUG // We often fallback to frame pointer unwinding (which logs an error) if (dwfl_error_value) { - LG_DBG("Error flagged at depth = %lu -- %d Error:%s ", us->output.nb_locs, + LG_DBG("Error flagged at depth = %lu -- %d Error:%s ", us->output.locs.size(), dwfl_error_value, dwfl_errmsg(dwfl_error_value)); } #endif @@ -248,8 +248,8 @@ DDRes unwind_dwfl(UnwindState *us) { 0) { trace_unwinding_end(us); } - res = us->output.nb_locs > 0 ? ddres_init() - : ddres_warn(DD_WHAT_DWFL_LIB_ERROR); + res = us->output.locs.size() > 0 ? ddres_init() + : ddres_warn(DD_WHAT_DWFL_LIB_ERROR); return res; } diff --git a/src/unwind_helpers.cc b/src/unwind_helpers.cc index 1f90679b3..270b6ad03 100644 --- a/src/unwind_helpers.cc +++ b/src/unwind_helpers.cc @@ -15,35 +15,33 @@ namespace ddprof { bool is_max_stack_depth_reached(const UnwindState &us) { // +2 to keep room for common base frame - return us.output.nb_locs + 2 >= DD_MAX_STACK_DEPTH; + return us.output.locs.size() + 2 >= DD_MAX_STACK_DEPTH; } DDRes add_frame(SymbolIdx_t symbol_idx, MapInfoIdx_t map_idx, ElfAddress_t pc, UnwindState *us) { UnwindOutput *output = &us->output; - int64_t current_loc_idx = output->nb_locs; - if (output->nb_locs >= DD_MAX_STACK_DEPTH) { + if (output->locs.size() >= DD_MAX_STACK_DEPTH) { DDRES_RETURN_WARN_LOG(DD_WHAT_UW_MAX_DEPTH, "Max stack depth reached"); // avoid overflow } - - output->locs[current_loc_idx]._symbol_idx = symbol_idx; - output->locs[current_loc_idx].ip = pc; + FunLoc current; + current._symbol_idx = symbol_idx; + current.ip = pc; if (map_idx == -1) { // just add an empty element for mapping info - output->locs[current_loc_idx]._map_info_idx = - us->symbol_hdr._common_mapinfo_lookup.get_or_insert( - CommonMapInfoLookup::MappingErrors::empty, - us->symbol_hdr._mapinfo_table); + current._map_info_idx = us->symbol_hdr._common_mapinfo_lookup.get_or_insert( + CommonMapInfoLookup::MappingErrors::empty, + us->symbol_hdr._mapinfo_table); } else { - output->locs[current_loc_idx]._map_info_idx = map_idx; + current._map_info_idx = map_idx; } #ifdef DEBUG LG_NTC("Considering frame with IP : %lx / %s ", pc, - us->symbol_hdr._symbol_table[output->locs[current_loc_idx]._symbol_idx] - ._symname.c_str()); + us->symbol_hdr._symbol_table[current._symbol_idx]._symname.c_str()); #endif - output->nb_locs++; + output->locs.push_back(current); + return ddres_init(); } @@ -178,7 +176,7 @@ bool memory_read(ProcessAddress_t addr, ElfWord_t *result, int regno, // requested when unwinding the leaf function for a register, we simply // return the initial register value. constexpr uint64_t k_red_zone_size = 128; - if (us->output.nb_locs <= 1 && regno != -1 && + if (us->output.locs.size() <= 1 && regno != -1 && addr >= sp_start - k_red_zone_size) { *result = us->initial_regs.regs[regno]; return true; @@ -232,6 +230,6 @@ void add_error_frame(const Dso *dso, UnwindState *us, } else { add_common_frame(us, error_case); } - LG_DBG("Error frame (depth#%lu)", us->output.nb_locs); + LG_DBG("Error frame (depth#%lu)", us->output.locs.size()); } } // namespace ddprof diff --git a/src/unwind_output.cc b/src/unwind_output.cc deleted file mode 100644 index 34a927dbb..000000000 --- a/src/unwind_output.cc +++ /dev/null @@ -1,19 +0,0 @@ -// Unless explicitly stated otherwise all files in this repository are licensed -// under the Apache License Version 2.0. This product includes software -// developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present -// Datadog, Inc. - -#include -#include - -#include "unwind_output.hpp" - -static void FunLoc_clear(FunLoc *locs) { - memset(locs, 0, sizeof(*locs) * DD_MAX_STACK_DEPTH); -} - -void uw_output_clear(UnwindOutput *output) { - FunLoc_clear(output->locs); - output->nb_locs = 0; - output->is_incomplete = true; -} diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 200091049..8e1acfac2 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -151,7 +151,7 @@ add_unit_test( DEFINITIONS MYNAME="pevent-ut") add_unit_test( - ddprof_pprof-ut ../src/pprof/ddprof_pprof.cc ../src/unwind_output.cc ../src/perf_watcher.cc + ddprof_pprof-ut ../src/pprof/ddprof_pprof.cc ../src/perf_watcher.cc ddprof_pprof-ut.cc LIBRARIES Datadog::Profiling DDProf::Parser DEFINITIONS MYNAME="ddprof_pprof-ut") @@ -161,7 +161,6 @@ add_unit_test( ../src/exporter/ddprof_exporter.cc ../src/ddprof_cmdline.cc ../src/pprof/ddprof_pprof.cc - ../src/unwind_output.cc ../src/perf_watcher.cc ../src/tags.cc ddprof_exporter-ut.cc @@ -240,7 +239,6 @@ add_unit_test( ../src/unwind_dwfl.cc ../src/unwind_helpers.cc ../src/unwind_metrics.cc - ../src/unwind_output.cc ../src/user_override.cc LIBRARIES ${ELFUTILS_LIBRARIES} llvm-demangle DEFINITIONS MYNAME="savecontext-ut") @@ -285,7 +283,6 @@ add_unit_test( ../src/unwind_dwfl.cc ../src/unwind_helpers.cc ../src/unwind_metrics.cc - ../src/unwind_output.cc LIBRARIES ${ELFUTILS_LIBRARIES} llvm-demangle DEFINITIONS ${DDPROF_DEFINITION_LIST}) diff --git a/test/allocation_tracker-ut.cc b/test/allocation_tracker-ut.cc index e8307486f..a9ef6bd28 100644 --- a/test/allocation_tracker-ut.cc +++ b/test/allocation_tracker-ut.cc @@ -5,7 +5,9 @@ #include "allocation_tracker.hpp" #include "ddprof_base.hpp" +#include "ddprof_perf_event.hpp" #include "ipc.hpp" +#include "loghandle.hpp" #include "perf_watcher.hpp" #include "pevent_lib.hpp" #include "ringbuffer_holder.hpp" @@ -24,6 +26,12 @@ DDPROF_NOINLINE void my_malloc(size_t size) { getpid(); } +DDPROF_NOINLINE void my_free(uintptr_t addr) { + ddprof::AllocationTracker::track_deallocation(addr); + // prevent tail call optimization + getpid(); +} + extern "C" { DDPROF_NOINLINE void my_func_calling_malloc(size_t size) { my_malloc(size); @@ -38,43 +46,68 @@ TEST(allocation_tracker, start_stop) { ddprof::RingBufferHolder ring_buffer{buf_size_order, RingBufferType::kMPSCRingBuffer}; ddprof::AllocationTracker::allocation_tracking_init( - rate, ddprof::AllocationTracker::kDeterministicSampling, + rate, + ddprof::AllocationTracker::kDeterministicSampling | + ddprof::AllocationTracker::kTrackDeallocations, ring_buffer.get_buffer_info()); ASSERT_TRUE(ddprof::AllocationTracker::is_active()); my_func_calling_malloc(1); - - ddprof::MPSCRingBufferReader reader{ring_buffer.get_ring_buffer()}; - ASSERT_GT(reader.available_size(), 0); - - auto buf = reader.read_sample(); - ASSERT_FALSE(buf.empty()); - const perf_event_header *hdr = - reinterpret_cast(buf.data()); - ASSERT_EQ(hdr->type, PERF_RECORD_SAMPLE); - - perf_event_sample *sample = hdr2samp(hdr, perf_event_default_sample_type()); - - ASSERT_EQ(sample->period, 1); - ASSERT_EQ(sample->pid, getpid()); - ASSERT_EQ(sample->tid, ddprof::gettid()); - - UnwindState state; - ddprof::unwind_init_sample(&state, sample->regs, sample->pid, - sample->size_stack, sample->data_stack); - ddprof::unwindstate__unwind(&state); - - const auto &symbol_table = state.symbol_hdr._symbol_table; - ASSERT_GT(state.output.nb_locs, NB_FRAMES_TO_SKIP); - const auto &symbol = - symbol_table[state.output.locs[NB_FRAMES_TO_SKIP]._symbol_idx]; - ASSERT_EQ(symbol._symname, "my_func_calling_malloc"); - + { // check that we get the relevant info for this allocation + ddprof::MPSCRingBufferReader reader{ring_buffer.get_ring_buffer()}; + ASSERT_GT(reader.available_size(), 0); + + auto buf = reader.read_sample(); + ASSERT_FALSE(buf.empty()); + const perf_event_header *hdr = + reinterpret_cast(buf.data()); + ASSERT_EQ(hdr->type, PERF_RECORD_SAMPLE); + + perf_event_sample *sample = + hdr2samp(hdr, perf_event_default_sample_type() | PERF_SAMPLE_ADDR); + + ASSERT_EQ(sample->period, 1); + ASSERT_EQ(sample->pid, getpid()); + ASSERT_EQ(sample->tid, ddprof::gettid()); + ASSERT_EQ(sample->addr, 0xdeadbeef); + + UnwindState state; + ddprof::unwind_init_sample(&state, sample->regs, sample->pid, + sample->size_stack, sample->data_stack); + ddprof::unwindstate__unwind(&state); + + const auto &symbol_table = state.symbol_hdr._symbol_table; + ASSERT_GT(state.output.locs.size(), NB_FRAMES_TO_SKIP); + const auto &symbol = + symbol_table[state.output.locs[NB_FRAMES_TO_SKIP]._symbol_idx]; + ASSERT_EQ(symbol._symname, "my_func_calling_malloc"); + } + my_free(0xdeadbeef); + // ensure we get a deallocation event + { + ddprof::MPSCRingBufferReader reader{ring_buffer.get_ring_buffer()}; + ASSERT_GT(reader.available_size(), 0); + + auto buf = reader.read_sample(); + ASSERT_FALSE(buf.empty()); + const perf_event_header *hdr = + reinterpret_cast(buf.data()); + ASSERT_EQ(hdr->type, PERF_CUSTOM_EVENT_DEALLOCATION); + const ddprof::DeallocationEvent *sample = + reinterpret_cast(hdr); + ASSERT_EQ(sample->ptr, 0xdeadbeef); + } + my_free(0xcafebabe); + // { + // ddprof::MPSCRingBufferReader reader{ring_buffer.get_ring_buffer()}; + // ASSERT_EQ(reader.available_size(), 0); + // } ddprof::AllocationTracker::allocation_tracking_free(); ASSERT_FALSE(ddprof::AllocationTracker::is_active()); } TEST(allocation_tracker, stale_lock) { + LogHandle log_handle; const uint64_t rate = 1; const size_t buf_size_order = 5; ddprof::RingBufferHolder ring_buffer{buf_size_order, @@ -92,4 +125,4 @@ TEST(allocation_tracker, stale_lock) { } ASSERT_FALSE(ddprof::AllocationTracker::is_active()); ddprof::AllocationTracker::allocation_tracking_free(); -} \ No newline at end of file +} diff --git a/test/savecontext-ut.cc b/test/savecontext-ut.cc index 48cc7ae21..f44ff5530 100644 --- a/test/savecontext-ut.cc +++ b/test/savecontext-ut.cc @@ -35,12 +35,12 @@ void funcB() { auto &symbol_table = state.symbol_hdr._symbol_table; - for (size_t iloc = 0; iloc < state.output.nb_locs; ++iloc) { + for (size_t iloc = 0; iloc < state.output.locs.size(); ++iloc) { auto &symbol = symbol_table[state.output.locs[iloc]._symbol_idx]; printf("%zu: %s\n", iloc, symbol._demangle_name.c_str()); } - EXPECT_GT(state.output.nb_locs, 3); + EXPECT_GT(state.output.locs.size(), 3); auto &symbol0 = symbol_table[state.output.locs[0]._symbol_idx]; EXPECT_TRUE(symbol0._demangle_name.starts_with("save_context(")); auto &symbol1 = symbol_table[state.output.locs[1]._symbol_idx]; @@ -104,7 +104,7 @@ TEST(getcontext, unwind_from_sighandler) { auto &symbol_table = state.symbol_hdr._symbol_table; - for (size_t iloc = 0; iloc < state.output.nb_locs; ++iloc) { + for (size_t iloc = 0; iloc < state.output.locs.size(); ++iloc) { auto &symbol = symbol_table[state.output.locs[iloc]._symbol_idx]; printf("%zu: %s %lx \n", iloc, symbol._demangle_name.c_str(), state.output.locs[iloc].ip); @@ -113,12 +113,12 @@ TEST(getcontext, unwind_from_sighandler) { return symbol_table[state.output.locs[idx]._symbol_idx]; }; - EXPECT_GT(state.output.nb_locs, 5); - EXPECT_LT(state.output.nb_locs, 20); + EXPECT_GT(state.output.locs.size(), 5); + EXPECT_LT(state.output.locs.size(), 20); EXPECT_TRUE(get_symbol(0)._demangle_name.starts_with("save_context(")); EXPECT_EQ(get_symbol(1)._demangle_name, "handler(int)"); size_t next_idx = 3; - while (next_idx < state.output.nb_locs - 1 && + while (next_idx < state.output.locs.size() - 1 && get_symbol(next_idx)._demangle_name != "funcD()") { ++next_idx; } diff --git a/test/self_unwind/self_unwind.cc b/test/self_unwind/self_unwind.cc index 575cfd62b..9c990915d 100644 --- a/test/self_unwind/self_unwind.cc +++ b/test/self_unwind/self_unwind.cc @@ -118,7 +118,7 @@ bool stack_addtomap(const UnwindOutput *unwind_output, const DDProfContext *ctx, assert(callback_ctx); suw::SymbolMap *symbol_map = reinterpret_cast(callback_ctx); assert(perf_option_pos == 0); - for (unsigned i = 0; i < unwind_output->nb_locs; ++i) { + for (unsigned i = 0; i < unwind_output->locs.size(); ++i) { const ddprof::Symbol &symbol = ddprof::get_symbol(ctx, unwind_output, i); if (symbol._demangle_name.find("0x") != std::string::npos) { // skip non symbolized frames diff --git a/test/simple_malloc-ut.sh b/test/simple_malloc-ut.sh index 2919f1640..bc27895ed 100755 --- a/test/simple_malloc-ut.sh +++ b/test/simple_malloc-ut.sh @@ -96,6 +96,9 @@ check "./ddprof ./test/simple_malloc ${opts}" 1 # Test wrapper mode with forks + threads check "./ddprof ./test/simple_malloc ${opts} --fork 2 --threads 2" 2 4 +# Test wrapper mode with forks + threads +# check "./ddprof --live_allocations yes ./test/simple_malloc ${opts} --fork 2 --threads 2 --skip-free 100" 2 4 + # Test slow profiler startup check "env DD_PROFILING_NATIVE_STARTUP_WAIT_MS=200 ./ddprof ./test/simple_malloc ${opts}" 1 diff --git a/test/simple_malloc.cc b/test/simple_malloc.cc index 4239180e6..65723bc9a 100644 --- a/test/simple_malloc.cc +++ b/test/simple_malloc.cc @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -24,6 +25,28 @@ # include "dd_profiling.h" #endif +#ifdef __GLIBC__ +# include +#endif + +/***************************** SIGSEGV Handler *******************************/ +static void sigsegv_handler(int sig, siginfo_t *si, void *uc) { + // TODO this really shouldn't call printf-family functions... + (void)uc; +#ifdef __GLIBC__ + static void *buf[4096] = {0}; + size_t sz = backtrace(buf, 4096); +#endif + fprintf(stderr, "simplemalloc[%d]:has encountered an error and will exit\n", + getpid()); + if (sig == SIGSEGV) + printf("Fault address: %p\n", si->si_addr); +#ifdef __GLIBC__ + backtrace_symbols_fd(buf, sz, STDERR_FILENO); +#endif + exit(-1); +} + struct thread_cpu_clock { using duration = std::chrono::nanoseconds; using rep = duration::rep; @@ -57,6 +80,7 @@ struct Options { std::chrono::milliseconds timeout_duration; uint32_t callstack_depth; uint32_t frame_size; + uint32_t skip_free; }; extern "C" DDPROF_NOINLINE void do_lot_of_allocations(const Options &options, @@ -67,6 +91,7 @@ extern "C" DDPROF_NOINLINE void do_lot_of_allocations(const Options &options, auto start_time = std::chrono::steady_clock::now(); auto deadline_time = start_time + options.timeout_duration; auto start_cpu = thread_cpu_clock::now(); + unsigned skip_free = 0; for (uint64_t i = 0; i < options.loop_count; ++i) { void *p = nullptr; if (options.malloc_size) { @@ -84,7 +109,12 @@ extern "C" DDPROF_NOINLINE void do_lot_of_allocations(const Options &options, p2 = p; } ddprof::DoNotOptimize(p2); - free(p2); + + if (skip_free++ >= options.skip_free) { + free(p2); + skip_free = 0; + } + if (options.sleep_duration_per_loop.count()) { std::this_thread::sleep_for(options.sleep_duration_per_loop); } @@ -142,6 +172,11 @@ void print_stats(pid_t pid, const Stats &stats) { } int main(int argc, char *argv[]) { + struct sigaction sigaction_handlers = {}; + sigaction_handlers.sa_sigaction = sigsegv_handler; + sigaction_handlers.sa_flags = SA_SIGINFO; + sigaction(SIGSEGV, &(sigaction_handlers), NULL); + try { CLI::App app{"Simple allocation test"}; @@ -170,6 +205,9 @@ int main(int argc, char *argv[]) { app.add_option("--frame-size", opts.frame_size, "Size to allocate on the stack for each frame") ->default_val(0); + app.add_option("--skip-free", opts.skip_free, + "Only free every N allocations (default is 0)") + ->default_val(0); app.add_option( "--timeout", opts.timeout_duration, "Timeout after N milliseconds") diff --git a/test/unwind_output_mock.hpp b/test/unwind_output_mock.hpp index af0c072c1..69742f504 100644 --- a/test/unwind_output_mock.hpp +++ b/test/unwind_output_mock.hpp @@ -43,11 +43,11 @@ static inline void fill_mapinfo_table_1(MapInfoTable &mapinfo_table) { } static inline void fill_unwind_output_1(UnwindOutput &uw_output) { - uw_output_clear(&uw_output); - uw_output.nb_locs = K_MOCK_LOC_SIZE; + uw_output.clear(); + uw_output.locs.resize(K_MOCK_LOC_SIZE); - FunLoc *locs = uw_output.locs; - for (unsigned i = 0; i < uw_output.nb_locs; ++i) { + std::vector &locs = uw_output.locs; + for (unsigned i = 0; i < uw_output.locs.size(); ++i) { locs[i].ip = 42 + i; locs[i]._symbol_idx = i; locs[i]._map_info_idx = i; From 31a05cee5f6ac891d53e1a7ff4ffe2e46e0c1bb1 Mon Sep 17 00:00:00 2001 From: Nicolas Savoire Date: Tue, 31 Jan 2023 16:07:35 +0100 Subject: [PATCH 02/21] Add mmap hooks --- src/lib/symbol_overrides.cc | 85 +++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/src/lib/symbol_overrides.cc b/src/lib/symbol_overrides.cc index c30d3b478..86b6e5f04 100644 --- a/src/lib/symbol_overrides.cc +++ b/src/lib/symbol_overrides.cc @@ -14,6 +14,7 @@ #include #include #include +#include #include #if defined(__GNUC__) && !defined(__clang__) @@ -43,6 +44,27 @@ timer_t g_timerid; int g_timer_sig = -1; int g_nb_loaded_libraries = -1; +// \fixme{nsavoire} The goal of this flag is to avoid double-counting +// mmaps that done inside malloc, not sure if it desirable or not. +// We should probably merge this TL state with AllocationTracker::tl_state +// to have a single TL state since accessing it is costly (call to tls_get_addr) +thread_local bool g_in_allocator_guard = false; + +class Guard { +public: + explicit Guard(bool *guard) : _guard(guard), _ok(!*guard) { *_guard = true; } + ~Guard() { + if (_ok) { + *_guard = false; + } + } + explicit operator bool() const { return _ok; } + +private: + bool *_guard; + bool _ok; +}; + DDPROF_NOINLINE bool loaded_libraries_have_changed() { int nb = ddprof::count_loaded_libraries(); if (nb != g_nb_loaded_libraries) { @@ -70,6 +92,7 @@ struct malloc { static void *hook(size_t size) noexcept { check_libraries(); + Guard guard(&g_in_allocator_guard); auto ptr = ref(size); ddprof::AllocationTracker::track_allocation( reinterpret_cast(ptr), size); @@ -101,6 +124,7 @@ struct calloc { static void *hook(size_t nmemb, size_t size) noexcept { check_libraries(); + Guard guard(&g_in_allocator_guard); auto ptr = ref(nmemb, size); ddprof::AllocationTracker::track_allocation( reinterpret_cast(ptr), size * nmemb); @@ -116,6 +140,7 @@ struct realloc { static void *hook(void *ptr, size_t size) noexcept { check_libraries(); + Guard guard(&g_in_allocator_guard); if (likely(ptr)) { ddprof::AllocationTracker::track_deallocation( reinterpret_cast(ptr)); @@ -138,6 +163,7 @@ struct posix_memalign { static int hook(void **memptr, size_t alignment, size_t size) noexcept { check_libraries(); + Guard guard(&g_in_allocator_guard); auto ret = ref(memptr, alignment, size); if (likely(!ret)) { ddprof::AllocationTracker::track_allocation( @@ -154,6 +180,7 @@ struct aligned_alloc { static void *hook(size_t alignment, size_t size) noexcept { check_libraries(); + Guard guard(&g_in_allocator_guard); auto ptr = ref(alignment, size); ddprof::AllocationTracker::track_allocation( reinterpret_cast(ptr), size); @@ -169,6 +196,7 @@ struct memalign { static void *hook(size_t alignment, size_t size) noexcept { check_libraries(); + Guard guard(&g_in_allocator_guard); auto ptr = ref(alignment, size); ddprof::AllocationTracker::track_allocation( reinterpret_cast(ptr), size); @@ -184,6 +212,7 @@ struct pvalloc { static void *hook(size_t size) noexcept { check_libraries(); + Guard guard(&g_in_allocator_guard); auto ptr = ref(size); ddprof::AllocationTracker::track_allocation( reinterpret_cast(ptr), size); @@ -199,6 +228,7 @@ struct valloc { static void *hook(size_t size) noexcept { check_libraries(); + Guard guard(&g_in_allocator_guard); auto ptr = ref(size); ddprof::AllocationTracker::track_allocation( reinterpret_cast(ptr), size); @@ -214,6 +244,7 @@ struct reallocarray { static void *hook(void *ptr, size_t nmemb, size_t size) noexcept { check_libraries(); + Guard guard(&g_in_allocator_guard); if (ptr) { ddprof::AllocationTracker::track_deallocation( reinterpret_cast(ptr)); @@ -277,6 +308,56 @@ struct pthread_create { } }; +struct mmap { + static constexpr auto name = "mmap"; + static inline auto ref = &::mmap; + static inline bool ref_checked = false; + + static void *hook(void *addr, size_t length, int prot, int flags, int fd, + off_t offset) noexcept { + void *ptr = ref(addr, length, prot, flags, fd, offset); + + if (addr == nullptr && fd == -1 && ptr != nullptr && + !g_in_allocator_guard) { + ddprof::AllocationTracker::track_allocation( + reinterpret_cast(ptr), length); + } + return ptr; + } +}; + +struct mmap64_ { + static constexpr auto name = "mmap64"; + static inline auto ref = &::mmap64; + static inline bool ref_checked = false; + + static void *hook(void *addr, size_t length, int prot, int flags, int fd, + off_t offset) noexcept { + void *ptr = ref(addr, length, prot, flags, fd, offset); + + if (addr == nullptr && fd == -1 && ptr != nullptr && + !g_in_allocator_guard) { + ddprof::AllocationTracker::track_allocation( + reinterpret_cast(ptr), length); + } + return ptr; + } +}; + +struct munmap { + static constexpr auto name = "munmap"; + static inline auto ref = &::munmap; + static inline bool ref_checked = false; + + static int hook(void *addr, size_t length) noexcept { + if (!g_in_allocator_guard) { + ddprof::AllocationTracker::track_deallocation( + reinterpret_cast(addr)); + } + return ref(addr, length); + } +}; + template void install_hook(bool restore) { // On ubuntu 16, some symbols might be bound to @plt symbols // in exe and since we override the symbols in the exe, this would cause @@ -375,6 +456,10 @@ void setup_hooks(bool restore) { install_hook(restore); install_hook(restore); + install_hook(restore); + install_hook(restore); + install_hook(restore); + if (reallocarray::ref) { install_hook(restore); } From edd1f7e338d3e64c57a728a358d380b24c25122d Mon Sep 17 00:00:00 2001 From: Nicolas Savoire Date: Thu, 6 Oct 2022 13:14:30 +0000 Subject: [PATCH 03/21] Add more m(un)map overrides --- src/lib/symbol_overrides.cc | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/lib/symbol_overrides.cc b/src/lib/symbol_overrides.cc index 86b6e5f04..991a6ac92 100644 --- a/src/lib/symbol_overrides.cc +++ b/src/lib/symbol_overrides.cc @@ -326,6 +326,24 @@ struct mmap { } }; +struct mmap_ { + static constexpr auto name = "__mmap"; + static inline auto ref = &::mmap; + static inline bool ref_checked = false; + + static void *hook(void *addr, size_t length, int prot, int flags, int fd, + off_t offset) noexcept { + void *ptr = ref(addr, length, prot, flags, fd, offset); + + if (addr == nullptr && fd == -1 && ptr != nullptr && + !g_in_allocator_guard) { + ddprof::AllocationTracker::track_allocation( + reinterpret_cast(ptr), length); + } + return ptr; + } +}; + struct mmap64_ { static constexpr auto name = "mmap64"; static inline auto ref = &::mmap64; @@ -358,6 +376,20 @@ struct munmap { } }; +struct munmap_ { + static constexpr auto name = "__munmap"; + static inline auto ref = &::munmap; + static inline bool ref_checked = false; + + static int hook(void *addr, size_t length) noexcept { + if (!g_in_allocator_guard) { + ddprof::AllocationTracker::track_deallocation( + reinterpret_cast(addr)); + } + return ref(addr, length); + } +}; + template void install_hook(bool restore) { // On ubuntu 16, some symbols might be bound to @plt symbols // in exe and since we override the symbols in the exe, this would cause @@ -459,6 +491,8 @@ void setup_hooks(bool restore) { install_hook(restore); install_hook(restore); install_hook(restore); + install_hook(restore); + install_hook(restore); if (reallocarray::ref) { install_hook(restore); From ceeb757c078be861a7cc9394ff84a36e151ead31 Mon Sep 17 00:00:00 2001 From: Nicolas Savoire Date: Tue, 31 Jan 2023 15:56:59 +0100 Subject: [PATCH 04/21] Add system allocation profiling (#192) Adds * system for combining multiple watchers into one event * watcher overloads * tallocsys --- include/ddprof_cmdline.hpp | 2 + include/ddprof_worker_context.hpp | 2 + include/live_sysallocations.hpp | 114 +++++++++++++++++++++++++++++ include/perf.hpp | 3 + include/perf_watcher.hpp | 56 ++++++++------ include/pevent.hpp | 2 + src/ddprof_cmdline.cc | 92 +++++++++++++++++++++++ src/ddprof_context_lib.cc | 11 +++ src/ddprof_worker.cc | 118 ++++++++++++++++++++++++++++-- src/perf_watcher.cc | 1 + src/pevent_lib.cc | 111 +++++++++++++++++++++++++++- test/CMakeLists.txt | 10 ++- 12 files changed, 488 insertions(+), 34 deletions(-) create mode 100644 include/live_sysallocations.hpp diff --git a/include/ddprof_cmdline.hpp b/include/ddprof_cmdline.hpp index aad07b003..8dc6b6e4c 100644 --- a/include/ddprof_cmdline.hpp +++ b/include/ddprof_cmdline.hpp @@ -27,4 +27,6 @@ bool arg_inset(const char *str, char const *const *set, int sz_set); bool arg_yesno(const char *str, int mode); +long id_from_tracepoint(const char *gname, const char *tname); + bool watcher_from_str(const char *str, PerfWatcher *watcher); diff --git a/include/ddprof_worker_context.hpp b/include/ddprof_worker_context.hpp index f7c969ace..344386c5a 100644 --- a/include/ddprof_worker_context.hpp +++ b/include/ddprof_worker_context.hpp @@ -6,6 +6,7 @@ #pragma once #include "live_allocation.hpp" +#include "live_sysallocations.hpp" #include "pevent.hpp" #include "proc_status.hpp" @@ -38,4 +39,5 @@ struct DDProfWorkerContext { uint32_t count_worker; // exports since last cache clear std::array lost_events_per_watcher; ddprof::LiveAllocation live_allocation; + ddprof::SystemAllocation sys_allocation; }; diff --git a/include/live_sysallocations.hpp b/include/live_sysallocations.hpp new file mode 100644 index 000000000..52afddcb9 --- /dev/null +++ b/include/live_sysallocations.hpp @@ -0,0 +1,114 @@ +#pragma once + +#include "ddprof_defs.hpp" +#include "logger.hpp" +#include "unwind_output.hpp" + +#include +#include + +#include + +namespace ddprof { + +class SystemAllocation { +private: + template T to_page(T a) { + return ((a + T{4095ull}) & (~T{4095ull})) >> T{12ull}; + } + +public: + void add_allocs(const UnwindOutput &stack, uintptr_t addr, size_t size, + pid_t pid) { + StackMap &stack_map = _pid_map[pid]; + + // Convert addr to page idx, then page-align size and decimate + uintptr_t page_start = to_page(addr); + uintptr_t page_end = to_page(addr + size); + + for (auto i = page_start; i <= page_end; ++i) { + stack_map[i] = stack; + } + _visited_recently.insert(pid); + } + + void move_allocs(uintptr_t addr0, uintptr_t addr1, size_t size, pid_t pid) { + StackMap &stack_map = _pid_map[pid]; + + // Convert addr to page idx + uintptr_t page_start_0 = to_page(addr0); + uintptr_t page_end_0 = to_page(addr0 + size); + uintptr_t page_start_1 = to_page(addr1); + uintptr_t page_idx_max = page_end_0 - page_start_0; + + // Can ranges overlap? Better not try to delete them all at end... + for (uintptr_t i = 0; i < page_idx_max; ++i) { + stack_map[page_start_1 + i] = stack_map[page_start_0 + i]; + stack_map.erase(page_start_0 + i); + } + _visited_recently.insert(pid); + } + + void del_allocs(uintptr_t addr, size_t size, pid_t pid) { + StackMap &stack_map = _pid_map[pid]; + + // Convert addr to page idx, then page-align size and decimate + uintptr_t page_start = to_page(addr); + uintptr_t page_end = to_page(addr + size); + + for (auto i = page_start; i <= page_end; ++i) { + stack_map.erase(i); + } + _visited_recently.insert(pid); + } + + void do_mmap(const UnwindOutput &stack, uintptr_t addr, size_t size, + pid_t pid) { + add_allocs(stack, addr, size, pid); + } + + void do_munmap(uintptr_t addr, size_t size, pid_t pid) { + del_allocs(addr, size, pid); + } + + void do_madvise(uintptr_t addr, size_t size, int flags, pid_t pid) { + // No reason to worry about this yet, since it only has to do with RSS + } + + void do_mremap(const UnwindOutput &stack, uintptr_t addr0, uintptr_t addr1, + size_t size0, size_t size1, pid_t pid) { + // We could either classify these pages as belonging to the original mmap + // or to the mremap. We chose the latter for now. + // Note that we potentially duplicate a lot of work here in the case + // that addr0 == addr1 + del_allocs(addr0, size0, pid); + add_allocs(stack, addr1, size1, pid); + } + + void do_exit(pid_t pid) { + StackMap &stack_map = _pid_map[pid]; + stack_map.clear(); + _visited_recently.erase(pid); + } + + void sanitize_pids() { + for (auto &stack_map : _pid_map) { + if (!_visited_recently.contains(stack_map.first)) { + // This PID wasn't visited recently. Is it still around? + if (kill(stack_map.first, 0)) { + _pid_map[stack_map.first].clear(); + } + } + } + _visited_recently.clear(); + } + + using StackMap = std::unordered_map; + using PidMap = std::unordered_map; + + PidMap _pid_map; + std::unordered_set _visited_recently; + int watcher_pos; +}; + +} // namespace ddprof diff --git a/include/perf.hpp b/include/perf.hpp index f1910beb4..d18fb98b4 100644 --- a/include/perf.hpp +++ b/include/perf.hpp @@ -150,3 +150,6 @@ all_perf_configs_from_watcher(const PerfWatcher *watcher, bool extras); uint64_t perf_value_from_sample(const PerfWatcher *watcher, const perf_event_sample *sample); } // namespace ddprof + +perf_event_attr perf_config_from_watcher(const PerfWatcher *watcher, + bool extras); diff --git a/include/perf_watcher.hpp b/include/perf_watcher.hpp index 7fd0a7d38..cbfe111b1 100644 --- a/include/perf_watcher.hpp +++ b/include/perf_watcher.hpp @@ -24,6 +24,9 @@ struct PerfWatcherOptions { uint8_t nb_frames_to_skip; // number of bottom frames to skip in stack trace // (useful for allocation profiling to remove // frames belonging to lib_ddprofiling.so) + bool is_overloaded; // Isn't actually needed, but makes it clear from this + // file that additional state is injected into the + // watcher in ddprof_cmdline.cc }; struct PerfWatcher { @@ -56,6 +59,7 @@ struct PerfWatcher { bool suppress_tid; int pprof_sample_idx; // index into the SampleType in the pprof int pprof_count_sample_idx; // index into the pprof for the count + bool instrument_self; // do my own perfopen, etc EventConfMode output_mode; // defines how sample data is aggregated }; @@ -80,7 +84,10 @@ typedef enum DDPROF_SAMPLE_TYPES { // Define our own event type on top of perf event types enum DDProfTypeId { kDDPROF_TYPE_CUSTOM = PERF_TYPE_MAX + 100 }; -enum DDProfCustomCountId { kDDPROF_COUNT_ALLOCATIONS = 0 }; +enum DDProfCustomCountId { + kDDPROF_COUNT_ALLOCATIONS = 0, + kDDPROF_COUNT_SYSALLOCATIONS, +}; // Kernel events are necessary to get a full accounting of CPU // This depend on the state of configuration (capabilities / @@ -103,35 +110,40 @@ enum DDProfCustomCountId { kDDPROF_COUNT_ALLOCATIONS = 0 }; #define SKIP_FRAMES \ { .nb_frames_to_skip = NB_FRAMES_TO_SKIP } +#define IS_OVERLOADED \ + { .is_overloaded = true } + // Whereas tracepoints are dynamically configured and can be checked at runtime, // we lack the ability to inspect events of type other than TYPE_TRACEPOINT. // Accordingly, we maintain a list of events, even though the type of these // events are marked as tracepoint unless they represent a well-known profiling // type! // clang-format off -// short desc perf event type perf event count type period/freq profile sample type addtl. configs +// short desc perf event type perf event count type period/freq profile sample type addtl. configs // cppcheck-suppress preprocessorErrorDirective #define EVENT_CONFIG_TABLE(X) \ - X(hCPU, "CPU Cycles", PERF_TYPE_HARDWARE, PERF_COUNT_HW_CPU_CYCLES, 99, DDPROF_PWT_TRACEPOINT, IS_FREQ) \ - X(hREF, "Ref. CPU Cycles", PERF_TYPE_HARDWARE, PERF_COUNT_HW_REF_CPU_CYCLES, 1000, DDPROF_PWT_TRACEPOINT, IS_FREQ) \ - X(hINST, "Instr. Count", PERF_TYPE_HARDWARE, PERF_COUNT_HW_INSTRUCTIONS, 1000, DDPROF_PWT_TRACEPOINT, IS_FREQ) \ - X(hCREF, "Cache Ref.", PERF_TYPE_HARDWARE, PERF_COUNT_HW_CACHE_REFERENCES, 999, DDPROF_PWT_TRACEPOINT, {}) \ - X(hCMISS, "Cache Miss", PERF_TYPE_HARDWARE, PERF_COUNT_HW_CACHE_MISSES, 999, DDPROF_PWT_TRACEPOINT, {}) \ - X(hBRANCH, "Branche Instr.", PERF_TYPE_HARDWARE, PERF_COUNT_HW_BRANCH_INSTRUCTIONS, 999, DDPROF_PWT_TRACEPOINT, {}) \ - X(hBMISS, "Branch Miss", PERF_TYPE_HARDWARE, PERF_COUNT_HW_BRANCH_MISSES, 999, DDPROF_PWT_TRACEPOINT, {}) \ - X(hBUS, "Bus Cycles", PERF_TYPE_HARDWARE, PERF_COUNT_HW_BUS_CYCLES, 1000, DDPROF_PWT_TRACEPOINT, IS_FREQ) \ - X(hBSTF, "Bus Stalls(F)", PERF_TYPE_HARDWARE, PERF_COUNT_HW_STALLED_CYCLES_FRONTEND, 1000, DDPROF_PWT_TRACEPOINT, IS_FREQ) \ - X(hBSTB, "Bus Stalls(B)", PERF_TYPE_HARDWARE, PERF_COUNT_HW_STALLED_CYCLES_BACKEND, 1000, DDPROF_PWT_TRACEPOINT, IS_FREQ) \ - X(sCPU, "CPU Time", PERF_TYPE_SOFTWARE, PERF_COUNT_SW_TASK_CLOCK, 99, DDPROF_PWT_CPU_NANOS, IS_FREQ_TRY_KERNEL) \ - X(sPF, "Page Faults", PERF_TYPE_SOFTWARE, PERF_COUNT_SW_PAGE_FAULTS, 1, DDPROF_PWT_TRACEPOINT, USE_KERNEL) \ - X(sCS, "Con. Switch", PERF_TYPE_SOFTWARE, PERF_COUNT_SW_CONTEXT_SWITCHES, 1, DDPROF_PWT_TRACEPOINT, USE_KERNEL) \ - X(sMig, "CPU Migrations", PERF_TYPE_SOFTWARE, PERF_COUNT_SW_CPU_MIGRATIONS, 99, DDPROF_PWT_TRACEPOINT, IS_FREQ) \ - X(sPFMAJ, "Minor Faults", PERF_TYPE_SOFTWARE, PERF_COUNT_SW_PAGE_FAULTS_MIN, 99, DDPROF_PWT_TRACEPOINT, USE_KERNEL) \ - X(sPFMIN, "Major Faults", PERF_TYPE_SOFTWARE, PERF_COUNT_SW_PAGE_FAULTS_MAJ, 99, DDPROF_PWT_TRACEPOINT, USE_KERNEL) \ - X(sALGN, "Align. Faults", PERF_TYPE_SOFTWARE, PERF_COUNT_SW_ALIGNMENT_FAULTS, 99, DDPROF_PWT_TRACEPOINT, IS_FREQ) \ - X(sEMU, "Emu. Faults", PERF_TYPE_SOFTWARE, PERF_COUNT_SW_EMULATION_FAULTS, 99, DDPROF_PWT_TRACEPOINT, IS_FREQ) \ - X(sDUM, "Dummy", PERF_TYPE_SOFTWARE, PERF_COUNT_SW_DUMMY, 1, DDPROF_PWT_NOCOUNT, {}) \ - X(sALLOC, "Allocations", kDDPROF_TYPE_CUSTOM, kDDPROF_COUNT_ALLOCATIONS, 524288, DDPROF_PWT_ALLOC_SPACE, SKIP_FRAMES) + X(hCPU, "CPU Cycles", PERF_TYPE_HARDWARE, PERF_COUNT_HW_CPU_CYCLES, 99, DDPROF_PWT_TRACEPOINT, IS_FREQ) \ + X(hREF, "Ref. CPU Cycles", PERF_TYPE_HARDWARE, PERF_COUNT_HW_REF_CPU_CYCLES, 1000, DDPROF_PWT_TRACEPOINT, IS_FREQ) \ + X(hINST, "Instr. Count", PERF_TYPE_HARDWARE, PERF_COUNT_HW_INSTRUCTIONS, 1000, DDPROF_PWT_TRACEPOINT, IS_FREQ) \ + X(hCREF, "Cache Ref.", PERF_TYPE_HARDWARE, PERF_COUNT_HW_CACHE_REFERENCES, 999, DDPROF_PWT_TRACEPOINT, {}) \ + X(hCMISS, "Cache Miss", PERF_TYPE_HARDWARE, PERF_COUNT_HW_CACHE_MISSES, 999, DDPROF_PWT_TRACEPOINT, {}) \ + X(hBRANCH, "Branche Instr.", PERF_TYPE_HARDWARE, PERF_COUNT_HW_BRANCH_INSTRUCTIONS, 999, DDPROF_PWT_TRACEPOINT, {}) \ + X(hBMISS, "Branch Miss", PERF_TYPE_HARDWARE, PERF_COUNT_HW_BRANCH_MISSES, 999, DDPROF_PWT_TRACEPOINT, {}) \ + X(hBUS, "Bus Cycles", PERF_TYPE_HARDWARE, PERF_COUNT_HW_BUS_CYCLES, 1000, DDPROF_PWT_TRACEPOINT, IS_FREQ) \ + X(hBSTF, "Bus Stalls(F)", PERF_TYPE_HARDWARE, PERF_COUNT_HW_STALLED_CYCLES_FRONTEND, 1000, DDPROF_PWT_TRACEPOINT, IS_FREQ) \ + X(hBSTB, "Bus Stalls(B)", PERF_TYPE_HARDWARE, PERF_COUNT_HW_STALLED_CYCLES_BACKEND, 1000, DDPROF_PWT_TRACEPOINT, IS_FREQ) \ + X(sCPU, "CPU Time", PERF_TYPE_SOFTWARE, PERF_COUNT_SW_TASK_CLOCK, 99, DDPROF_PWT_CPU_NANOS, IS_FREQ_TRY_KERNEL) \ + X(sPF, "Page Faults", PERF_TYPE_SOFTWARE, PERF_COUNT_SW_PAGE_FAULTS, 1, DDPROF_PWT_TRACEPOINT, USE_KERNEL) \ + X(sCS, "Con. Switch", PERF_TYPE_SOFTWARE, PERF_COUNT_SW_CONTEXT_SWITCHES, 1, DDPROF_PWT_TRACEPOINT, USE_KERNEL) \ + X(sMig, "CPU Migrations", PERF_TYPE_SOFTWARE, PERF_COUNT_SW_CPU_MIGRATIONS, 99, DDPROF_PWT_TRACEPOINT, IS_FREQ) \ + X(sPFMAJ, "Major Faults", PERF_TYPE_SOFTWARE, PERF_COUNT_SW_PAGE_FAULTS_MAJ, 99, DDPROF_PWT_TRACEPOINT, USE_KERNEL) \ + X(sPFMIN, "Minor Faults", PERF_TYPE_SOFTWARE, PERF_COUNT_SW_PAGE_FAULTS_MIN, 99, DDPROF_PWT_TRACEPOINT, USE_KERNEL) \ + X(sALGN, "Align. Faults", PERF_TYPE_SOFTWARE, PERF_COUNT_SW_ALIGNMENT_FAULTS, 99, DDPROF_PWT_TRACEPOINT, IS_FREQ) \ + X(sEMU, "Emu. Faults", PERF_TYPE_SOFTWARE, PERF_COUNT_SW_EMULATION_FAULTS, 99, DDPROF_PWT_TRACEPOINT, IS_FREQ) \ + X(sDUM, "Dummy", PERF_TYPE_SOFTWARE, PERF_COUNT_SW_DUMMY, 1, DDPROF_PWT_NOCOUNT, {}) \ + X(tALLOCSYS1, "System Allocations", PERF_TYPE_TRACEPOINT, kDDPROF_COUNT_SYSALLOCATIONS, 1, DDPROF_PWT_ALLOC_SPACE, IS_OVERLOADED) \ + X(tALLOCSYS2, "System Al. (heavy)", PERF_TYPE_TRACEPOINT, kDDPROF_COUNT_SYSALLOCATIONS, 1, DDPROF_PWT_ALLOC_SPACE, IS_OVERLOADED) \ + X(sALLOC, "Allocations", kDDPROF_TYPE_CUSTOM, kDDPROF_COUNT_ALLOCATIONS, 524288, DDPROF_PWT_ALLOC_SPACE, SKIP_FRAMES) // clang-format on #define X_ENUM(a, b, c, d, e, f, g) DDPROF_PWE_##a, diff --git a/include/pevent.hpp b/include/pevent.hpp index d2016c8b4..f4c17efcd 100644 --- a/include/pevent.hpp +++ b/include/pevent.hpp @@ -24,6 +24,8 @@ typedef struct PEvent { bool custom_event; // true if custom event (not handled by perf, eg. memory // allocations) RingBuffer rb; // metadata and buffers for processing perf ringbuffer + int child_fds[MAX_NB_PERF_EVENT_OPEN]; + int current_child_fd; } PEvent; typedef struct PEventHdr { diff --git a/src/ddprof_cmdline.cc b/src/ddprof_cmdline.cc index 11b0645c6..73dbd75d5 100644 --- a/src/ddprof_cmdline.cc +++ b/src/ddprof_cmdline.cc @@ -48,6 +48,67 @@ bool arg_yesno(const char *str, int mode) { return false; } +long id_from_tracepoint(const char *gname, const char *tname) { + char path[2048] = {0}; // somewhat arbitrarily + size_t sz_path = sizeof(path); + char buf[64] = {0}; + char *buf_copy = buf; + + // Need to figure out whether we use debugfs or tracefs + static int use_tracefs = -1; // -2 error, -1 init, 0 no, 1 yes + static char tracefs_path[] = "/sys/kernel/tracing/events"; + static char debugfs_path[] = "/sys/kernel/debug/tracing/events"; + + if (!gname || !*gname || !tname || !*tname) { + return -1; + } + + if (use_tracefs == -2) { + // We checked in a previous loop and couldn't read tracef or debugfs + return -1; + } else if (use_tracefs == -1) { + struct stat sb; + if (stat(tracefs_path, &sb)) { + // If we're here, the stat failed so we can't use tracefs + if (stat(debugfs_path, &sb)) { + // If we're here, debugfs failed too, return error + use_tracefs = -2; + return -1; + } + use_tracefs = 0; // Use debugfs + } else { + use_tracefs = 1; // Use tracefs + } + } + + // Check validity of given tracepoint + char *spath = use_tracefs ? tracefs_path : debugfs_path; + int pathsz = snprintf(path, sz_path, "%s/%s/%s/id", spath, gname, tname); + if (static_cast(pathsz) >= sz_path) { + // Possibly ran out of room + return -1; + } + int fd = open(path, O_RDONLY); + if (-1 == fd) { + return -1; + } + + // Read the data in an eintr-safe way + int read_ret = -1; + long trace_id = -1; + do { + read_ret = read(fd, buf, sizeof(buf)); + } while (read_ret == -1 && errno == EINTR); + close(fd); + if (read_ret > 0) + trace_id = strtol(buf, &buf_copy, 10); + if (*buf_copy && *buf_copy != '\n') { + return -1; + } + + return trace_id; +} + unsigned int tracepoint_id_from_event(const char *eventname, const char *groupname) { if (!eventname || !*eventname || !groupname || !*groupname) @@ -169,5 +230,36 @@ bool watcher_from_str(const char *str, PerfWatcher *watcher) { watcher->tracepoint_event = conf->eventname; watcher->tracepoint_group = conf->groupname; watcher->tracepoint_label = conf->label; + + // Certain watcher configs get additional event information + if (watcher->config == kDDPROF_COUNT_ALLOCATIONS) { + watcher->sample_type |= PERF_SAMPLE_ADDR; + } + + // Some profiling types get lots of additional state transplanted here + if (watcher->options.is_overloaded) { + if (watcher->ddprof_event_type == DDPROF_PWE_tALLOCSYS1) { + // tALLOCSY1 overrides perfopen to bind together many file descriptors + watcher->tracepoint_group = "syscalls"; + watcher->tracepoint_label = "sys_exit_mmap"; + watcher->instrument_self = true; + watcher->options.use_kernel = PerfWatcherUseKernel::kTry; + watcher->sample_stack_size /= 2; // Make this one smaller than normal + + } else if (watcher->ddprof_event_type == DDPROF_PWE_tALLOCSYS2) { + // tALLOCSYS2 captures all syscalls; used to troubleshoot 1 + watcher->tracepoint_group = "raw_syscalls"; + watcher->tracepoint_label = "sys_exit"; + long id = id_from_tracepoint("raw_syscalls", "sys_exit"); + if (-1 == id) { + // We mutated the user's event, but it is invalid. + return false; + } + watcher->config = id; + } + watcher->sample_type |= PERF_SAMPLE_RAW; + watcher->options.use_kernel = PerfWatcherUseKernel::kTry; + } + return true; } diff --git a/src/ddprof_context_lib.cc b/src/ddprof_context_lib.cc index 78a18c6dd..00f0fc274 100644 --- a/src/ddprof_context_lib.cc +++ b/src/ddprof_context_lib.cc @@ -152,6 +152,17 @@ DDRes ddprof_context_set(DDProfInput *input, DDProfContext *ctx) { ctx->watchers[nwatchers] = input->watchers[nwatchers]; } ctx->num_watchers = nwatchers; + + // Some profiling features, like system allocations, uses ctx storage and + // needs to associate a watcher (but only one watcher) to that storage. + for (int i = 0; i < ctx->num_watchers; ++i) { + if (ctx->watchers[i].ddprof_event_type == DDPROF_PWE_tALLOCSYS1 || + ctx->watchers[i].ddprof_event_type == DDPROF_PWE_tALLOCSYS2) { + ctx->worker_ctx.sys_allocation.watcher_pos = i; + break; + } + } + // Set defaults ctx->params.upload_period = 60.0; diff --git a/src/ddprof_worker.cc b/src/ddprof_worker.cc index 6f3d47304..ecc1d436f 100644 --- a/src/ddprof_worker.cc +++ b/src/ddprof_worker.cc @@ -354,6 +354,77 @@ DDRes ddprof_pr_allocation_tracking(DDProfContext *ctx, return ddres_init(); } +DDRes ddprof_pr_sysallocation_tracking(DDProfContext *ctx, + perf_event_sample *sample, + int watcher_pos) { + + // Syscall parameters. Suppressing nags because it's annoying to look these + // up and it isn't totally appropriate to spin out a new header just + // for this + int64_t id; + memcpy(&id, sample->data_raw + 8, sizeof(id)); + auto &sysalloc = ctx->worker_ctx.sys_allocation; + +#ifdef __x86_64__ + [[maybe_unused]] uint64_t sc_ret = sample->regs[PAM_X86_RAX]; + [[maybe_unused]] uint64_t sc_p1 = sample->regs[PAM_X86_RDI]; + [[maybe_unused]] uint64_t sc_p2 = sample->regs[PAM_X86_RSI]; + [[maybe_unused]] uint64_t sc_p3 = sample->regs[PAM_X86_RDX]; + [[maybe_unused]] uint64_t sc_p4 = sample->regs[PAM_X86_R10]; + [[maybe_unused]] uint64_t sc_p5 = sample->regs[PAM_X86_R8]; + [[maybe_unused]] uint64_t sc_p6 = sample->regs[PAM_X86_R9]; +#elif __aarch64__ + // Obviously ARM is totally broken here. + [[maybe_unused]] uint64_t sc_ret = sample->regs[PAM_ARM_X0]; + [[maybe_unused]] uint64_t sc_p1 = sample->regs[PAM_ARM_X0]; + [[maybe_unused]] uint64_t sc_p2 = sample->regs[PAM_ARM_X1]; + [[maybe_unused]] uint64_t sc_p3 = sample->regs[PAM_ARM_X2]; + [[maybe_unused]] uint64_t sc_p4 = sample->regs[PAM_ARM_X3]; + [[maybe_unused]] uint64_t sc_p5 = sample->regs[PAM_ARM_X4]; + [[maybe_unused]] uint64_t sc_p6 = sample->regs[PAM_ARM_X5]; +#else +# error Architecture not supported +#endif + if (sc_ret > -4096UL) { + // If the syscall returned error, it didn't mutate state. Skip! + // ("high" values are errors, as per standard) + return ddres_init(); + } + + // Only unwind if we will need to propagate unwinding information forward + DDRes res = {}; + UnwindOutput *uwo = NULL; + if (id == 9 || id == 25) { + auto ticks0 = ddprof::get_tsc_cycles(); + res = ddprof_unwind_sample(ctx, sample, watcher_pos); + auto unwind_ticks = ddprof::get_tsc_cycles(); + ddprof_stats_add(STATS_UNWIND_AVG_TIME, unwind_ticks - ticks0, NULL); + uwo = &ctx->worker_ctx.us->output; + + // TODO: propagate fatal + if (IsDDResFatal(res)) { + return ddres_init(); + } + } + + // hardcoded syscall numbers; these are uniform between x86/arm + if (id == 9) { + sysalloc.do_mmap(*uwo, sc_ret, sc_p2, sample->pid); + } else if (id == 11) { + sysalloc.do_munmap(sc_p1, sc_p2, sample->pid); + } else if (id == 28) { + // Unhandled, no need to handle + } else if (id == 25) { + sysalloc.do_mremap(*uwo, sc_ret, sc_p1, sc_p2, sc_p3, sample->pid); + } else if (id == 60 || id == 231 || id == 59 || id == 322 || id == 520 || + id == 545) { + // Erase upon exit or exec + sysalloc.do_exit(sample->pid); + } + + return ddres_init(); +} + static void ddprof_reset_worker_stats() { for (unsigned i = 0; i < std::size(s_cycled_stats); ++i) { ddprof_stats_clear(s_cycled_stats[i]); @@ -415,6 +486,30 @@ static DDRes aggregate_live_allocations(DDProfContext *ctx) { } return ddres_init(); } + +static DDRes aggregate_sys_allocations(DDProfContext *ctx) { + struct UnwindState *us = ctx->worker_ctx.us; + SystemAllocation &sysallocs = ctx->worker_ctx.sys_allocation; + PerfWatcher *watcher = &ctx->watchers[sysallocs.watcher_pos]; + int i_export = ctx->worker_ctx.i_current_pprof; + DDProfPProf *pprof = ctx->worker_ctx.pprof[i_export]; + + // Before we do anything, clear the pids that died in this period + sysallocs.sanitize_pids(); + + // Iterate through each PID + for (auto &stack_map : sysallocs._pid_map) { + + // Iterate through pages... + // TODO Probably aggregate into ranges of pages or something, but once per + // page is just too much + for (const auto &page : stack_map.second) { + DDRES_CHECK_FWD(pprof_aggregate(&page.second, &us->symbol_hdr, 4096, 1, + watcher, pprof)); + } + } + return ddres_init(); +} #endif /// Cycle operations : export, sync metrics, update counters @@ -424,6 +519,7 @@ DDRes ddprof_worker_cycle(DDProfContext *ctx, int64_t now, #ifndef DDPROF_NATIVE_LIB // TODO: lib mode (unhandled for now) DDRES_CHECK_FWD(aggregate_live_allocations(ctx)); + DDRES_CHECK_FWD(aggregate_sys_allocations(ctx)); // Take the current pprof contents and ship them to the backend. This also // clears the pprof for reuse @@ -671,19 +767,27 @@ DDRes ddprof_worker_process_event(const perf_event_header *hdr, int watcher_pos, ddprof_stats_add(STATS_EVENT_COUNT, 1, NULL); const perf_event_hdr_wpid *wpid = static_cast(hdr); + PerfWatcher *watcher = &ctx->watchers[watcher_pos]; switch (hdr->type) { /* Cases where the target type has a PID */ case PERF_RECORD_SAMPLE: if (wpid->pid) { - uint64_t mask = ctx->watchers[watcher_pos].sample_type; - bool is_allocation = - (ctx->watchers[watcher_pos].type == kDDPROF_TYPE_CUSTOM && - ctx->watchers[watcher_pos].config == kDDPROF_COUNT_ALLOCATIONS); - if (is_allocation) // temp hack - mask |= PERF_SAMPLE_ADDR; + uint64_t mask = watcher->sample_type; perf_event_sample *sample = hdr2samp(hdr, mask); + + // Various checks for allocation profiling + // - sALLOC + // - mmap/munmap syscalls + bool is_allocation = watcher->type == kDDPROF_TYPE_CUSTOM && + watcher->config == kDDPROF_COUNT_ALLOCATIONS; if (sample) { - if (is_allocation && ctx->params.live_allocations) { + + // Handle special profiling types first + if (watcher->ddprof_event_type == DDPROF_PWE_tALLOCSYS1 || + watcher->ddprof_event_type == DDPROF_PWE_tALLOCSYS2) { + DDRES_CHECK_FWD( + ddprof_pr_sysallocation_tracking(ctx, sample, watcher_pos)); + } else if (is_allocation && ctx->params.live_allocations) { DDRES_CHECK_FWD( ddprof_pr_allocation_tracking(ctx, sample, watcher_pos)); } else { diff --git a/src/perf_watcher.cc b/src/perf_watcher.cc index cad7562bc..4267b6609 100644 --- a/src/perf_watcher.cc +++ b/src/perf_watcher.cc @@ -94,6 +94,7 @@ const PerfWatcher *tracepoint_default_watcher() { .type = PERF_TYPE_TRACEPOINT, .sample_period = 1, .sample_type_id = DDPROF_PWT_TRACEPOINT, + .sample_stack_size = PERF_SAMPLE_STACK_SIZE, .options = {.use_kernel = PerfWatcherUseKernel::kRequired}, .value_scale = 1.0, }; diff --git a/src/pevent_lib.cc b/src/pevent_lib.cc index 07997aff5..184984c30 100644 --- a/src/pevent_lib.cc +++ b/src/pevent_lib.cc @@ -5,6 +5,7 @@ #include "pevent_lib.hpp" +#include "ddprof_cmdline.hpp" #include "ddres.hpp" #include "defer.hpp" #include "perf.hpp" @@ -63,6 +64,85 @@ static void pevent_set_info(int fd, int attr_idx, PEvent &pevent) { pevent.attr_idx = attr_idx; } +static void pevent_add_child_fd(int child_fd, PEvent &pevent) { + pevent.child_fds[pevent.current_child_fd++] = child_fd; +} + +static DDRes tallocsys1_open(PerfWatcher *watcher, int watcher_idx, pid_t pid, + int num_cpu, PEventHdr *pevent_hdr) { + PerfWatcher watcher_copy = *watcher; + PEvent *pes = pevent_hdr->pes; + + struct talloc_conf { + int fd; + bool enable_userstack; + }; + std::unordered_map kprobes{ + {"sys_exit_mmap", {-1, true}}, + {"sys_exit_munmap", {-1, false}}, + {"sys_exit_mremap", {-1, true}}}; + + // Set the IDs + for (auto &kprobe : kprobes) { + long id = id_from_tracepoint("syscalls", kprobe.first.c_str()); + if (-1 == id) { + DDRES_RETURN_ERROR_LOG(DD_WHAT_PERFOPEN, + "Error opening tracefs for tALLOCSYS1 on %s", + kprobe.first.c_str()); + } + kprobes[kprobe.first].fd = id; + } + + // Iterate + for (int cpu_idx = 0; cpu_idx < num_cpu; ++cpu_idx) { + int fd = -1; + // Create the pevent which will consolidate this watcher + size_t pevent_idx = -1; + DDRES_CHECK_FWD(pevent_create(pevent_hdr, watcher_idx, &pevent_idx)); + perf_event_attr attr = {}; + std::vector cleanup_fds; + + // This is very imperfect since failure leaves a dangling pevent + // TODO + for (auto &kprobe : kprobes) { + watcher_copy.tracepoint_group = "syscalls"; + watcher_copy.tracepoint_label = kprobe.first.c_str(); + watcher_copy.config = kprobe.second.fd; + + // THIS IS WRONG + if (kprobe.second.enable_userstack) { + watcher_copy.sample_stack_size = watcher->sample_stack_size; + } else { + watcher_copy.sample_stack_size = 0; + } + + attr = perf_config_from_watcher(&watcher_copy, true); + int fd_tmp = -1; + fd_tmp = perf_event_open(&attr, pid, cpu_idx, -1, PERF_FLAG_FD_CLOEXEC); + + if (-1 == fd_tmp) { + for (auto cleanup_fd : cleanup_fds) { + close(cleanup_fd); + } + DDRES_RETURN_ERROR_LOG(DD_WHAT_PERFOPEN, + "Error calling perfopen for tALLOCSYS1 on %s", + kprobe.first.c_str()); + } + if (-1 != fd) { + pevent_add_child_fd(fd_tmp, pes[pevent_idx]); + } else { + fd = fd_tmp; + } + cleanup_fds.push_back(fd_tmp); + } + pevent_hdr->attrs[pevent_hdr->nb_attrs] = attr; + pevent_set_info(fd, pes[pevent_idx].attr_idx, pes[pevent_idx]); + ++pevent_hdr->nb_attrs; + } + + return ddres_init(); +} + static DDRes pevent_register_cpu_0(const PerfWatcher *watcher, int watcher_idx, pid_t pid, PEventHdr *pevent_hdr, size_t &pevent_idx) { @@ -131,9 +211,17 @@ DDRes pevent_open(DDProfContext *ctx, pid_t pid, int num_cpu, PEventHdr *pevent_hdr) { assert(pevent_hdr->size == 0); // check for previous init for (int watcher_idx = 0; watcher_idx < ctx->num_watchers; ++watcher_idx) { - if (ctx->watchers[watcher_idx].type < kDDPROF_TYPE_CUSTOM) { - DDRES_CHECK_FWD(pevent_open_all_cpus( - &ctx->watchers[watcher_idx], watcher_idx, pid, num_cpu, pevent_hdr)); + PerfWatcher *watcher = &ctx->watchers[watcher_idx]; + if (watcher->instrument_self) { + // Here we inline a lookup for the specific handler, but in reality this + // should be defined at the level of the watcher + if (watcher->ddprof_event_type == DDPROF_PWE_tALLOCSYS1) { + DDRES_CHECK_FWD( + tallocsys1_open(watcher, watcher_idx, pid, num_cpu, pevent_hdr)); + } + } else if (watcher->type < kDDPROF_TYPE_CUSTOM) { + DDRES_CHECK_FWD( + pevent_open_all_cpus(watcher, watcher_idx, pid, num_cpu, pevent_hdr)); } else { // custom event, eg.allocation profiling size_t pevent_idx = 0; @@ -201,6 +289,23 @@ DDRes pevent_setup(DDProfContext *ctx, pid_t pid, int num_cpu, LG_NTC("Retrying attachment without user override"); DDRES_CHECK_FWD(pevent_mmap(pevent_hdr, false)); } + + // If any watchers have self-instrumentation, then they may have set up child + // fds which now need to be consolidated via ioctl. These fds cannot be + // closed until profiling is completed. + for (unsigned i = 0; i < pevent_hdr->size; i++) { + PEvent *pes = &pevent_hdr->pes[i]; + if (ctx->watchers[pes->watcher_pos].instrument_self) { + int fd = pes->fd; + for (int j = 0; j < pes->current_child_fd; ++j) { + int child_fd = pes->child_fds[j]; + if (ioctl(child_fd, PERF_EVENT_IOC_SET_OUTPUT, fd)) { + DDRES_RETURN_ERROR_LOG(DD_WHAT_PERFOPEN, + "Could not ioctl() tALLOCSYS1"); + } + } + } + } return ddres_init(); } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 8e1acfac2..07011474c 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -140,6 +140,7 @@ add_unit_test(perf_ringbuffer-ut ../src/perf.cc ../src/perf_watcher.cc ../src/pe add_unit_test( pevent-ut + ../src/ddprof_cmdline.cc ../src/pevent_lib.cc ../src/user_override.cc ../src/perf.cc @@ -148,6 +149,7 @@ add_unit_test( ../src/ringbuffer_utils.cc ../src/sys_utils.cc pevent-ut.cc + LIBRARIES DDProf::Parser DEFINITIONS MYNAME="pevent-ut") add_unit_test( @@ -251,6 +253,7 @@ add_unit_test( ../src/build_id.cc ../src/common_mapinfo_lookup.cc ../src/common_symbol_lookup.cc + ../src/ddprof_cmdline.cc ../src/ddprof_file_info.cc ../src/ddprof_stats.cc ../src/dso.cc @@ -283,7 +286,7 @@ add_unit_test( ../src/unwind_dwfl.cc ../src/unwind_helpers.cc ../src/unwind_metrics.cc - LIBRARIES ${ELFUTILS_LIBRARIES} llvm-demangle + LIBRARIES ${ELFUTILS_LIBRARIES} llvm-demangle DDProf::Parser DEFINITIONS ${DDPROF_DEFINITION_LIST}) add_unit_test(sys_utils-ut sys_utils-ut.cc ../src/sys_utils.cc) @@ -291,12 +294,15 @@ add_unit_test(sys_utils-ut sys_utils-ut.cc ../src/sys_utils.cc) add_unit_test( ringbuffer-ut ringbuffer-ut.cc + ../src/ddprof_cmdline.cc ../src/perf.cc ../src/perf_ringbuffer.cc + ../src/perf_watcher.cc ../src/pevent_lib.cc ../src/ringbuffer_utils.cc ../src/sys_utils.cc - ../src/user_override.cc) + ../src/user_override.cc + LIBRARIES DDProf::Parser) add_unit_test(timer-ut timer-ut.cc ../src/timer.cc ../src/perf.cc) From b332e2c47fe001e1e74ae96ada217a2128e6fd99 Mon Sep 17 00:00:00 2001 From: r1viollet Date: Thu, 3 Nov 2022 11:03:59 +0100 Subject: [PATCH 05/21] PID cleanup Ensuse that the mechanism to clear unvisited PIDs is shared --- include/dwfl_hdr.hpp | 3 ++- src/ddprof_worker.cc | 11 +++++++++++ src/dwfl_hdr.cc | 9 ++++----- src/unwind.cc | 1 - 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/include/dwfl_hdr.hpp b/include/dwfl_hdr.hpp index 2669156e2..f719ef191 100644 --- a/include/dwfl_hdr.hpp +++ b/include/dwfl_hdr.hpp @@ -64,7 +64,8 @@ struct DwflWrapper { class DwflHdr { public: DwflWrapper &get_or_insert(pid_t pid); - void clear_unvisited(); + std::vector get_unvisited() const; + std::vector reset_unvisited(); void clear_pid(pid_t pid); // get number of accessed modules diff --git a/src/ddprof_worker.cc b/src/ddprof_worker.cc index ecc1d436f..6f4211a5a 100644 --- a/src/ddprof_worker.cc +++ b/src/ddprof_worker.cc @@ -512,6 +512,15 @@ static DDRes aggregate_sys_allocations(DDProfContext *ctx) { } #endif +static void clear_unvisted_pids(DDProfWorkerContext &worker_ctx) { + UnwindState *us = worker_ctx.us; + const std::vector pids_remove = us->dwfl_hdr.get_unvisited(); + for (pid_t el : pids_remove) { + unwind_pid_free(us, el); + } + us->dwfl_hdr.reset_unvisited(); +} + /// Cycle operations : export, sync metrics, update counters DDRes ddprof_worker_cycle(DDProfContext *ctx, int64_t now, [[maybe_unused]] bool synchronous_export) { @@ -604,6 +613,8 @@ DDRes ddprof_worker_cycle(DDProfContext *ctx, int64_t now, } unwind_cycle(ctx->worker_ctx.us); + clear_unvisted_pids(ctx->worker_ctx); + // Reset stats relevant to a single cycle ddprof_reset_worker_stats(); diff --git a/src/dwfl_hdr.cc b/src/dwfl_hdr.cc index 67ff4628b..3406a3756 100644 --- a/src/dwfl_hdr.cc +++ b/src/dwfl_hdr.cc @@ -81,7 +81,7 @@ DDProfMod *DwflWrapper::register_mod(ProcessAddress_t pc, const Dso &dso, .first->second; } -void DwflHdr::clear_unvisited() { +std::vector DwflHdr::get_unvisited() const { std::vector pids_remove; for (auto &el : _dwfl_map) { if (_visited_pid.find(el.first) == _visited_pid.end()) { @@ -89,11 +89,10 @@ void DwflHdr::clear_unvisited() { pids_remove.push_back(el.first); } } - for (pid_t el : pids_remove) { - _dwfl_map.erase(el); - LG_NFO("[DWFL] DWFL Map Clearing PID%d", el); - } + return pids_remove; +} +std::vector DwflHdr::reset_unvisited() { // clear the list of visited for next cycle _visited_pid.clear(); } diff --git a/src/unwind.cc b/src/unwind.cc index d7c7e12b3..3aad887a3 100644 --- a/src/unwind.cc +++ b/src/unwind.cc @@ -120,7 +120,6 @@ void unwind_cycle(UnwindState *us) { us->symbol_hdr.cycle(); // clean up pids that we did not see recently us->dwfl_hdr.display_stats(); - us->dwfl_hdr.clear_unvisited(); us->dso_hdr._stats.reset(); unwind_metrics_reset(); From 2599c39d45be02f66044ef14661fecaccd7aa81a Mon Sep 17 00:00:00 2001 From: Nicolas Savoire Date: Mon, 30 Jan 2023 15:02:17 +0000 Subject: [PATCH 06/21] Fix missing return value --- include/dwfl_hdr.hpp | 2 +- src/dwfl_hdr.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/dwfl_hdr.hpp b/include/dwfl_hdr.hpp index f719ef191..c37dde031 100644 --- a/include/dwfl_hdr.hpp +++ b/include/dwfl_hdr.hpp @@ -65,7 +65,7 @@ class DwflHdr { public: DwflWrapper &get_or_insert(pid_t pid); std::vector get_unvisited() const; - std::vector reset_unvisited(); + void reset_unvisited(); void clear_pid(pid_t pid); // get number of accessed modules diff --git a/src/dwfl_hdr.cc b/src/dwfl_hdr.cc index 3406a3756..ee93c4dea 100644 --- a/src/dwfl_hdr.cc +++ b/src/dwfl_hdr.cc @@ -92,7 +92,7 @@ std::vector DwflHdr::get_unvisited() const { return pids_remove; } -std::vector DwflHdr::reset_unvisited() { +void DwflHdr::reset_unvisited() { // clear the list of visited for next cycle _visited_pid.clear(); } From 05558d919366cc2bd24ca80281421b6a510ec08b Mon Sep 17 00:00:00 2001 From: r1viollet Date: Sun, 12 Mar 2023 10:53:20 +0100 Subject: [PATCH 07/21] Formalize live alloc setting --- include/ddprof_context.hpp | 1 - include/ddprof_context_lib.hpp | 2 ++ include/ddprof_input.hpp | 4 +-- include/event_config.hpp | 47 ++++++++++++------------- include/ipc.hpp | 2 +- include/perf_watcher.hpp | 1 + src/ddprof_cmdline.cc | 5 +-- src/ddprof_context_lib.cc | 12 +++---- src/ddprof_input.cc | 4 +-- src/ddprof_worker.cc | 6 ++-- src/event_parser/event_parser.y | 20 +++++++---- src/exe/main.cc | 7 ++-- src/lib/dd_profiling.cc | 3 +- test/ddprof_input-ut.cc | 61 +++++++++++++++++++++++++++++++++ test/simple_malloc-ut.sh | 4 +-- 15 files changed, 123 insertions(+), 56 deletions(-) diff --git a/include/ddprof_context.hpp b/include/ddprof_context.hpp index 426d8cc30..e73edb716 100644 --- a/include/ddprof_context.hpp +++ b/include/ddprof_context.hpp @@ -32,7 +32,6 @@ typedef struct DDProfContext { int sockfd; bool wait_on_socket; bool show_samples; - bool live_allocations; // for now this overrides cpu_set_t cpu_affinity; const char *switch_user; const char *internal_stats; diff --git a/include/ddprof_context_lib.hpp b/include/ddprof_context_lib.hpp index af352f5e3..94430a5cb 100644 --- a/include/ddprof_context_lib.hpp +++ b/include/ddprof_context_lib.hpp @@ -14,4 +14,6 @@ typedef struct PerfWatcher PerfWatcher; DDRes ddprof_context_set(DDProfInput *input, DDProfContext *); void ddprof_context_free(DDProfContext *); +void log_watcher(const PerfWatcher *w, int idx); + int ddprof_context_allocation_profiling_watcher_idx(const DDProfContext *ctx); diff --git a/include/ddprof_input.hpp b/include/ddprof_input.hpp index 8cf3b0141..5a0e8a8f7 100644 --- a/include/ddprof_input.hpp +++ b/include/ddprof_input.hpp @@ -37,7 +37,6 @@ typedef struct DDProfInput { char *socket; char *preset; char *switch_user; - char *live_allocations; // Watcher presets PerfWatcher watchers[MAX_TYPE_WATCHER]; int num_watchers; @@ -101,8 +100,7 @@ typedef struct DDProfInput { XX(DD_PROFILING_NATIVE_PRESET, preset, D, 'D', 1, input, NULL, "", ) \ XX(DD_PROFILING_NATIVE_SHOW_SAMPLES, show_samples, y, 'y', 0, input, NULL, "", ) \ XX(DD_PROFILING_NATIVE_CPU_AFFINITY, affinity, a, 'a', 1, input, NULL, "", ) \ - XX(DD_PROFILING_NATIVE_SWITCH_USER, switch_user, W, 'W', 1, input, NULL, "", ) \ - XX(DD_PROFILING_NATIVE_LIVE_ALLOC, live_allocations, k, 'k', 1, input, NULL, "no", ) + XX(DD_PROFILING_NATIVE_SWITCH_USER, switch_user, W, 'W', 1, input, NULL, "", ) // clang-format on #define X_ENUM(a, b, c, d, e, f, g, h, i) a, diff --git a/include/event_config.hpp b/include/event_config.hpp index 9ddcc51e1..9690ee378 100644 --- a/include/event_config.hpp +++ b/include/event_config.hpp @@ -5,39 +5,40 @@ #pragma once -#include - #include +#include // Defines how a sample is aggregated when it is received -enum class EventConfMode { +enum class EventConfMode : uint32_t { kDisabled = 0, - kCallgraph = 1 << 0, - kMetric = 1 << 1, - kAll = kCallgraph | kMetric, + kCallgraph = 1 << 0, // flamegraph of resource usage + kMetric = 1 << 1, // gauge of resource usage + kLiveCallgraph = 1 << 2, // report callgraph of resources still in use + kAll = kCallgraph | kMetric | kLiveCallgraph, }; -// EventConfMode &operator|=(EventConfMode &A, const EventConfMode &B); -// EventConfMode operator&(const EventConfMode &A, const EventConfMode &B); -// bool operator<=(const EventConfMode A, const EventConfMode B); // inclusion -// -constexpr EventConfMode &operator|=(EventConfMode &A, const EventConfMode &B) { - A = static_cast(static_cast(A) | - static_cast(B)); - return A; +constexpr EventConfMode operator|(EventConfMode A, const EventConfMode B) { + return static_cast(static_cast(A) | + static_cast(B)); +} + +constexpr EventConfMode operator|=(EventConfMode &A, const EventConfMode B) { + return A = A | B; +} + +constexpr EventConfMode operator&(const EventConfMode A, + const EventConfMode B) { + return static_cast(static_cast(A) & + static_cast(B)); } -constexpr EventConfMode operator&(const EventConfMode &A, - const EventConfMode &B) { - // & on bitmask enums is valid only in the space spanned by the values - return static_cast(static_cast(A) & - static_cast(B) & - static_cast(EventConfMode::kAll)); +constexpr bool Any(EventConfMode arg) { + return arg != EventConfMode::kDisabled; } -// Bitmask inclusion -constexpr bool operator<=(const EventConfMode A, const EventConfMode B) { - return EventConfMode::kDisabled != ((EventConfMode::kAll & A) & B); +constexpr bool AnyCallgraph(EventConfMode arg) { + return Any((arg & EventConfMode::kLiveCallgraph) | + (arg & EventConfMode::kCallgraph)); } // Defines how samples are weighted diff --git a/include/ipc.hpp b/include/ipc.hpp index 330aa717e..6c78343cc 100644 --- a/include/ipc.hpp +++ b/include/ipc.hpp @@ -94,7 +94,7 @@ struct RingBufferInfo { }; struct ReplyMessage { - enum { kLiveAllocation = 0 }; + enum { kLiveCallgraph = 0 }; // reply with the request flags from the request uint32_t request = 0; // profiler pid diff --git a/include/perf_watcher.hpp b/include/perf_watcher.hpp index cbfe111b1..f658d873f 100644 --- a/include/perf_watcher.hpp +++ b/include/perf_watcher.hpp @@ -144,6 +144,7 @@ enum DDProfCustomCountId { X(tALLOCSYS1, "System Allocations", PERF_TYPE_TRACEPOINT, kDDPROF_COUNT_SYSALLOCATIONS, 1, DDPROF_PWT_ALLOC_SPACE, IS_OVERLOADED) \ X(tALLOCSYS2, "System Al. (heavy)", PERF_TYPE_TRACEPOINT, kDDPROF_COUNT_SYSALLOCATIONS, 1, DDPROF_PWT_ALLOC_SPACE, IS_OVERLOADED) \ X(sALLOC, "Allocations", kDDPROF_TYPE_CUSTOM, kDDPROF_COUNT_ALLOCATIONS, 524288, DDPROF_PWT_ALLOC_SPACE, SKIP_FRAMES) + // clang-format on #define X_ENUM(a, b, c, d, e, f, g) DDPROF_PWE_##a, diff --git a/src/ddprof_cmdline.cc b/src/ddprof_cmdline.cc index 73dbd75d5..ca7824fce 100644 --- a/src/ddprof_cmdline.cc +++ b/src/ddprof_cmdline.cc @@ -222,9 +222,10 @@ bool watcher_from_str(const char *str, PerfWatcher *watcher) { // The output mode isn't set as part of the configuration templates; we // always default to callgraph mode - watcher->output_mode = EventConfMode::kCallgraph; - if (EventConfMode::kAll <= conf->mode) { + if (conf->mode != EventConfMode::kDisabled) { watcher->output_mode = conf->mode; + } else { + watcher->output_mode = EventConfMode::kCallgraph; } watcher->tracepoint_event = conf->eventname; diff --git a/src/ddprof_context_lib.cc b/src/ddprof_context_lib.cc index 00f0fc274..6245296a0 100644 --- a/src/ddprof_context_lib.cc +++ b/src/ddprof_context_lib.cc @@ -56,6 +56,7 @@ DDRes add_preset(DDProfContext *ctx, const char *preset, {"default-pid", {"sCPU"}}, {"cpu_only", {"sCPU"}}, {"alloc_only", {"sALLOC"}}, + {"cpu_live_heap", {"sCPU", "sALLOC mode=l"}}, }; if (preset == "default"sv && pid_or_global_mode) { @@ -99,7 +100,7 @@ DDRes add_preset(DDProfContext *ctx, const char *preset, return {}; } -static void log_watcher(const PerfWatcher *w, int idx) { +void log_watcher(const PerfWatcher *w, int idx) { PRINT_NFO(" ID: %s, Pos: %d, Index: %lu", w->desc.c_str(), idx, w->config); switch (w->value_source) { case EventConfValueSource::kSample: @@ -126,11 +127,12 @@ static void log_watcher(const PerfWatcher *w, int idx) { PRINT_NFO(" Cadence: Freq, Freq: %lu", w->sample_frequency); else PRINT_NFO(" Cadence: Period, Period: %lu", w->sample_period); - - if (EventConfMode::kCallgraph <= w->output_mode) + if (Any(EventConfMode::kCallgraph & w->output_mode)) PRINT_NFO(" Outputting to callgraph (flamegraph)"); - if (EventConfMode::kMetric <= w->output_mode) + if (Any(EventConfMode::kMetric & w->output_mode)) PRINT_NFO(" Outputting to metric"); + if (Any(EventConfMode::kLiveCallgraph & w->output_mode)) + PRINT_NFO(" Outputting to live callgraph"); } /**************************** Argument Processor ***************************/ @@ -324,8 +326,6 @@ DDRes ddprof_context_set(DDProfInput *input, DDProfContext *ctx) { } ctx->params.show_samples = input->show_samples != nullptr; - ctx->params.live_allocations = - arg_yesno(input->live_allocations, 1); // default no if (input->switch_user) { ctx->params.switch_user = strdup(input->switch_user); diff --git a/src/ddprof_input.cc b/src/ddprof_input.cc index 6ac6c870c..223cb5693 100644 --- a/src/ddprof_input.cc +++ b/src/ddprof_input.cc @@ -146,9 +146,7 @@ const char* help_str[DD_KLEN] = { [DD_PROFILING_NATIVE_SHOW_SAMPLES] = STR_UNDF, [DD_PROFILING_NATIVE_CPU_AFFINITY] = STR_UNDF, [DD_PROFILING_NATIVE_SWITCH_USER] = - " Run the target process under the given user.\n", - [DD_PROFILING_NATIVE_LIVE_ALLOC] = - " Report only allocations that were not matched with a free.\n", + " Run the target process under the given user.\n" }; // clang-format on diff --git a/src/ddprof_worker.cc b/src/ddprof_worker.cc index 6f4211a5a..6ecf6789b 100644 --- a/src/ddprof_worker.cc +++ b/src/ddprof_worker.cc @@ -244,7 +244,7 @@ static DDRes ddprof_unwind_sample(DDProfContext *ctx, perf_event_sample *sample, // Attempt to fully unwind if the watcher has a callgraph type DDRes res = {}; - if (EventConfMode::kCallgraph <= watcher->output_mode) + if (AnyCallgraph(watcher->output_mode)) res = unwindstate__unwind(us); /* This test is not 100% accurate: @@ -792,13 +792,13 @@ DDRes ddprof_worker_process_event(const perf_event_header *hdr, int watcher_pos, bool is_allocation = watcher->type == kDDPROF_TYPE_CUSTOM && watcher->config == kDDPROF_COUNT_ALLOCATIONS; if (sample) { - // Handle special profiling types first if (watcher->ddprof_event_type == DDPROF_PWE_tALLOCSYS1 || watcher->ddprof_event_type == DDPROF_PWE_tALLOCSYS2) { DDRES_CHECK_FWD( ddprof_pr_sysallocation_tracking(ctx, sample, watcher_pos)); - } else if (is_allocation && ctx->params.live_allocations) { + } else if (is_allocation && + watcher->output_mode == EventConfMode::kLiveCallgraph) { DDRES_CHECK_FWD( ddprof_pr_allocation_tracking(ctx, sample, watcher_pos)); } else { diff --git a/src/event_parser/event_parser.y b/src/event_parser/event_parser.y index 5402225bd..0fa30b8e5 100644 --- a/src/event_parser/event_parser.y +++ b/src/event_parser/event_parser.y @@ -38,18 +38,24 @@ EventConfMode mode_from_str(const std::string &str) { EventConfMode mode = EventConfMode::kDisabled; if (str.empty()) return mode; - - const std::string m_str{"Mm"}; - const std::string g_str{"Gg"}; const std::string a_str{"Aa*"}; + const std::string l_str{"Ll"}; + const std::string g_str{"Gg"}; + const std::string m_str{"Mm"}; for (const char &c : str) { - if (m_str.find(c) != std::string::npos) + if (m_str.find(c) != std::string::npos) { mode |= EventConfMode::kMetric; - if (g_str.find(c) != std::string::npos) + } + if (g_str.find(c) != std::string::npos) { mode |= EventConfMode::kCallgraph; - if (a_str.find(c) != std::string::npos) + } + if (l_str.find(c) != std::string::npos) { + mode |= EventConfMode::kLiveCallgraph; + } + if (a_str.find(c) != std::string::npos) { mode |= EventConfMode::kAll; + } } return mode; } @@ -89,7 +95,7 @@ void conf_print(const EventConf *tp) { else printf(" label: \n"); - const char *modenames[] = {"ILLEGAL", "callgraph", "metric", "metric and callgraph"}; + const char *modenames[] = {"ILLEGAL", "callgraph", "metric", "live callgraph", "metric and callgraph"}; printf(" type: %s\n", modenames[static_cast(tp->mode)]); diff --git a/src/exe/main.cc b/src/exe/main.cc index 346adb9fb..3f827c76b 100644 --- a/src/exe/main.cc +++ b/src/exe/main.cc @@ -356,9 +356,10 @@ static int start_profiler_internal(DDProfContext *ctx, bool &is_profiler) { static_cast(event_it->ring_buffer_type); reply.allocation_profiling_rate = ctx->watchers[alloc_watcher_idx].sample_period; - if (ctx->params.live_allocations) { - reply.allocation_flags |= - (1 << ddprof::ReplyMessage::kLiveAllocation); + + if (ctx->watchers[alloc_watcher_idx].output_mode == + EventConfMode::kLiveCallgraph) { + reply.allocation_flags |= (1 << ddprof::ReplyMessage::kLiveCallgraph); } } } diff --git a/src/lib/dd_profiling.cc b/src/lib/dd_profiling.cc index 457002a22..a38dbe0ee 100644 --- a/src/lib/dd_profiling.cc +++ b/src/lib/dd_profiling.cc @@ -250,8 +250,7 @@ int ddprof_start_profiling_internal() { info.allocation_profiling_rate = -info.allocation_profiling_rate; } - if (info.allocation_flags & - (1 << ddprof::ReplyMessage::kLiveAllocation)) { + if (info.allocation_flags & (1 << ddprof::ReplyMessage::kLiveCallgraph)) { // tracking deallocations to allow a live view flags |= ddprof::AllocationTracker::kTrackDeallocations; } diff --git a/test/ddprof_input-ut.cc b/test/ddprof_input-ut.cc index 5f64b5254..0b6f6637e 100644 --- a/test/ddprof_input-ut.cc +++ b/test/ddprof_input-ut.cc @@ -6,6 +6,7 @@ #include "ddprof_input.hpp" #include "constants.hpp" +#include "ddprof_cmdline.hpp" #include "ddprof_context.hpp" #include "ddprof_context_lib.hpp" #include "defer.hpp" @@ -30,6 +31,15 @@ bool s_version_called = false; void print_version() { s_version_called = true; } string_view str_version() { return STRING_VIEW_LITERAL("1.2.3"); } +TEST_F(InputTest, watcher_from_str) { + LogHandle handle; + const char *str_event = "sALLOC mode=l"; + PerfWatcher watcher; + bool ret = watcher_from_str(str_event, &watcher); + ASSERT_TRUE(ret); + log_watcher(&watcher, 0); +} + TEST_F(InputTest, default_values) { DDProfInput input; DDRes res = ddprof_input_default(&input); @@ -229,6 +239,7 @@ TEST_F(InputTest, duplicate_events) { } TEST_F(InputTest, presets) { + LogHandle handle; { // Default preset should be CPU + ALLOC DDProfInput input; @@ -262,6 +273,7 @@ TEST_F(InputTest, presets) { ddprof_input_free(&input); ddprof_context_free(&ctx); } + { // Default preset for PID mode should be CPU DDProfInput input; @@ -329,6 +341,55 @@ TEST_F(InputTest, presets) { ddprof_input_free(&input); ddprof_context_free(&ctx); } + { + // Check manual setting of live allocation + DDProfInput input; + bool contine_exec = true; + const char *input_values[] = {MYNAME, "-e", "sALLOC mode=l", "my_program"}; + DDRes res = ddprof_input_parse( + std::size(input_values), (char **)input_values, &input, &contine_exec); + + EXPECT_TRUE(IsDDResOK(res)); + EXPECT_TRUE(contine_exec); + + DDProfContext ctx; + res = ddprof_context_set(&input, &ctx); + EXPECT_TRUE(IsDDResOK(res)); + + EXPECT_EQ(ctx.num_watchers, 2); + EXPECT_EQ(ctx.watchers[1].ddprof_event_type, DDPROF_PWE_sALLOC); + EXPECT_EQ(ctx.watchers[1].output_mode, EventConfMode::kLiveCallgraph); + log_watcher(&ctx.watchers[0], 0); + log_watcher(&ctx.watchers[1], 1); + + ddprof_input_free(&input); + ddprof_context_free(&ctx); + } + { + // Check cpu_live_heap preset + DDProfInput input; + bool contine_exec = true; + const char *input_values[] = {MYNAME, "--preset", "cpu_live_heap", + "my_program"}; + DDRes res = ddprof_input_parse( + std::size(input_values), (char **)input_values, &input, &contine_exec); + + EXPECT_TRUE(IsDDResOK(res)); + EXPECT_TRUE(contine_exec); + + DDProfContext ctx; + res = ddprof_context_set(&input, &ctx); + EXPECT_TRUE(IsDDResOK(res)); + + EXPECT_EQ(ctx.num_watchers, 2); + EXPECT_EQ(ctx.watchers[1].ddprof_event_type, DDPROF_PWE_sALLOC); + EXPECT_EQ(ctx.watchers[1].output_mode, EventConfMode::kLiveCallgraph); + EXPECT_EQ(ctx.watchers[0].ddprof_event_type, DDPROF_PWE_sCPU); + EXPECT_EQ(ctx.watchers[0].output_mode, EventConfMode::kCallgraph); + + ddprof_input_free(&input); + ddprof_context_free(&ctx); + } { // Default preset should not be loaded if an event is given in input DDProfInput input; diff --git a/test/simple_malloc-ut.sh b/test/simple_malloc-ut.sh index bc27895ed..35c9179a3 100755 --- a/test/simple_malloc-ut.sh +++ b/test/simple_malloc-ut.sh @@ -96,8 +96,8 @@ check "./ddprof ./test/simple_malloc ${opts}" 1 # Test wrapper mode with forks + threads check "./ddprof ./test/simple_malloc ${opts} --fork 2 --threads 2" 2 4 -# Test wrapper mode with forks + threads -# check "./ddprof --live_allocations yes ./test/simple_malloc ${opts} --fork 2 --threads 2 --skip-free 100" 2 4 +# Test leak mode with forks + threads +# check "./ddprof --preset cpu_live_heap ./test/simple_malloc ${opts} --fork 2 --threads 2 --skip-free 100" 2 4 # Test slow profiler startup check "env DD_PROFILING_NATIVE_STARTUP_WAIT_MS=200 ./ddprof ./test/simple_malloc ${opts}" 1 From b92e10778b3bf5b512365fdd65d258d890feed29 Mon Sep 17 00:00:00 2001 From: r1viollet Date: Tue, 7 Mar 2023 11:21:42 +0100 Subject: [PATCH 08/21] Extract some of the logics reading tracepoint configuration --- CMakeLists.txt | 1 + include/ddprof_cmdline.hpp | 2 - include/tracepoint_config.hpp | 15 +++++ src/ddprof_cmdline.cc | 109 +--------------------------------- src/pevent_lib.cc | 3 +- src/tracepoint_config.cc | 45 ++++++++++++++ test/CMakeLists.txt | 13 +++- test/tracepoint_config-ut.cc | 20 +++++++ 8 files changed, 96 insertions(+), 112 deletions(-) create mode 100644 include/tracepoint_config.hpp create mode 100644 src/tracepoint_config.cc create mode 100644 test/tracepoint_config-ut.cc diff --git a/CMakeLists.txt b/CMakeLists.txt index a4a008e3d..36008c160 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -215,6 +215,7 @@ set(DD_PROFILING_SOURCES src/ringbuffer_utils.cc src/signal_helper.cc src/sys_utils.cc + src/tracepoint_config.cc src/user_override.cc) if(BUILD_UNIVERSAL_DDPROF) diff --git a/include/ddprof_cmdline.hpp b/include/ddprof_cmdline.hpp index 8dc6b6e4c..aad07b003 100644 --- a/include/ddprof_cmdline.hpp +++ b/include/ddprof_cmdline.hpp @@ -27,6 +27,4 @@ bool arg_inset(const char *str, char const *const *set, int sz_set); bool arg_yesno(const char *str, int mode); -long id_from_tracepoint(const char *gname, const char *tname); - bool watcher_from_str(const char *str, PerfWatcher *watcher); diff --git a/include/tracepoint_config.hpp b/include/tracepoint_config.hpp new file mode 100644 index 000000000..87fbaa53a --- /dev/null +++ b/include/tracepoint_config.hpp @@ -0,0 +1,15 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. This product includes software +// developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present +// Datadog, Inc. + +#pragma once + +#include +#include + +namespace ddprof { +// Returns the ID of the given Linux tracepoint, or -1 if an error occurs. +int64_t tracepoint_get_id(std::string_view global_name, + std::string_view tracepoint_name); +} // namespace ddprof diff --git a/src/ddprof_cmdline.cc b/src/ddprof_cmdline.cc index ca7824fce..802555a5a 100644 --- a/src/ddprof_cmdline.cc +++ b/src/ddprof_cmdline.cc @@ -7,19 +7,10 @@ #include #include -#include -#include -#include -#include -#include -#include -#include -#include "ddres_helpers.hpp" #include "event_config.hpp" -#include "event_parser.h" -#include "perf_archmap.hpp" #include "perf_watcher.hpp" +#include "tracepoint_config.hpp" int arg_which(const char *str, char const *const *set, int sz_set) { if (!str || !set) @@ -48,99 +39,6 @@ bool arg_yesno(const char *str, int mode) { return false; } -long id_from_tracepoint(const char *gname, const char *tname) { - char path[2048] = {0}; // somewhat arbitrarily - size_t sz_path = sizeof(path); - char buf[64] = {0}; - char *buf_copy = buf; - - // Need to figure out whether we use debugfs or tracefs - static int use_tracefs = -1; // -2 error, -1 init, 0 no, 1 yes - static char tracefs_path[] = "/sys/kernel/tracing/events"; - static char debugfs_path[] = "/sys/kernel/debug/tracing/events"; - - if (!gname || !*gname || !tname || !*tname) { - return -1; - } - - if (use_tracefs == -2) { - // We checked in a previous loop and couldn't read tracef or debugfs - return -1; - } else if (use_tracefs == -1) { - struct stat sb; - if (stat(tracefs_path, &sb)) { - // If we're here, the stat failed so we can't use tracefs - if (stat(debugfs_path, &sb)) { - // If we're here, debugfs failed too, return error - use_tracefs = -2; - return -1; - } - use_tracefs = 0; // Use debugfs - } else { - use_tracefs = 1; // Use tracefs - } - } - - // Check validity of given tracepoint - char *spath = use_tracefs ? tracefs_path : debugfs_path; - int pathsz = snprintf(path, sz_path, "%s/%s/%s/id", spath, gname, tname); - if (static_cast(pathsz) >= sz_path) { - // Possibly ran out of room - return -1; - } - int fd = open(path, O_RDONLY); - if (-1 == fd) { - return -1; - } - - // Read the data in an eintr-safe way - int read_ret = -1; - long trace_id = -1; - do { - read_ret = read(fd, buf, sizeof(buf)); - } while (read_ret == -1 && errno == EINTR); - close(fd); - if (read_ret > 0) - trace_id = strtol(buf, &buf_copy, 10); - if (*buf_copy && *buf_copy != '\n') { - return -1; - } - - return trace_id; -} - -unsigned int tracepoint_id_from_event(const char *eventname, - const char *groupname) { - if (!eventname || !*eventname || !groupname || !*groupname) - return 0; - - static char path[4096]; // Arbitrary, but path sizes limits are difficult - static char buf[sizeof("4294967296")]; // For reading 32-bit decimal int - char *buf_copy = buf; - size_t pathsz = - snprintf(path, sizeof(path), "/sys/kernel/tracing/events/%s/%s/id", - groupname, eventname); - if (pathsz >= sizeof(path)) - return 0; - int fd = open(path, O_RDONLY); - if (-1 == fd) - return 0; - - // Read the data in an eintr-safe way - int read_ret = -1; - long trace_id = 0; - do { - read_ret = read(fd, buf, sizeof(buf)); - } while (read_ret == -1 && errno == EINTR); - close(fd); - if (read_ret > 0) - trace_id = strtol(buf, &buf_copy, 10); - if (*buf_copy && *buf_copy != '\n') - return 0; - - return trace_id; -} - // If this returns false, then the passed watcher should be regarded as invalid constexpr uint64_t kIgnoredWatcherID = -1ul; bool watcher_from_str(const char *str, PerfWatcher *watcher) { @@ -182,8 +80,7 @@ bool watcher_from_str(const char *str, PerfWatcher *watcher) { if (conf->id > 0) { tracepoint_id = conf->id; } else { - tracepoint_id = tracepoint_id_from_event(conf->eventname.c_str(), - conf->groupname.c_str()); + tracepoint_id = ddprof::tracepoint_get_id(conf->eventname, conf->groupname); } // 0 is an error, "-1" is ignored @@ -251,7 +148,7 @@ bool watcher_from_str(const char *str, PerfWatcher *watcher) { // tALLOCSYS2 captures all syscalls; used to troubleshoot 1 watcher->tracepoint_group = "raw_syscalls"; watcher->tracepoint_label = "sys_exit"; - long id = id_from_tracepoint("raw_syscalls", "sys_exit"); + long id = ddprof::tracepoint_get_id("raw_syscalls", "sys_exit"); if (-1 == id) { // We mutated the user's event, but it is invalid. return false; diff --git a/src/pevent_lib.cc b/src/pevent_lib.cc index 184984c30..bd48df60f 100644 --- a/src/pevent_lib.cc +++ b/src/pevent_lib.cc @@ -12,6 +12,7 @@ #include "ringbuffer_utils.hpp" #include "sys_utils.hpp" #include "syscalls.hpp" +#include "tracepoint_config.hpp" #include "user_override.hpp" #include @@ -84,7 +85,7 @@ static DDRes tallocsys1_open(PerfWatcher *watcher, int watcher_idx, pid_t pid, // Set the IDs for (auto &kprobe : kprobes) { - long id = id_from_tracepoint("syscalls", kprobe.first.c_str()); + long id = ddprof::tracepoint_get_id("syscalls", kprobe.first); if (-1 == id) { DDRES_RETURN_ERROR_LOG(DD_WHAT_PERFOPEN, "Error opening tracefs for tALLOCSYS1 on %s", diff --git a/src/tracepoint_config.cc b/src/tracepoint_config.cc new file mode 100644 index 000000000..209c9bb2d --- /dev/null +++ b/src/tracepoint_config.cc @@ -0,0 +1,45 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. This product includes software +// developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present +// Datadog, Inc. + +#include +#include +#include +#include + +namespace ddprof { +// Returns the ID of the given Linux tracepoint, or -1 if an error occurs. +int64_t tracepoint_get_id(std::string_view global_name, + std::string_view tracepoint_name) { + if (global_name.empty() || tracepoint_name.empty()) { + return -1; + } + + // todo: should we even consider the debug path ? (is it not deprecated?) + std::string fs_path; + struct stat sb; + if (stat("/sys/kernel/tracing/events", &sb) == 0) { + fs_path = "/sys/kernel/tracing/events"; + } else if (stat("/sys/kernel/debug/tracing/events", &sb) == 0) { + fs_path = "/sys/kernel/debug/tracing/events"; + } else { + return -1; // Neither debugfs nor tracefs is available. + } + + // todo this path could change (from user overrides) + std::stringstream id_path; + id_path << fs_path << "/" << global_name << "/" << tracepoint_name << "/id"; + + // Read the ID from the file. + std::ifstream id_file(id_path.str()); + if (!id_file) { + return -1; + } + long trace_id; + if (!(id_file >> trace_id)) { + return -1; + } + return trace_id; +} +} // namespace ddprof diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 07011474c..4d96e76ee 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -87,8 +87,8 @@ endfunction() # Definition of unit tests add_compile_definitions("UNIT_TEST_DATA=\"${CMAKE_CURRENT_SOURCE_DIR}/data\"") -add_unit_test(ddprofcmdline-ut ../src/ddprof_cmdline.cc ../src/perf_watcher.cc ddprofcmdline-ut.cc - LIBRARIES DDProf::Parser) +add_unit_test(ddprofcmdline-ut ../src/ddprof_cmdline.cc ../src/perf_watcher.cc + ../src/tracepoint_config.cc ddprofcmdline-ut.cc LIBRARIES DDProf::Parser) add_unit_test(logger-ut logger-ut.cc) @@ -131,16 +131,18 @@ add_unit_test( ../src/ddprof_cpumask.cc ../src/logger_setup.cc ../src/perf_watcher.cc + ../src/tracepoint_config.cc ddprof_input-ut.cc LIBRARIES DDProf::Parser DEFINITIONS MYNAME="ddprof_input-ut") -add_unit_test(perf_ringbuffer-ut ../src/perf.cc ../src/perf_watcher.cc ../src/perf_ringbuffer.cc +add_unit_test(perf_ringbuffer-ut ../src/perf.cc ../src/perf_watcher.cc ../src/perf_ringbuffer.cc ../src/tracepoint_config.cc perf_ringbuffer-ut.cc DEFINITIONS MYNAME="perf_ringbuffer-ut") add_unit_test( pevent-ut ../src/ddprof_cmdline.cc + ../src/tracepoint_config.cc ../src/pevent_lib.cc ../src/user_override.cc ../src/perf.cc @@ -162,6 +164,7 @@ add_unit_test( ddprof_exporter-ut ../src/exporter/ddprof_exporter.cc ../src/ddprof_cmdline.cc + ../src/tracepoint_config.cc ../src/pprof/ddprof_pprof.cc ../src/perf_watcher.cc ../src/tags.cc @@ -281,6 +284,7 @@ add_unit_test( ../src/signal_helper.cc ../src/statsd.cc ../src/sys_utils.cc + ../src/tracepoint_config.cc ../src/user_override.cc ../src/unwind.cc ../src/unwind_dwfl.cc @@ -295,6 +299,7 @@ add_unit_test( ringbuffer-ut ringbuffer-ut.cc ../src/ddprof_cmdline.cc + ../src/tracepoint_config.cc ../src/perf.cc ../src/perf_ringbuffer.cc ../src/perf_watcher.cc @@ -319,6 +324,8 @@ add_unit_test(build_id-ut build_id-ut.cc ../src/build_id.cc) add_unit_test(jitdump-ut jitdump-ut.cc ../src/jit/jitdump.cc) +add_unit_test(tracepoint_config-ut tracepoint_config-ut.cc ../src/tracepoint_config.cc) + add_benchmark(savecontext-bench savecontext-bench.cc ../src/lib/savecontext.cc ../src/lib/saveregisters.cc) diff --git a/test/tracepoint_config-ut.cc b/test/tracepoint_config-ut.cc new file mode 100644 index 000000000..074fb391c --- /dev/null +++ b/test/tracepoint_config-ut.cc @@ -0,0 +1,20 @@ +#include + +#include "loghandle.hpp" +#include "tracepoint_config.hpp" + +namespace ddprof { + +TEST(tracepoint_config, getid) { + LogHandle handle; + int64_t id = tracepoint_get_id("raw_syscalls", "sys_exit"); + // This can fail without the appropriate permissions + LG_DBG("Tracepoint: raw_syscall/sys_exit id=%ld ", id); +#if defined(__x86_64__) + if (id != -1) { + EXPECT_EQ(id, 348); + } +#endif +} + +} // namespace ddprof From de78185151acf635b36d12441fdf6837e65a49aa Mon Sep 17 00:00:00 2001 From: r1viollet Date: Wed, 22 Mar 2023 10:14:04 +0100 Subject: [PATCH 09/21] Minor merge fix Removal of old wrapper functions --- include/live_allocation.hpp | 5 + src/lib/malloc_wrapper.cc | 222 ------------------------------------ test/CMakeLists.txt | 5 +- 3 files changed, 8 insertions(+), 224 deletions(-) delete mode 100644 src/lib/malloc_wrapper.cc diff --git a/include/live_allocation.hpp b/include/live_allocation.hpp index 821545c27..4a5ffcdb6 100644 --- a/include/live_allocation.hpp +++ b/include/live_allocation.hpp @@ -1,3 +1,8 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. This product includes software +// developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present +// Datadog, Inc. + #pragma once #include "ddprof_defs.hpp" diff --git a/src/lib/malloc_wrapper.cc b/src/lib/malloc_wrapper.cc deleted file mode 100644 index e1797106b..000000000 --- a/src/lib/malloc_wrapper.cc +++ /dev/null @@ -1,222 +0,0 @@ -// Unless explicitly stated otherwise all files in this repository are licensed -// under the Apache License Version 2.0. This product includes software -// developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present -// Datadog, Inc. - -#include "allocation_tracker.hpp" -#include "ddprof_base.hpp" -#include "unlikely.hpp" - -#include -#include -#include -#include -#include -#include - -// Declaration of reallocarray is only available starting from glibc 2.28 -extern "C" { -#ifdef __llvm__ -void *reallocarray(void *ptr, size_t nmemb, size_t size) noexcept; -#else -void *reallocarray(void *ptr, size_t nmemb, size_t size); -#endif -void *pvalloc(size_t size) noexcept; - -void *temp_malloc(size_t size) noexcept; -void temp_free(void *ptr) noexcept; -void *temp_calloc(size_t nmemb, size_t size) noexcept; -void *temp_realloc(void *ptr, size_t size) noexcept; -int temp_posix_memalign(void **memptr, size_t alignment, size_t size) noexcept; -void *temp_aligned_alloc(size_t alignment, size_t size) noexcept; -void *temp_memalign(size_t alignment, size_t size) noexcept; -void *temp_pvalloc(size_t size) noexcept; -void *temp_valloc(size_t size) noexcept; -void *temp_reallocarray(void *ptr, size_t nmemb, size_t size) noexcept; -} - -#define ORIGINAL_FUNC(name) get_next(#name) -#define DECLARE_FUNC(name) decltype(&::name) s_##name = &temp_##name; - -template F get_next(const char *name) { - auto *func = reinterpret_cast(dlsym(RTLD_NEXT, name)); - return func; -} - -DECLARE_FUNC(malloc); -DECLARE_FUNC(calloc); -DECLARE_FUNC(realloc); -DECLARE_FUNC(free); -DECLARE_FUNC(posix_memalign); -DECLARE_FUNC(aligned_alloc); -DECLARE_FUNC(reallocarray); -// obsolete allocation functions -DECLARE_FUNC(memalign); -DECLARE_FUNC(pvalloc); -DECLARE_FUNC(valloc); - -namespace { -DDPROF_NOINLINE void init(); - -// calloc is invoked by dlsym, returning a null value in this case is well -// handled by glibc -void *temp_calloc2(size_t, size_t) noexcept { return nullptr; } - -inline DDPROF_NO_SANITIZER_ADDRESS void check_init() { - [[maybe_unused]] static bool init_once = []() { - init(); - return true; - }(); -} - -void init() { - s_calloc = &temp_calloc2; - - s_calloc = ORIGINAL_FUNC(calloc); - s_malloc = ORIGINAL_FUNC(malloc); - s_free = ORIGINAL_FUNC(free); - s_realloc = ORIGINAL_FUNC(realloc); - s_posix_memalign = ORIGINAL_FUNC(posix_memalign); - s_aligned_alloc = ORIGINAL_FUNC(aligned_alloc); - s_memalign = ORIGINAL_FUNC(memalign); - s_pvalloc = ORIGINAL_FUNC(pvalloc); - s_valloc = ORIGINAL_FUNC(valloc); - s_reallocarray = ORIGINAL_FUNC(reallocarray); -} - -} // namespace - -void *malloc(size_t size) { - void *ptr = s_malloc(size); - ddprof::AllocationTracker::track_allocation(reinterpret_cast(ptr), - size); - return ptr; -} - -void *temp_malloc(size_t size) noexcept { - check_init(); - return s_malloc(size); -} - -void free(void *ptr) { - if (ptr == nullptr) { - return; - } - ddprof::AllocationTracker::track_deallocation( - reinterpret_cast(ptr)); - s_free(ptr); -} - -void temp_free(void *ptr) noexcept { - check_init(); - return s_free(ptr); -} - -void *calloc(size_t nmemb, size_t size) { - void *ptr = s_calloc(nmemb, size); - ddprof::AllocationTracker::track_allocation(reinterpret_cast(ptr), - size * nmemb); - return ptr; -} - -void *temp_calloc(size_t nmemb, size_t size) noexcept { - check_init(); - return s_calloc(nmemb, size); -} - -void *realloc(void *ptr, size_t size) { - if (ptr) { - ddprof::AllocationTracker::track_deallocation( - reinterpret_cast(ptr)); - } - void *newptr = s_realloc(ptr, size); - ddprof::AllocationTracker::track_allocation( - reinterpret_cast(newptr), size); - return newptr; -} - -void *temp_realloc(void *ptr, size_t size) noexcept { - check_init(); - return s_realloc(ptr, size); -} - -int posix_memalign(void **memptr, size_t alignment, size_t size) { - int ret = s_posix_memalign(memptr, alignment, size); - if (likely(!ret)) { - ddprof::AllocationTracker::track_allocation( - reinterpret_cast(*memptr), size); - } - return ret; -} - -int temp_posix_memalign(void **memptr, size_t alignment, size_t size) noexcept { - check_init(); - return s_posix_memalign(memptr, alignment, size); -} - -void *aligned_alloc(size_t alignment, size_t size) { - void *ptr = s_aligned_alloc(alignment, size); - ddprof::AllocationTracker::track_allocation(reinterpret_cast(ptr), - size); - return ptr; -} - -void *temp_aligned_alloc(size_t alignment, size_t size) noexcept { - check_init(); - return s_aligned_alloc(alignment, size); -} - -void *memalign(size_t alignment, size_t size) { - void *ptr = s_memalign(alignment, size); - ddprof::AllocationTracker::track_allocation(reinterpret_cast(ptr), - size); - return ptr; -} -void *temp_memalign(size_t alignment, size_t size) noexcept { - check_init(); - return s_memalign(alignment, size); -} - -void *pvalloc(size_t size) noexcept { - void *ptr = s_pvalloc(size); - ddprof::AllocationTracker::track_allocation(reinterpret_cast(ptr), - size); - return ptr; -} - -void *temp_pvalloc(size_t size) noexcept { - check_init(); - return s_pvalloc(size); -} - -void *valloc(size_t size) { - void *ptr = s_valloc(size); - ddprof::AllocationTracker::track_allocation(reinterpret_cast(ptr), - size); - return ptr; -} - -void *temp_valloc(size_t size) noexcept { - check_init(); - return s_valloc(size); -} - -#ifdef __llvm__ -void *reallocarray(void *ptr, size_t nmemb, size_t size) noexcept { -#else -void *reallocarray(void *ptr, size_t nmemb, size_t size) { -#endif - if (ptr) { - ddprof::AllocationTracker::track_deallocation( - reinterpret_cast(ptr)); - } - void *newptr = s_reallocarray(ptr, nmemb, size); - ddprof::AllocationTracker::track_allocation( - reinterpret_cast(newptr), size * nmemb); - return newptr; -} - -void *temp_reallocarray(void *ptr, size_t nmemb, size_t size) noexcept { - check_init(); - return s_reallocarray(ptr, nmemb, size); -} diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 4d96e76ee..c6bcfd482 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -136,8 +136,9 @@ add_unit_test( LIBRARIES DDProf::Parser DEFINITIONS MYNAME="ddprof_input-ut") -add_unit_test(perf_ringbuffer-ut ../src/perf.cc ../src/perf_watcher.cc ../src/perf_ringbuffer.cc ../src/tracepoint_config.cc - perf_ringbuffer-ut.cc DEFINITIONS MYNAME="perf_ringbuffer-ut") +add_unit_test( + perf_ringbuffer-ut ../src/perf.cc ../src/perf_watcher.cc ../src/perf_ringbuffer.cc + ../src/tracepoint_config.cc perf_ringbuffer-ut.cc DEFINITIONS MYNAME="perf_ringbuffer-ut") add_unit_test( pevent-ut From 4feed8c621a2c3c22861f940d6faab549dbf3db8 Mon Sep 17 00:00:00 2001 From: r1viollet Date: Wed, 22 Mar 2023 21:28:27 +0100 Subject: [PATCH 10/21] Minor refactoring for Watcher Configurations Removal of "<=" operators for event configuration. --- include/event_config.hpp | 7 +++- src/ddprof_cmdline.cc | 36 ++++++++++---------- src/ddprof_worker.cc | 3 +- test/ddprofcmdline-ut.cc | 72 +++++++++++++++++++++++++++++----------- 4 files changed, 79 insertions(+), 39 deletions(-) diff --git a/include/event_config.hpp b/include/event_config.hpp index 9690ee378..f7f1c4a46 100644 --- a/include/event_config.hpp +++ b/include/event_config.hpp @@ -17,6 +17,11 @@ enum class EventConfMode : uint32_t { kAll = kCallgraph | kMetric | kLiveCallgraph, }; +bool operator<=(EventConfMode A, EventConfMode B) = delete; +bool operator<(EventConfMode A, EventConfMode B) = delete; +bool operator>(EventConfMode A, EventConfMode B) = delete; +bool operator>=(EventConfMode A, EventConfMode B) = delete; + constexpr EventConfMode operator|(EventConfMode A, const EventConfMode B) { return static_cast(static_cast(A) | static_cast(B)); @@ -140,7 +145,7 @@ enum class EventConfField { struct EventConf { EventConfMode mode; - uint64_t id; + int64_t id; std::string eventname; std::string groupname; diff --git a/src/ddprof_cmdline.cc b/src/ddprof_cmdline.cc index 802555a5a..d616d3a82 100644 --- a/src/ddprof_cmdline.cc +++ b/src/ddprof_cmdline.cc @@ -9,6 +9,7 @@ #include #include "event_config.hpp" +#include "logger.hpp" #include "perf_watcher.hpp" #include "tracepoint_config.hpp" @@ -40,7 +41,7 @@ bool arg_yesno(const char *str, int mode) { } // If this returns false, then the passed watcher should be regarded as invalid -constexpr uint64_t kIgnoredWatcherID = -1ul; +constexpr int64_t kIgnoredWatcherID = -1l; bool watcher_from_str(const char *str, PerfWatcher *watcher) { EventConf *conf = EventConf_parse(str); if (!conf) { @@ -71,22 +72,23 @@ bool watcher_from_str(const char *str, PerfWatcher *watcher) { return false; } - // The most likely thing to be invalid is the selection of the tracepoint - // from the trace events system. If the conf has a nonzero number for the id - // we assume the user has privileged information and knows what they want. - // Else, we use the group/event combination to extract that id from the - // tracefs filesystem in the canonical way. - uint64_t tracepoint_id = 0; - if (conf->id > 0) { - tracepoint_id = conf->id; - } else { - tracepoint_id = ddprof::tracepoint_get_id(conf->eventname, conf->groupname); - } - - // 0 is an error, "-1" is ignored - if (!tracepoint_id) { - return false; - } else if (tracepoint_id != kIgnoredWatcherID) { + if (conf->id != kIgnoredWatcherID) { + // The most likely thing to be invalid is the selection of the tracepoint + // from the trace events system. If the conf has a nonzero number for the + // id we assume the user has privileged information and knows what they + // want. Else, we use the group/event combination to extract that id from + // the tracefs filesystem in the canonical way. + int64_t tracepoint_id = 0; + if (conf->id > 0) { + tracepoint_id = conf->id; + } else { + tracepoint_id = + ddprof::tracepoint_get_id(conf->eventname, conf->groupname); + } + // At this point we needed to find a valid tracepoint id + if (tracepoint_id == kIgnoredWatcherID) { + return false; + } watcher->config = tracepoint_id; } diff --git a/src/ddprof_worker.cc b/src/ddprof_worker.cc index 6ecf6789b..ce9e82bbe 100644 --- a/src/ddprof_worker.cc +++ b/src/ddprof_worker.cc @@ -295,7 +295,8 @@ DDRes ddprof_pr_sample(DDProfContext *ctx, perf_event_sample *sample, PerfWatcher *watcher = &ctx->watchers[watcher_pos]; // Aggregate if unwinding went well (todo : fatal error propagation) - if (!IsDDResFatal(res) && EventConfMode::kCallgraph <= watcher->output_mode) { + if (!IsDDResFatal(res) && + Any(EventConfMode::kCallgraph & watcher->output_mode)) { struct UnwindState *us = ctx->worker_ctx.us; #ifndef DDPROF_NATIVE_LIB // Depending on the type of watcher, compute a value for sample diff --git a/test/ddprofcmdline-ut.cc b/test/ddprofcmdline-ut.cc index 1b7ae4b40..44e2afb13 100644 --- a/test/ddprofcmdline-ut.cc +++ b/test/ddprofcmdline-ut.cc @@ -157,43 +157,75 @@ TEST(CmdLineTst, ParserKeyPatterns) { // Mode is permissive ASSERT_TRUE(watcher_from_str("e=hCPU mode=magnanimous", &watcher)); - ASSERT_TRUE(watcher.output_mode <= EventConfMode::kCallgraph); - ASSERT_TRUE(watcher.output_mode <= EventConfMode::kMetric); + ASSERT_TRUE(Any(watcher.output_mode & EventConfMode::kCallgraph)); + ASSERT_TRUE(Any(watcher.output_mode & EventConfMode::kMetric)); // A or a designate all ASSERT_TRUE(watcher_from_str("e=hCPU mode=A", &watcher)); - ASSERT_TRUE(watcher.output_mode <= EventConfMode::kCallgraph); - ASSERT_TRUE(watcher.output_mode <= EventConfMode::kMetric); + ASSERT_TRUE(Any(watcher.output_mode & EventConfMode::kCallgraph)); + ASSERT_TRUE(Any(watcher.output_mode & EventConfMode::kMetric)); ASSERT_TRUE(watcher_from_str("e=hCPU mode=a", &watcher)); - ASSERT_TRUE(watcher.output_mode <= EventConfMode::kCallgraph); - ASSERT_TRUE(watcher.output_mode <= EventConfMode::kMetric); + ASSERT_TRUE(Any(watcher.output_mode & + EventConfMode::kCallgraph)); // watcher.output_mode <= + // EventConfMode::kCallgraph + ASSERT_TRUE(Any( + watcher.output_mode & + EventConfMode::kMetric)); // watcher.output_mode <= EventConfMode::kMetric // both m and g together designate all ASSERT_TRUE(watcher_from_str("e=hCPU mode=MG", &watcher)); - ASSERT_TRUE(watcher.output_mode <= EventConfMode::kCallgraph); - ASSERT_TRUE(watcher.output_mode <= EventConfMode::kMetric); + ASSERT_TRUE(Any(watcher.output_mode & + EventConfMode::kCallgraph)); // watcher.output_mode <= + // EventConfMode::kCallgraph + ASSERT_TRUE(Any( + watcher.output_mode & + EventConfMode::kMetric)); // watcher.output_mode <= EventConfMode::kMetric ASSERT_TRUE(watcher_from_str("e=hCPU mode=mg", &watcher)); - ASSERT_TRUE(watcher.output_mode <= EventConfMode::kCallgraph); - ASSERT_TRUE(watcher.output_mode <= EventConfMode::kMetric); + ASSERT_TRUE(Any(watcher.output_mode & + EventConfMode::kCallgraph)); // watcher.output_mode <= + // EventConfMode::kCallgraph + ASSERT_TRUE(Any( + watcher.output_mode & + EventConfMode::kMetric)); // watcher.output_mode <= EventConfMode::kMetric // M or m is a metric (no callgraph unless specified) ASSERT_TRUE(watcher_from_str("e=hCPU mode=M", &watcher)); - ASSERT_FALSE(watcher.output_mode <= EventConfMode::kCallgraph); - ASSERT_TRUE(watcher.output_mode <= EventConfMode::kMetric); + ASSERT_FALSE(Any(watcher.output_mode & + EventConfMode::kCallgraph)); // watcher.output_mode <= + // EventConfMode::kCallgraph + ASSERT_TRUE(Any( + watcher.output_mode & + EventConfMode::kMetric)); // watcher.output_mode <= EventConfMode::kMetric ASSERT_TRUE(watcher_from_str("e=hCPU mode=m", &watcher)); - ASSERT_FALSE(watcher.output_mode <= EventConfMode::kCallgraph); - ASSERT_TRUE(watcher.output_mode <= EventConfMode::kMetric); + ASSERT_FALSE(Any(watcher.output_mode & + EventConfMode::kCallgraph)); // watcher.output_mode <= + // EventConfMode::kCallgraph + ASSERT_TRUE(Any( + watcher.output_mode & + EventConfMode::kMetric)); // watcher.output_mode <= EventConfMode::kMetric // G or g designate callgraph (default) ASSERT_TRUE(watcher_from_str("e=hCPU", &watcher)); - ASSERT_TRUE(watcher.output_mode <= EventConfMode::kCallgraph); - ASSERT_FALSE(watcher.output_mode <= EventConfMode::kMetric); + ASSERT_TRUE(Any(watcher.output_mode & + EventConfMode::kCallgraph)); // watcher.output_mode <= + // EventConfMode::kCallgraph + ASSERT_FALSE(Any( + watcher.output_mode & + EventConfMode::kMetric)); // watcher.output_mode <= EventConfMode::kMetric ASSERT_TRUE(watcher_from_str("e=hCPU mode=G", &watcher)); - ASSERT_TRUE(watcher.output_mode <= EventConfMode::kCallgraph); - ASSERT_FALSE(watcher.output_mode <= EventConfMode::kMetric); + ASSERT_TRUE(Any(watcher.output_mode & + EventConfMode::kCallgraph)); // watcher.output_mode <= + // EventConfMode::kCallgraph + ASSERT_FALSE(Any( + watcher.output_mode & + EventConfMode::kMetric)); // watcher.output_mode <= EventConfMode::kMetric ASSERT_TRUE(watcher_from_str("e=hCPU mode=g", &watcher)); - ASSERT_TRUE(watcher.output_mode <= EventConfMode::kCallgraph); - ASSERT_FALSE(watcher.output_mode <= EventConfMode::kMetric); + ASSERT_TRUE(Any(watcher.output_mode & + EventConfMode::kCallgraph)); // watcher.output_mode <= + // EventConfMode::kCallgraph + ASSERT_FALSE(Any( + watcher.output_mode & + EventConfMode::kMetric)); // watcher.output_mode <= EventConfMode::kMetric // n|arg_num|argno ASSERT_TRUE(watcher_from_str("e=hCPU n=1", &watcher)); From ab05d2aafd710de8ae967db79d530f65b4e8aa39 Mon Sep 17 00:00:00 2001 From: r1viollet Date: Wed, 22 Mar 2023 22:12:15 +0100 Subject: [PATCH 11/21] Minor rebase fix Replace usage of nb_locs with vector size operator --- src/unwind_dwfl.cc | 8 +++++--- test/CMakeLists.txt | 3 +-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/unwind_dwfl.cc b/src/unwind_dwfl.cc index 8e27283ae..12a61c852 100644 --- a/src/unwind_dwfl.cc +++ b/src/unwind_dwfl.cc @@ -77,7 +77,8 @@ static void trace_unwinding_end(UnwindState *us) { if (LL_DEBUG <= LOG_getlevel()) { DsoHdr::DsoFindRes find_res = us->dso_hdr.dso_find_closest(us->pid, us->current_ip); - SymbolIdx_t symIdx = us->output.locs[us->output.nb_locs - 1]._symbol_idx; + SymbolIdx_t symIdx = + us->output.locs[us->output.locs.size() - 1]._symbol_idx; if (find_res.second) { const std::string &last_func = us->symbol_hdr._symbol_table[symIdx]._symname; @@ -222,8 +223,9 @@ static int frame_cb(Dwfl_Frame *dwfl_frame, void *arg) { #ifdef DEBUG // We often fallback to frame pointer unwinding (which logs an error) if (dwfl_error_value) { - LG_DBG("Error flagged at depth = %lu -- %d Error:%s ", us->output.locs.size(), - dwfl_error_value, dwfl_errmsg(dwfl_error_value)); + LG_DBG("Error flagged at depth = %lu -- %d Error:%s ", + us->output.locs.size(), dwfl_error_value, + dwfl_errmsg(dwfl_error_value)); } #endif // Before we potentially exit, record the fact that we're processing a frame diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index c6bcfd482..9ee2e06e6 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -156,8 +156,7 @@ add_unit_test( DEFINITIONS MYNAME="pevent-ut") add_unit_test( - ddprof_pprof-ut ../src/pprof/ddprof_pprof.cc ../src/perf_watcher.cc - ddprof_pprof-ut.cc + ddprof_pprof-ut ../src/pprof/ddprof_pprof.cc ../src/perf_watcher.cc ddprof_pprof-ut.cc LIBRARIES Datadog::Profiling DDProf::Parser DEFINITIONS MYNAME="ddprof_pprof-ut") From 494bbfacfa013b9cc2860939b5ba8fc9a8931936 Mon Sep 17 00:00:00 2001 From: r1viollet Date: Thu, 23 Mar 2023 08:28:45 +0100 Subject: [PATCH 12/21] Re-introduce the memory leak test --- test/simple_malloc-ut.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/simple_malloc-ut.sh b/test/simple_malloc-ut.sh index 35c9179a3..512bd65dd 100755 --- a/test/simple_malloc-ut.sh +++ b/test/simple_malloc-ut.sh @@ -97,7 +97,7 @@ check "./ddprof ./test/simple_malloc ${opts}" 1 check "./ddprof ./test/simple_malloc ${opts} --fork 2 --threads 2" 2 4 # Test leak mode with forks + threads -# check "./ddprof --preset cpu_live_heap ./test/simple_malloc ${opts} --fork 2 --threads 2 --skip-free 100" 2 4 +check "./ddprof --preset cpu_live_heap ./test/simple_malloc ${opts} --fork 2 --threads 2 --skip-free 100" 2 4 # Test slow profiler startup check "env DD_PROFILING_NATIVE_STARTUP_WAIT_MS=200 ./ddprof ./test/simple_malloc ${opts}" 1 From 492e9c21c8270362d37176b20318f71a1fc82699 Mon Sep 17 00:00:00 2001 From: r1viollet Date: Thu, 23 Mar 2023 09:38:30 +0100 Subject: [PATCH 13/21] Revert "Re-introduce the memory leak test" This reverts commit bf47e8cd7939d8718075be388efdc93361ba6c36. --- test/simple_malloc-ut.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/simple_malloc-ut.sh b/test/simple_malloc-ut.sh index 512bd65dd..35c9179a3 100755 --- a/test/simple_malloc-ut.sh +++ b/test/simple_malloc-ut.sh @@ -97,7 +97,7 @@ check "./ddprof ./test/simple_malloc ${opts}" 1 check "./ddprof ./test/simple_malloc ${opts} --fork 2 --threads 2" 2 4 # Test leak mode with forks + threads -check "./ddprof --preset cpu_live_heap ./test/simple_malloc ${opts} --fork 2 --threads 2 --skip-free 100" 2 4 +# check "./ddprof --preset cpu_live_heap ./test/simple_malloc ${opts} --fork 2 --threads 2 --skip-free 100" 2 4 # Test slow profiler startup check "env DD_PROFILING_NATIVE_STARTUP_WAIT_MS=200 ./ddprof ./test/simple_malloc ${opts}" 1 From 0a09fc47dd89ef7e340e75d16cd7fabf5948a8e5 Mon Sep 17 00:00:00 2001 From: r1viollet Date: Tue, 4 Apr 2023 10:55:23 +0200 Subject: [PATCH 14/21] De-duplicate the live allocation workflow --- include/ddres_list.hpp | 1 + src/ddprof_worker.cc | 93 ++++++++++++++++------------------------ test/simple_malloc-ut.sh | 3 ++ 3 files changed, 41 insertions(+), 56 deletions(-) diff --git a/include/ddres_list.hpp b/include/ddres_list.hpp index 399808907..8488fc814 100644 --- a/include/ddres_list.hpp +++ b/include/ddres_list.hpp @@ -50,6 +50,7 @@ X(DSO, "") \ X(JIT, "Error parsing JIT files") \ X(NO_JIT_FILE, "File not readable for JIT") \ + X(UNHANDLED_CONFIG, "unhandled configuration") \ X(UNHANDLED_DSO, "ignore dso type") \ X(WORKERLOOP_INIT, "error initializing the worker loop") \ X(SYS_CONFIG, "error checking system configuration") \ diff --git a/src/ddprof_worker.cc b/src/ddprof_worker.cc index ce9e82bbe..71fcef01f 100644 --- a/src/ddprof_worker.cc +++ b/src/ddprof_worker.cc @@ -295,32 +295,46 @@ DDRes ddprof_pr_sample(DDProfContext *ctx, perf_event_sample *sample, PerfWatcher *watcher = &ctx->watchers[watcher_pos]; // Aggregate if unwinding went well (todo : fatal error propagation) - if (!IsDDResFatal(res) && - Any(EventConfMode::kCallgraph & watcher->output_mode)) { + if (!IsDDResFatal(res)) { struct UnwindState *us = ctx->worker_ctx.us; + if (Any(EventConfMode::kLiveCallgraph & watcher->output_mode)) { + // Live callgraph mode + if (watcher->ddprof_event_type == DDPROF_PWE_sALLOC) { + // for now we hard code the live aggregation mode + ctx->worker_ctx.live_allocation.register_allocation( + us->output, sample->addr, sample->period, watcher_pos, sample->pid); + } + // live callgraph not compatible with all watcher types + else { + DDRES_RETURN_ERROR_LOG(DD_WHAT_UNHANDLED_CONFIG, + "Live callgraph configuration unhandled"); + } + } else if (Any(EventConfMode::kCallgraph & watcher->output_mode)) { #ifndef DDPROF_NATIVE_LIB - // Depending on the type of watcher, compute a value for sample - uint64_t sample_val = perf_value_from_sample(watcher, sample); - - // in lib mode we don't aggregate (protect to avoid link failures) - int i_export = ctx->worker_ctx.i_current_pprof; - DDProfPProf *pprof = ctx->worker_ctx.pprof[i_export]; - DDRES_CHECK_FWD(pprof_aggregate(&us->output, &us->symbol_hdr, sample_val, 1, - watcher, pprof)); - if (ctx->params.show_samples) { - ddprof_print_sample(us->output, us->symbol_hdr, sample->period, *watcher); - } + // Depending on the type of watcher, compute a value for sample + uint64_t sample_val = perf_value_from_sample(watcher, sample); + + // in lib mode we don't aggregate (protect to avoid link failures) + int i_export = ctx->worker_ctx.i_current_pprof; + DDProfPProf *pprof = ctx->worker_ctx.pprof[i_export]; + DDRES_CHECK_FWD(pprof_aggregate(&us->output, &us->symbol_hdr, sample_val, + 1, watcher, pprof)); + if (ctx->params.show_samples) { + ddprof_print_sample(us->output, us->symbol_hdr, sample->period, + *watcher); + } #else - // Call the user's stack handler - if (ctx->stack_handler) { - if (!ctx->stack_handler->apply(&us->output, ctx, - ctx->stack_handler->callback_ctx, - watcher_pos)) { - DDRES_RETURN_ERROR_LOG(DD_WHAT_STACK_HANDLE, - "Stack handler returning errors"); + // Call the user's stack handler + if (ctx->stack_handler) { + if (!ctx->stack_handler->apply(&us->output, ctx, + ctx->stack_handler->callback_ctx, + watcher_pos)) { + DDRES_RETURN_ERROR_LOG(DD_WHAT_STACK_HANDLE, + "Stack handler returning errors"); + } } - } #endif + } } ddprof_stats_add(STATS_AGGREGATION_AVG_TIME, @@ -329,32 +343,6 @@ DDRes ddprof_pr_sample(DDProfContext *ctx, perf_event_sample *sample, return {}; } -DDRes ddprof_pr_allocation_tracking(DDProfContext *ctx, - perf_event_sample *sample, - int watcher_pos) { - if (!sample) - return ddres_warn(DD_WHAT_PERFSAMP); - - // If this is a SW_TASK_CLOCK-type event, then aggregate the time - if (ctx->watchers[watcher_pos].config == PERF_COUNT_SW_TASK_CLOCK) - ddprof_stats_add(STATS_TARGET_CPU_USAGE, sample->period, NULL); - - auto ticks0 = ddprof::get_tsc_cycles(); - DDRes res = ddprof_unwind_sample(ctx, sample, watcher_pos); - auto unwind_ticks = ddprof::get_tsc_cycles(); - ddprof_stats_add(STATS_UNWIND_AVG_TIME, unwind_ticks - ticks0, NULL); - - // Aggregate if unwinding went well (todo : fatal error propagation) - if (!IsDDResFatal(res)) { - struct UnwindState *us = ctx->worker_ctx.us; - ctx->worker_ctx.live_allocation.register_allocation( - us->output, sample->addr, sample->period, watcher_pos, sample->pid); - } - - // TODO: propagate fatal - return ddres_init(); -} - DDRes ddprof_pr_sysallocation_tracking(DDProfContext *ctx, perf_event_sample *sample, int watcher_pos) { @@ -787,21 +775,14 @@ DDRes ddprof_worker_process_event(const perf_event_header *hdr, int watcher_pos, uint64_t mask = watcher->sample_type; perf_event_sample *sample = hdr2samp(hdr, mask); - // Various checks for allocation profiling - // - sALLOC - // - mmap/munmap syscalls - bool is_allocation = watcher->type == kDDPROF_TYPE_CUSTOM && - watcher->config == kDDPROF_COUNT_ALLOCATIONS; if (sample) { // Handle special profiling types first if (watcher->ddprof_event_type == DDPROF_PWE_tALLOCSYS1 || watcher->ddprof_event_type == DDPROF_PWE_tALLOCSYS2) { + // For now we have a different path for + // - mmap/munmap syscalls DDRES_CHECK_FWD( ddprof_pr_sysallocation_tracking(ctx, sample, watcher_pos)); - } else if (is_allocation && - watcher->output_mode == EventConfMode::kLiveCallgraph) { - DDRES_CHECK_FWD( - ddprof_pr_allocation_tracking(ctx, sample, watcher_pos)); } else { DDRES_CHECK_FWD(ddprof_pr_sample(ctx, sample, watcher_pos)); } diff --git a/test/simple_malloc-ut.sh b/test/simple_malloc-ut.sh index 35c9179a3..713d290f8 100755 --- a/test/simple_malloc-ut.sh +++ b/test/simple_malloc-ut.sh @@ -93,6 +93,9 @@ check "./test/simple_malloc-shared --profile ${opts}" 1 # Test wrapper mode check "./ddprof ./test/simple_malloc ${opts}" 1 +# Test live heap mode +check "./ddprof --preset cpu_live_heap ./test/simple_malloc ${opts} --skip-free 100" 1 + # Test wrapper mode with forks + threads check "./ddprof ./test/simple_malloc ${opts} --fork 2 --threads 2" 2 4 From 46f8f0fcfc1e32d36b741ebf3972a1542ba0a03c Mon Sep 17 00:00:00 2001 From: r1viollet <74836499+r1viollet@users.noreply.github.com> Date: Thu, 6 Apr 2023 10:19:35 +0200 Subject: [PATCH 15/21] Clear memory tracking state beyond a fixed bound (#239) * Clear memory tracking state beyond a fixed bound --- include/ddprof_context_lib.hpp | 2 - include/ddprof_perf_event.hpp | 14 +++++- include/lib/allocation_tracker.hpp | 18 +++++--- include/live_allocation-c.hpp | 17 +++++++ include/live_allocation.hpp | 3 +- include/perf_watcher.hpp | 1 + src/ddprof_context_lib.cc | 35 --------------- src/ddprof_worker.cc | 12 +++-- src/lib/allocation_tracker.cc | 65 ++++++++++++++++++++++++--- src/perf_watcher.cc | 36 +++++++++++++++ test/CMakeLists.txt | 2 +- test/allocation_tracker-ut.cc | 71 +++++++++++++++++++++++++++--- test/simple_malloc-ut.sh | 3 -- 13 files changed, 213 insertions(+), 66 deletions(-) create mode 100644 include/live_allocation-c.hpp diff --git a/include/ddprof_context_lib.hpp b/include/ddprof_context_lib.hpp index 94430a5cb..af352f5e3 100644 --- a/include/ddprof_context_lib.hpp +++ b/include/ddprof_context_lib.hpp @@ -14,6 +14,4 @@ typedef struct PerfWatcher PerfWatcher; DDRes ddprof_context_set(DDProfInput *input, DDProfContext *); void ddprof_context_free(DDProfContext *); -void log_watcher(const PerfWatcher *w, int idx); - int ddprof_context_allocation_profiling_watcher_idx(const DDProfContext *ctx); diff --git a/include/ddprof_perf_event.hpp b/include/ddprof_perf_event.hpp index d64c6b88a..ceda11982 100644 --- a/include/ddprof_perf_event.hpp +++ b/include/ddprof_perf_event.hpp @@ -10,9 +10,13 @@ // Extend the perf event types // There are <30 different perf events (starting at 1000 seems safe) -constexpr uint32_t PERF_CUSTOM_EVENT_DEALLOCATION = 1000; +enum : uint32_t { + PERF_CUSTOM_EVENT_DEALLOCATION = 1000, + PERF_CUSTOM_EVENT_CLEAR_LIVE_ALLOCATION +}; -static_assert(PERF_CUSTOM_EVENT_DEALLOCATION > PERF_RECORD_MAX, +static_assert(static_cast(PERF_CUSTOM_EVENT_DEALLOCATION) > + PERF_RECORD_MAX, "Error from PERF_CUSTOM_EVENT_DEALLOCATION definition"); namespace ddprof { @@ -24,4 +28,10 @@ struct DeallocationEvent { uintptr_t ptr; }; +// Event to notify we have tracked too many allocations +struct ClearLiveAllocationEvent { + perf_event_header hdr; + struct sample_id sample_id; +}; + } // namespace ddprof diff --git a/include/lib/allocation_tracker.hpp b/include/lib/allocation_tracker.hpp index 0e29deaf2..43efb31c4 100644 --- a/include/lib/allocation_tracker.hpp +++ b/include/lib/allocation_tracker.hpp @@ -66,12 +66,19 @@ class AllocationTracker { using AdressSet = std::unordered_set; struct TrackerState { + void init(bool track_alloc, bool track_dealloc) { + track_allocations = track_alloc; + track_deallocations = track_dealloc; + lost_count = 0; + failure_count = 0; + pid = 0; + } std::mutex mutex; std::atomic track_allocations = false; std::atomic track_deallocations = false; std::atomic lost_count; // count number of lost events std::atomic failure_count; - std::atomic pid; // cache of pid + std::atomic pid; // lazy cache of pid (0 is un-init value) }; AllocationTracker(); @@ -87,15 +94,16 @@ class AllocationTracker { TrackerThreadLocalState &tl_state); void track_deallocation(uintptr_t addr, TrackerThreadLocalState &tl_state); - DDRes push_sample(uintptr_t addr, uint64_t allocated_size, - TrackerThreadLocalState &tl_state); + DDRes push_alloc_sample(uintptr_t addr, uint64_t allocated_size, + TrackerThreadLocalState &tl_state); - // Return true if consumer should be notified + // If notify_needed is true, consumer should be notified DDRes push_lost_sample(MPSCRingBufferWriter &writer, bool ¬ify_needed); - // Return true if consumer should be notified DDRes push_dealloc_sample(uintptr_t addr, TrackerThreadLocalState &tl_state); + DDRes push_clear_live_allocation(TrackerThreadLocalState &tl_state); + void free_on_consecutive_failures(bool success); TrackerState _state; diff --git a/include/live_allocation-c.hpp b/include/live_allocation-c.hpp new file mode 100644 index 000000000..657e84b1b --- /dev/null +++ b/include/live_allocation-c.hpp @@ -0,0 +1,17 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. This product includes software +// developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present +// Datadog, Inc. + +#pragma once + +namespace ddprof { +namespace liveallocation { +#ifdef KMAX_TRACKED_ALLOCATIONS +// build time override to reduce execution time of test +static constexpr auto kMaxTracked = KMAX_TRACKED_ALLOCATIONS; +#else +static constexpr auto kMaxTracked = 500000; +#endif +} // namespace liveallocation +} // namespace ddprof diff --git a/include/live_allocation.hpp b/include/live_allocation.hpp index 4a5ffcdb6..966682a3f 100644 --- a/include/live_allocation.hpp +++ b/include/live_allocation.hpp @@ -15,7 +15,6 @@ namespace ddprof { class LiveAllocation { public: - static constexpr auto kMaxTracked = 200000; void register_allocation(const UnwindOutput &stack, uintptr_t addr, size_t size, int watcher_pos, pid_t pid) { StackMap &stack_map = _pid_map[pid]; @@ -30,6 +29,8 @@ class LiveAllocation { } } + void clear(pid_t pid) { _pid_map[pid].clear(); } + struct AllocationInfo { UnwindOutput _stack; size_t _size; diff --git a/include/perf_watcher.hpp b/include/perf_watcher.hpp index f658d873f..0685b5970 100644 --- a/include/perf_watcher.hpp +++ b/include/perf_watcher.hpp @@ -170,3 +170,4 @@ int sample_type_id_to_count_sample_type_id(int idx); // Helper functions, mostly for tests uint64_t perf_event_default_sample_type(); +void log_watcher(const PerfWatcher *w, int idx); \ No newline at end of file diff --git a/src/ddprof_context_lib.cc b/src/ddprof_context_lib.cc index 6245296a0..9d61a4596 100644 --- a/src/ddprof_context_lib.cc +++ b/src/ddprof_context_lib.cc @@ -100,41 +100,6 @@ DDRes add_preset(DDProfContext *ctx, const char *preset, return {}; } -void log_watcher(const PerfWatcher *w, int idx) { - PRINT_NFO(" ID: %s, Pos: %d, Index: %lu", w->desc.c_str(), idx, w->config); - switch (w->value_source) { - case EventConfValueSource::kSample: - PRINT_NFO(" Location: Sample"); - break; - case EventConfValueSource::kRegister: - PRINT_NFO(" Location: Register, regno: %d", w->regno); - break; - case EventConfValueSource::kRaw: - PRINT_NFO(" Location: Raw event, offset: %d, size: %d", w->raw_off, - w->raw_sz); - break; - default: - PRINT_NFO(" ILLEGAL LOCATION"); - break; - } - - PRINT_NFO(" Category: %s, EventName: %s, GroupName: %s, Label: %s", - sample_type_name_from_idx(w->sample_type_id), - w->tracepoint_event.c_str(), w->tracepoint_group.c_str(), - w->tracepoint_label.c_str()); - - if (w->options.is_freq) - PRINT_NFO(" Cadence: Freq, Freq: %lu", w->sample_frequency); - else - PRINT_NFO(" Cadence: Period, Period: %lu", w->sample_period); - if (Any(EventConfMode::kCallgraph & w->output_mode)) - PRINT_NFO(" Outputting to callgraph (flamegraph)"); - if (Any(EventConfMode::kMetric & w->output_mode)) - PRINT_NFO(" Outputting to metric"); - if (Any(EventConfMode::kLiveCallgraph & w->output_mode)) - PRINT_NFO(" Outputting to live callgraph"); -} - /**************************** Argument Processor ***************************/ DDRes ddprof_context_set(DDProfInput *input, DDProfContext *ctx) { *ctx = {}; diff --git a/src/ddprof_worker.cc b/src/ddprof_worker.cc index 71fcef01f..993c58210 100644 --- a/src/ddprof_worker.cc +++ b/src/ddprof_worker.cc @@ -468,10 +468,6 @@ static DDRes aggregate_live_allocations(DDProfContext *ctx) { } LG_NTC("Number of Live allocations for PID%d = %lu ", stack_map.first, stack_map.second.size()); - // Safety to avoid spending all the time reporting allocations - if (stack_map.second.size() >= LiveAllocation::kMaxTracked) { - stack_map.second.clear(); - } } return ddres_init(); } @@ -659,6 +655,11 @@ void ddprof_pr_exit(DDProfContext *ctx, const perf_event_exit *ext, } } +void ddprof_pr_clear_live_allocation(DDProfContext *ctx, + const ClearLiveAllocationEvent *event) { + ctx->worker_ctx.live_allocation.clear(event->sample_id.pid); +} + void ddprof_pr_deallocation(DDProfContext *ctx, const DeallocationEvent *event) { ctx->worker_ctx.live_allocation.register_deallocation(event->ptr, @@ -818,6 +819,9 @@ DDRes ddprof_worker_process_event(const perf_event_header *hdr, int watcher_pos, case PERF_CUSTOM_EVENT_DEALLOCATION: ddprof_pr_deallocation(ctx, reinterpret_cast(hdr)); + case PERF_CUSTOM_EVENT_CLEAR_LIVE_ALLOCATION: + ddprof_pr_clear_live_allocation( + ctx, reinterpret_cast(hdr)); default: break; } diff --git a/src/lib/allocation_tracker.cc b/src/lib/allocation_tracker.cc index 4e16e7424..2f1747cd8 100644 --- a/src/lib/allocation_tracker.cc +++ b/src/lib/allocation_tracker.cc @@ -9,6 +9,7 @@ #include "ddres.hpp" #include "defer.hpp" #include "ipc.hpp" +#include "live_allocation-c.hpp" #include "perf.hpp" #include "pevent_lib.hpp" #include "ringbuffer_utils.hpp" @@ -97,8 +98,8 @@ DDRes AllocationTracker::allocation_tracking_init( DDRES_CHECK_FWD(instance->init(allocation_profiling_rate, flags & kDeterministicSampling, ring_buffer)); _instance = instance; - state.track_allocations = true; - state.track_deallocations = flags & kTrackDeallocations; + + state.init(true, flags & kTrackDeallocations); return {}; } @@ -199,11 +200,22 @@ void AllocationTracker::track_allocation(uintptr_t addr, size_t size, tl_state.remaining_bytes = remaining_bytes; uint64_t total_size = nsamples * sampling_interval; - bool success = IsDDResOK(push_sample(addr, total_size, tl_state)); + bool success = IsDDResOK(push_alloc_sample(addr, total_size, tl_state)); free_on_consecutive_failures(success); - if (success) { // ensure we track this dealloc if it occurs + if (success && _state.track_deallocations) { + // ensure we track this dealloc if it occurs _address_set.insert(addr); + if (unlikely(_address_set.size() > ddprof::liveallocation::kMaxTracked)) { + if (IsDDResOK(push_clear_live_allocation(tl_state))) { + _address_set.clear(); + } else { + fprintf( + stderr, + "Stop allocation profiling. Unable to clear live allocation \n"); + free(); + } + } } } @@ -259,6 +271,46 @@ DDRes AllocationTracker::push_lost_sample(MPSCRingBufferWriter &writer, } // Return true if consumer should be notified +DDRes AllocationTracker::push_clear_live_allocation( + TrackerThreadLocalState &tl_state) { + MPSCRingBufferWriter writer{_pevent.rb}; + bool timeout = false; + + auto buffer = writer.reserve(sizeof(ClearLiveAllocationEvent), &timeout); + if (buffer.empty()) { + // unable to push a clear is an error (we don't want to grow too much) + // No use pushing a lost event. As this is a sync mechanism. + DDRES_RETURN_ERROR_LOG(DD_WHAT_PERFRB, + "Unable to get write lock on ring buffer"); + } + + ClearLiveAllocationEvent *event = + reinterpret_cast(buffer.data()); + event->hdr.misc = 0; + event->hdr.size = sizeof(ClearLiveAllocationEvent); + event->hdr.type = PERF_CUSTOM_EVENT_CLEAR_LIVE_ALLOCATION; + event->sample_id.time = 0; + if (_state.pid == 0) { + _state.pid = getpid(); + } + if (tl_state.tid == 0) { + tl_state.tid = ddprof::gettid(); + } + event->sample_id.pid = _state.pid; + event->sample_id.tid = tl_state.tid; + + if (writer.commit(buffer)) { + uint64_t count = 1; + if (write(_pevent.fd, &count, sizeof(count)) != sizeof(count)) { + DDRES_RETURN_ERROR_LOG(DD_WHAT_PERFRB, + "Error writing to memory allocation eventfd (%s)", + strerror(errno)); + } + } + + return {}; +} + DDRes AllocationTracker::push_dealloc_sample( uintptr_t addr, TrackerThreadLocalState &tl_state) { MPSCRingBufferWriter writer{_pevent.rb}; @@ -312,8 +364,9 @@ DDRes AllocationTracker::push_dealloc_sample( return {}; } -DDRes AllocationTracker::push_sample(uintptr_t addr, uint64_t allocated_size, - TrackerThreadLocalState &tl_state) { +DDRes AllocationTracker::push_alloc_sample(uintptr_t addr, + uint64_t allocated_size, + TrackerThreadLocalState &tl_state) { MPSCRingBufferWriter writer{_pevent.rb}; bool notify_consumer{false}; diff --git a/src/perf_watcher.cc b/src/perf_watcher.cc index 4267b6609..715036325 100644 --- a/src/perf_watcher.cc +++ b/src/perf_watcher.cc @@ -5,6 +5,7 @@ #include "perf_watcher.hpp" +#include "logger.hpp" #include "perf.hpp" #include @@ -104,3 +105,38 @@ const PerfWatcher *tracepoint_default_watcher() { bool watcher_has_tracepoint(const PerfWatcher *watcher) { return DDPROF_PWT_TRACEPOINT == watcher->sample_type_id; } + +void log_watcher(const PerfWatcher *w, int idx) { + PRINT_NFO(" ID: %s, Pos: %d, Index: %lu", w->desc.c_str(), idx, w->config); + switch (w->value_source) { + case EventConfValueSource::kSample: + PRINT_NFO(" Location: Sample"); + break; + case EventConfValueSource::kRegister: + PRINT_NFO(" Location: Register, regno: %d", w->regno); + break; + case EventConfValueSource::kRaw: + PRINT_NFO(" Location: Raw event, offset: %d, size: %d", w->raw_off, + w->raw_sz); + break; + default: + PRINT_NFO(" ILLEGAL LOCATION"); + break; + } + + PRINT_NFO(" Category: %s, EventName: %s, GroupName: %s, Label: %s", + sample_type_name_from_idx(w->sample_type_id), + w->tracepoint_event.c_str(), w->tracepoint_group.c_str(), + w->tracepoint_label.c_str()); + + if (w->options.is_freq) + PRINT_NFO(" Cadence: Freq, Freq: %lu", w->sample_frequency); + else + PRINT_NFO(" Cadence: Period, Period: %lu", w->sample_period); + if (Any(EventConfMode::kCallgraph & w->output_mode)) + PRINT_NFO(" Outputting to callgraph (flamegraph)"); + if (Any(EventConfMode::kMetric & w->output_mode)) + PRINT_NFO(" Outputting to metric"); + if (Any(EventConfMode::kLiveCallgraph & w->output_mode)) + PRINT_NFO(" Outputting to live callgraph"); +} diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 9ee2e06e6..e94229276 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -291,7 +291,7 @@ add_unit_test( ../src/unwind_helpers.cc ../src/unwind_metrics.cc LIBRARIES ${ELFUTILS_LIBRARIES} llvm-demangle DDProf::Parser - DEFINITIONS ${DDPROF_DEFINITION_LIST}) + DEFINITIONS ${DDPROF_DEFINITION_LIST} KMAX_TRACKED_ALLOCATIONS=100) add_unit_test(sys_utils-ut sys_utils-ut.cc ../src/sys_utils.cc) diff --git a/test/allocation_tracker-ut.cc b/test/allocation_tracker-ut.cc index a9ef6bd28..ba646ccc8 100644 --- a/test/allocation_tracker-ut.cc +++ b/test/allocation_tracker-ut.cc @@ -7,6 +7,7 @@ #include "ddprof_base.hpp" #include "ddprof_perf_event.hpp" #include "ipc.hpp" +#include "live_allocation-c.hpp" #include "loghandle.hpp" #include "perf_watcher.hpp" #include "pevent_lib.hpp" @@ -20,8 +21,8 @@ #include #include -DDPROF_NOINLINE void my_malloc(size_t size) { - ddprof::AllocationTracker::track_allocation(0xdeadbeef, size); +DDPROF_NOINLINE void my_malloc(size_t size, uintptr_t addr = 0xdeadbeef) { + ddprof::AllocationTracker::track_allocation(addr, size); // prevent tail call optimization getpid(); } @@ -98,10 +99,10 @@ TEST(allocation_tracker, start_stop) { ASSERT_EQ(sample->ptr, 0xdeadbeef); } my_free(0xcafebabe); - // { - // ddprof::MPSCRingBufferReader reader{ring_buffer.get_ring_buffer()}; - // ASSERT_EQ(reader.available_size(), 0); - // } + { + ddprof::MPSCRingBufferReader reader{ring_buffer.get_ring_buffer()}; + ASSERT_EQ(reader.available_size(), 0); + } ddprof::AllocationTracker::allocation_tracking_free(); ASSERT_FALSE(ddprof::AllocationTracker::is_active()); } @@ -113,7 +114,9 @@ TEST(allocation_tracker, stale_lock) { ddprof::RingBufferHolder ring_buffer{buf_size_order, RingBufferType::kMPSCRingBuffer}; ddprof::AllocationTracker::allocation_tracking_init( - rate, ddprof::AllocationTracker::kDeterministicSampling, + rate, + ddprof::AllocationTracker::kDeterministicSampling | + ddprof::AllocationTracker::kTrackDeallocations, ring_buffer.get_buffer_info()); // simulate stale lock @@ -126,3 +129,57 @@ TEST(allocation_tracker, stale_lock) { ASSERT_FALSE(ddprof::AllocationTracker::is_active()); ddprof::AllocationTracker::allocation_tracking_free(); } + +TEST(allocation_tracker, max_tracked_allocs) { + const uint64_t rate = 1; + const size_t buf_size_order = 9; + ddprof::RingBufferHolder ring_buffer{buf_size_order, + RingBufferType::kMPSCRingBuffer}; + ddprof::AllocationTracker::allocation_tracking_init( + rate, + ddprof::AllocationTracker::kDeterministicSampling | + ddprof::AllocationTracker::kTrackDeallocations, + ring_buffer.get_buffer_info()); + + ASSERT_TRUE(ddprof::AllocationTracker::is_active()); + + for (int i = 0; i <= ddprof::liveallocation::kMaxTracked + 1; ++i) { + my_malloc(1, 0x1000 + i); + ddprof::MPSCRingBufferReader reader{ring_buffer.get_ring_buffer()}; + if (i <= + ddprof::liveallocation::kMaxTracked) { // check that we get the relevant + // info for this allocation + ASSERT_GT(reader.available_size(), 0); + auto buf = reader.read_sample(); + ASSERT_FALSE(buf.empty()); + const perf_event_header *hdr = + reinterpret_cast(buf.data()); + ASSERT_EQ(hdr->type, PERF_RECORD_SAMPLE); + + perf_event_sample *sample = + hdr2samp(hdr, perf_event_default_sample_type() | PERF_SAMPLE_ADDR); + + ASSERT_EQ(sample->period, 1); + ASSERT_EQ(sample->pid, getpid()); + ASSERT_EQ(sample->tid, ddprof::gettid()); + ASSERT_EQ(sample->addr, 0x1000 + i); + } else { + bool clear_found = false; + int nb_read = 0; + ddprof::ConstBuffer buf; + do { + buf = reader.read_sample(); + ++nb_read; + if (buf.empty()) + break; + const perf_event_header *hdr = + reinterpret_cast(buf.data()); + if (hdr->type == PERF_CUSTOM_EVENT_CLEAR_LIVE_ALLOCATION) { + clear_found = true; + } + } while (true); + EXPECT_EQ(nb_read, 3); + EXPECT_TRUE(clear_found); + } + } +} diff --git a/test/simple_malloc-ut.sh b/test/simple_malloc-ut.sh index 713d290f8..1cf68e4b6 100755 --- a/test/simple_malloc-ut.sh +++ b/test/simple_malloc-ut.sh @@ -99,9 +99,6 @@ check "./ddprof --preset cpu_live_heap ./test/simple_malloc ${opts} --skip-free # Test wrapper mode with forks + threads check "./ddprof ./test/simple_malloc ${opts} --fork 2 --threads 2" 2 4 -# Test leak mode with forks + threads -# check "./ddprof --preset cpu_live_heap ./test/simple_malloc ${opts} --fork 2 --threads 2 --skip-free 100" 2 4 - # Test slow profiler startup check "env DD_PROFILING_NATIVE_STARTUP_WAIT_MS=200 ./ddprof ./test/simple_malloc ${opts}" 1 From e531c4b945d99b441d8d70cf315c3aa86b8f0b22 Mon Sep 17 00:00:00 2001 From: r1viollet <74836499+r1viollet@users.noreply.github.com> Date: Fri, 7 Apr 2023 15:05:13 +0200 Subject: [PATCH 16/21] Live Heap - Pid cleanup - Clear aggregated live information when we remove a PID - Re-introduce the watcher dimension in the aggregation of allocations --- include/live_allocation.hpp | 36 ++++-- include/live_sysallocations.hpp | 18 +-- include/pprof/ddprof_pprof.hpp | 2 +- src/ddprof_worker.cc | 216 +++++++++++++++++++++++--------- src/lib/allocation_tracker.cc | 1 - src/pprof/ddprof_pprof.cc | 6 +- src/unwind.cc | 5 - test/ddprof_exporter-ut.cc | 2 +- test/ddprof_pprof-ut.cc | 2 +- 9 files changed, 191 insertions(+), 97 deletions(-) diff --git a/include/live_allocation.hpp b/include/live_allocation.hpp index 966682a3f..51887d5fb 100644 --- a/include/live_allocation.hpp +++ b/include/live_allocation.hpp @@ -8,42 +8,60 @@ #include "ddprof_defs.hpp" #include "logger.hpp" #include "unwind_output.hpp" +#include "unlikely.hpp" #include namespace ddprof { +template +T& access_resize(std::vector& v, size_t index, const T& default_value = T()) { + if (unlikely(index >= v.size())) { + v.resize(index + 1, default_value); + } + return v[index]; +} + + class LiveAllocation { public: void register_allocation(const UnwindOutput &stack, uintptr_t addr, size_t size, int watcher_pos, pid_t pid) { - StackMap &stack_map = _pid_map[pid]; + PidMap &pid_map = access_resize(_watcher_vector, watcher_pos); + StackMap &stack_map = pid_map[pid]; stack_map[addr] = AllocationInfo{ ._stack = stack, ._size = size, ._watcher_pos = watcher_pos}; } - void register_deallocation(uintptr_t addr, pid_t pid) { - StackMap &stack_map = _pid_map[pid]; + void register_deallocation(uintptr_t addr, int watcher_pos, pid_t pid) { + PidMap &pid_map = access_resize(_watcher_vector, watcher_pos); + StackMap &stack_map = pid_map[pid]; if (!stack_map.erase(addr)) { LG_DBG("Unmatched deallocation at %lx of PID%d", addr, pid); } } - void clear(pid_t pid) { _pid_map[pid].clear(); } + void clear_pid_for_watcher(int watcher_pos, pid_t pid) { + PidMap &pid_map = access_resize(_watcher_vector, watcher_pos); + pid_map[pid].clear(); + } + + void clear_pid(pid_t pid) { + for (auto &pid_map : _watcher_vector) { + pid_map[pid].clear(); + } + } struct AllocationInfo { UnwindOutput _stack; size_t _size; - // Should the watcher be part of the key ? - // In theory we could watch allocations with different rules (per watcher) - // for now I'll leave it here int _watcher_pos; }; using StackMap = std::unordered_map; using PidMap = std::unordered_map; - - PidMap _pid_map; + using WatcherVector = std::vector; + WatcherVector _watcher_vector; }; } // namespace ddprof \ No newline at end of file diff --git a/include/live_sysallocations.hpp b/include/live_sysallocations.hpp index 52afddcb9..b6bde8896 100644 --- a/include/live_sysallocations.hpp +++ b/include/live_sysallocations.hpp @@ -85,23 +85,7 @@ class SystemAllocation { add_allocs(stack, addr1, size1, pid); } - void do_exit(pid_t pid) { - StackMap &stack_map = _pid_map[pid]; - stack_map.clear(); - _visited_recently.erase(pid); - } - - void sanitize_pids() { - for (auto &stack_map : _pid_map) { - if (!_visited_recently.contains(stack_map.first)) { - // This PID wasn't visited recently. Is it still around? - if (kill(stack_map.first, 0)) { - _pid_map[stack_map.first].clear(); - } - } - } - _visited_recently.clear(); - } + void clear_pid(pid_t pid) { _pid_map.erase(pid); } using StackMap = std::unordered_map; using PidMap = std::unordered_map; diff --git a/include/pprof/ddprof_pprof.hpp b/include/pprof/ddprof_pprof.hpp index 226f5e972..2e4d555f7 100644 --- a/include/pprof/ddprof_pprof.hpp +++ b/include/pprof/ddprof_pprof.hpp @@ -33,7 +33,7 @@ DDRes pprof_create_profile(DDProfPProf *pprof, DDProfContext *ctx); * @param pprof */ DDRes pprof_aggregate(const UnwindOutput *uw_output, - const SymbolHdr *symbol_hdr, uint64_t value, + const SymbolHdr &symbol_hdr, uint64_t value, uint64_t count, const PerfWatcher *watcher, DDProfPProf *pprof); diff --git a/src/ddprof_worker.cc b/src/ddprof_worker.cc index 993c58210..c7c820320 100644 --- a/src/ddprof_worker.cc +++ b/src/ddprof_worker.cc @@ -16,6 +16,7 @@ #include "pevent_lib.hpp" #include "pprof/ddprof_pprof.hpp" #include "procutils.hpp" +#include "signal_helper.hpp" #include "stack_handler.hpp" #include "tags.hpp" #include "timer.hpp" @@ -42,6 +43,11 @@ static const DDPROF_STATS s_cycled_stats[] = { static const long k_clock_ticks_per_sec = sysconf(_SC_CLK_TCK); +/// Remove all structures related to +static DDRes worker_pid_free(DDProfContext *ctx, pid_t el); + +static DDRes clear_unvisited_pids(DDProfContext *ctx); + /// Human readable runtime information static void print_diagnostics(const DsoHdr &dso_hdr) { LG_NFO("Printing internal diagnostics"); @@ -69,7 +75,7 @@ static DDRes report_lost_events(DDProfContext *ctx) { watcher->sample_period, watcher_idx); DDRES_CHECK_FWD(pprof_aggregate( - &us->output, &us->symbol_hdr, watcher->sample_period, + &us->output, us->symbol_hdr, watcher->sample_period, ctx->worker_ctx.lost_events_per_watcher[watcher_idx], watcher, ctx->worker_ctx.pprof[ctx->worker_ctx.i_current_pprof])); ctx->worker_ctx.lost_events_per_watcher[watcher_idx] = 0; @@ -271,6 +277,11 @@ static DDRes ddprof_unwind_sample(DDProfContext *ctx, perf_event_sample *sample, ddprof_stats_add(STATS_UNWIND_TRUNCATED_INPUT, 1, nullptr); } + if (us->_dwfl_wrapper->_inconsistent) { + // Loaded modules were inconsistend, assume we should flush everything. + LG_WRN("(Inconsistent DWFL/DSOs)%d - Free associated objects", us->pid); + DDRES_CHECK_FWD(worker_pid_free(ctx, us->pid)); + } return res; } @@ -299,16 +310,9 @@ DDRes ddprof_pr_sample(DDProfContext *ctx, perf_event_sample *sample, struct UnwindState *us = ctx->worker_ctx.us; if (Any(EventConfMode::kLiveCallgraph & watcher->output_mode)) { // Live callgraph mode - if (watcher->ddprof_event_type == DDPROF_PWE_sALLOC) { - // for now we hard code the live aggregation mode - ctx->worker_ctx.live_allocation.register_allocation( - us->output, sample->addr, sample->period, watcher_pos, sample->pid); - } - // live callgraph not compatible with all watcher types - else { - DDRES_RETURN_ERROR_LOG(DD_WHAT_UNHANDLED_CONFIG, - "Live callgraph configuration unhandled"); - } + // for now we hard code the live aggregation mode + ctx->worker_ctx.live_allocation.register_allocation( + us->output, sample->addr, sample->period, watcher_pos, sample->pid); } else if (Any(EventConfMode::kCallgraph & watcher->output_mode)) { #ifndef DDPROF_NATIVE_LIB // Depending on the type of watcher, compute a value for sample @@ -317,7 +321,7 @@ DDRes ddprof_pr_sample(DDProfContext *ctx, perf_event_sample *sample, // in lib mode we don't aggregate (protect to avoid link failures) int i_export = ctx->worker_ctx.i_current_pprof; DDProfPProf *pprof = ctx->worker_ctx.pprof[i_export]; - DDRES_CHECK_FWD(pprof_aggregate(&us->output, &us->symbol_hdr, sample_val, + DDRES_CHECK_FWD(pprof_aggregate(&us->output, us->symbol_hdr, sample_val, 1, watcher, pprof)); if (ctx->params.show_samples) { ddprof_print_sample(us->output, us->symbol_hdr, sample->period, @@ -345,8 +349,8 @@ DDRes ddprof_pr_sample(DDProfContext *ctx, perf_event_sample *sample, DDRes ddprof_pr_sysallocation_tracking(DDProfContext *ctx, perf_event_sample *sample, - int watcher_pos) { - + int watcher_pos, bool &clear_pid) { + clear_pid = false; // Syscall parameters. Suppressing nags because it's annoying to look these // up and it isn't totally appropriate to spin out a new header just // for this @@ -408,7 +412,7 @@ DDRes ddprof_pr_sysallocation_tracking(DDProfContext *ctx, } else if (id == 60 || id == 231 || id == 59 || id == 322 || id == 520 || id == 545) { // Erase upon exit or exec - sysalloc.do_exit(sample->pid); + clear_pid = true; } return ddres_init(); @@ -442,76 +446,151 @@ void *ddprof_worker_export_thread(void *arg) { #endif #ifndef DDPROF_NATIVE_LIB -static DDRes aggregate_stack(const LiveAllocation::AllocationInfo &alloc_info, - DDProfContext *ctx) { - struct UnwindState *us = ctx->worker_ctx.us; - int watcher_pos = alloc_info._watcher_pos; - PerfWatcher *watcher = &ctx->watchers[watcher_pos]; - int i_export = ctx->worker_ctx.i_current_pprof; - DDProfPProf *pprof = ctx->worker_ctx.pprof[i_export]; - DDRES_CHECK_FWD(pprof_aggregate(&alloc_info._stack, &us->symbol_hdr, +static DDRes +aggregate_livealloc_stack(const LiveAllocation::AllocationInfo &alloc_info, + DDProfContext *ctx, + const PerfWatcher *watcher, + DDProfPProf *pprof, + const SymbolHdr &symbol_hdr) { + DDRES_CHECK_FWD(pprof_aggregate(&alloc_info._stack, symbol_hdr, alloc_info._size, 1, watcher, pprof)); if (ctx->params.show_samples) { - ddprof_print_sample(alloc_info._stack, us->symbol_hdr, alloc_info._size, + ddprof_print_sample(alloc_info._stack, symbol_hdr, alloc_info._size, *watcher); } return ddres_init(); } +static DDRes aggregate_live_allocations_for_pid(DDProfContext *ctx, pid_t pid) { + struct UnwindState *us = ctx->worker_ctx.us; + int i_export = ctx->worker_ctx.i_current_pprof; + DDProfPProf *pprof = ctx->worker_ctx.pprof[i_export]; + const SymbolHdr &symbol_hdr = us->symbol_hdr; + LiveAllocation &live_allocations = ctx->worker_ctx.live_allocation; + for (unsigned watcher_pos = 0; + watcher_pos < live_allocations._watcher_vector.size(); ++watcher_pos) { + auto &pid_map = live_allocations._watcher_vector[watcher_pos]; + const PerfWatcher *watcher = &ctx->watchers[watcher_pos]; + auto &stack_map = pid_map[pid]; + for (const auto &alloc_info_pair : stack_map) { + DDRES_CHECK_FWD(aggregate_livealloc_stack(alloc_info_pair.second, ctx, + watcher, pprof, symbol_hdr)); + } + } + return ddres_init(); +} + static DDRes aggregate_live_allocations(DDProfContext *ctx) { // this would be more efficient if we could reuse the same stacks in // libdatadog + + struct UnwindState *us = ctx->worker_ctx.us; + int i_export = ctx->worker_ctx.i_current_pprof; + DDProfPProf *pprof = ctx->worker_ctx.pprof[i_export]; + const SymbolHdr &symbol_hdr = us->symbol_hdr; LiveAllocation &live_allocations = ctx->worker_ctx.live_allocation; - for (auto &stack_map : live_allocations._pid_map) { - for (const auto &alloc_info_pair : stack_map.second) { - DDRES_CHECK_FWD(aggregate_stack(alloc_info_pair.second, ctx)); + + for (unsigned watcher_pos = 0; + watcher_pos < live_allocations._watcher_vector.size(); ++watcher_pos) { + auto &pid_map = live_allocations._watcher_vector[watcher_pos]; + const PerfWatcher *watcher = &ctx->watchers[watcher_pos]; + for (auto &stack_map : pid_map) { + for (const auto &alloc_info_pair : stack_map.second) { + DDRES_CHECK_FWD(aggregate_livealloc_stack(alloc_info_pair.second, ctx, + watcher, + pprof, + symbol_hdr)); + } + LG_NTC("<%u> Number of Live allocations for PID%d = %lu ", + watcher_pos, + stack_map.first, + stack_map.second.size()); } - LG_NTC("Number of Live allocations for PID%d = %lu ", stack_map.first, - stack_map.second.size()); } return ddres_init(); } -static DDRes aggregate_sys_allocations(DDProfContext *ctx) { +DDRes aggregate_sys_allocation_stack(const UnwindOutput *uw_output, + const SymbolHdr *symbol_hdr, + const PerfWatcher *watcher, + DDProfPProf *pprof) { + DDRES_CHECK_FWD(pprof_aggregate(uw_output, *symbol_hdr, get_page_size(), 1, + watcher, pprof)); + return ddres_init(); +} + +static DDRes aggregate_sys_allocations_for_pid(DDProfContext *ctx, pid_t pid) { struct UnwindState *us = ctx->worker_ctx.us; SystemAllocation &sysallocs = ctx->worker_ctx.sys_allocation; PerfWatcher *watcher = &ctx->watchers[sysallocs.watcher_pos]; int i_export = ctx->worker_ctx.i_current_pprof; DDProfPProf *pprof = ctx->worker_ctx.pprof[i_export]; + const auto &stack_map = sysallocs._pid_map[pid]; + for (const auto &page : stack_map) { + DDRES_CHECK_FWD(aggregate_sys_allocation_stack( + &page.second, &us->symbol_hdr, watcher, pprof)); + } + return ddres_init(); +} - // Before we do anything, clear the pids that died in this period - sysallocs.sanitize_pids(); +static DDRes aggregate_sys_allocations(DDProfContext *ctx) { + struct UnwindState *us = ctx->worker_ctx.us; + SystemAllocation &sysallocs = ctx->worker_ctx.sys_allocation; + PerfWatcher *watcher = &ctx->watchers[sysallocs.watcher_pos]; + int i_export = ctx->worker_ctx.i_current_pprof; + DDProfPProf *pprof = ctx->worker_ctx.pprof[i_export]; // Iterate through each PID for (auto &stack_map : sysallocs._pid_map) { - // Iterate through pages... // TODO Probably aggregate into ranges of pages or something, but once per // page is just too much for (const auto &page : stack_map.second) { - DDRES_CHECK_FWD(pprof_aggregate(&page.second, &us->symbol_hdr, 4096, 1, - watcher, pprof)); + DDRES_CHECK_FWD(aggregate_sys_allocation_stack( + &page.second, &us->symbol_hdr, watcher, pprof)); } } return ddres_init(); } + +static DDRes worker_pid_free(DDProfContext *ctx, pid_t el) { + DDRES_CHECK_FWD(aggregate_sys_allocations_for_pid(ctx, el)); + DDRES_CHECK_FWD(aggregate_live_allocations_for_pid(ctx, el)); + UnwindState *us = ctx->worker_ctx.us; + unwind_pid_free(us, el); + ctx->worker_ctx.sys_allocation.clear_pid(el); + ctx->worker_ctx.live_allocation.clear_pid(el); + return ddres_init(); +} +#else +static DDRes worker_pid_free(DDProfContext *ctx, pid_t el) { + UnwindState *us = ctx->worker_ctx.us; + unwind_pid_free(us, el); + ctx->worker_ctx.sys_allocation.clear_pid(el); + ctx->worker_ctx.live_allocation.clear_pid(el); + return ddres_init(); +} #endif -static void clear_unvisted_pids(DDProfWorkerContext &worker_ctx) { - UnwindState *us = worker_ctx.us; +static DDRes clear_unvisited_pids(DDProfContext *ctx) { + UnwindState *us = ctx->worker_ctx.us; const std::vector pids_remove = us->dwfl_hdr.get_unvisited(); for (pid_t el : pids_remove) { - unwind_pid_free(us, el); + if (!process_is_alive(el)) { + DDRES_CHECK_FWD(worker_pid_free(ctx, el)); + } } us->dwfl_hdr.reset_unvisited(); + return ddres_init(); } /// Cycle operations : export, sync metrics, update counters DDRes ddprof_worker_cycle(DDProfContext *ctx, int64_t now, [[maybe_unused]] bool synchronous_export) { + // Clearing unused PIDs will ensure we don't report them at next cycle + DDRES_CHECK_FWD(clear_unvisited_pids(ctx)); #ifndef DDPROF_NATIVE_LIB - // TODO: lib mode (unhandled for now) DDRES_CHECK_FWD(aggregate_live_allocations(ctx)); DDRES_CHECK_FWD(aggregate_sys_allocations(ctx)); @@ -598,8 +677,6 @@ DDRes ddprof_worker_cycle(DDProfContext *ctx, int64_t now, } unwind_cycle(ctx->worker_ctx.us); - clear_unvisted_pids(ctx->worker_ctx); - // Reset stats relevant to a single cycle ddprof_reset_worker_stats(); @@ -622,22 +699,24 @@ void ddprof_pr_lost(DDProfContext *ctx, const perf_event_lost *lost, ctx->worker_ctx.lost_events_per_watcher[watcher_pos] += lost->lost; } -void ddprof_pr_comm(DDProfContext *ctx, const perf_event_comm *comm, - int watcher_pos) { +DDRes ddprof_pr_comm(DDProfContext *ctx, const perf_event_comm *comm, + int watcher_pos) { // Change in process name (assuming exec) : clear all associated dso if (comm->header.misc & PERF_RECORD_MISC_COMM_EXEC) { LG_DBG("<%d>(COMM)%d -> %s", watcher_pos, comm->pid, comm->comm); - unwind_pid_free(ctx->worker_ctx.us, comm->pid); + DDRES_CHECK_FWD(worker_pid_free(ctx, comm->pid)); } + return ddres_init(); } -void ddprof_pr_fork(DDProfContext *ctx, const perf_event_fork *frk, - int watcher_pos) { +DDRes ddprof_pr_fork(DDProfContext *ctx, const perf_event_fork *frk, + int watcher_pos) { LG_DBG("<%d>(FORK)%d -> %d/%d", watcher_pos, frk->ppid, frk->pid, frk->tid); if (frk->ppid != frk->pid) { // Clear everything and populate at next error or with coming samples - unwind_pid_free(ctx->worker_ctx.us, frk->pid); + DDRES_CHECK_FWD(worker_pid_free(ctx, frk->pid)); } + return ddres_init(); } void ddprof_pr_exit(DDProfContext *ctx, const perf_event_exit *ext, @@ -656,13 +735,17 @@ void ddprof_pr_exit(DDProfContext *ctx, const perf_event_exit *ext, } void ddprof_pr_clear_live_allocation(DDProfContext *ctx, - const ClearLiveAllocationEvent *event) { - ctx->worker_ctx.live_allocation.clear(event->sample_id.pid); + const ClearLiveAllocationEvent *event, + int watcher_pos) { + LG_DBG("<%d>(CLEAR LIVE)%d", watcher_pos, event->sample_id.pid); + ctx->worker_ctx.live_allocation.clear_pid_for_watcher(watcher_pos, + event->sample_id.pid); } void ddprof_pr_deallocation(DDProfContext *ctx, - const DeallocationEvent *event) { + const DeallocationEvent *event, int watcher_pos) { ctx->worker_ctx.live_allocation.register_deallocation(event->ptr, + watcher_pos, event->sample_id.pid); } @@ -782,8 +865,14 @@ DDRes ddprof_worker_process_event(const perf_event_header *hdr, int watcher_pos, watcher->ddprof_event_type == DDPROF_PWE_tALLOCSYS2) { // For now we have a different path for // - mmap/munmap syscalls - DDRES_CHECK_FWD( - ddprof_pr_sysallocation_tracking(ctx, sample, watcher_pos)); + bool clear_pid = false; + DDRES_CHECK_FWD(ddprof_pr_sysallocation_tracking( + ctx, sample, watcher_pos, clear_pid)); + if (clear_pid) { + LG_DBG("<%d>(SYSEXIT)%d", watcher_pos, wpid->pid); + // we could consider clearing the pid here + // though we could get other types of events + } } else { DDRES_CHECK_FWD(ddprof_pr_sample(ctx, sample, watcher_pos)); } @@ -797,8 +886,8 @@ DDRes ddprof_worker_process_event(const perf_event_header *hdr, int watcher_pos, break; case PERF_RECORD_COMM: if (wpid->pid) - ddprof_pr_comm(ctx, reinterpret_cast(hdr), - watcher_pos); + DDRES_CHECK_FWD(ddprof_pr_comm( + ctx, reinterpret_cast(hdr), watcher_pos)); break; case PERF_RECORD_EXIT: if (wpid->pid) @@ -807,8 +896,9 @@ DDRes ddprof_worker_process_event(const perf_event_header *hdr, int watcher_pos, break; case PERF_RECORD_FORK: if (wpid->pid) - ddprof_pr_fork(ctx, reinterpret_cast(hdr), - watcher_pos); + DDRES_CHECK_FWD(ddprof_pr_fork( + ctx, reinterpret_cast(hdr), watcher_pos)); + break; /* Cases where the target type might not have a PID */ @@ -818,10 +908,18 @@ DDRes ddprof_worker_process_event(const perf_event_header *hdr, int watcher_pos, break; case PERF_CUSTOM_EVENT_DEALLOCATION: ddprof_pr_deallocation(ctx, - reinterpret_cast(hdr)); + reinterpret_cast(hdr), + watcher_pos); + break; case PERF_CUSTOM_EVENT_CLEAR_LIVE_ALLOCATION: - ddprof_pr_clear_live_allocation( - ctx, reinterpret_cast(hdr)); + { + const ClearLiveAllocationEvent* event = reinterpret_cast(hdr); +#ifndef DDPROF_NATIVE_LIB + DDRES_CHECK_FWD(aggregate_live_allocations_for_pid(ctx, event->sample_id.pid)); +#endif + ddprof_pr_clear_live_allocation(ctx, event,watcher_pos); + } + break; default: break; } diff --git a/src/lib/allocation_tracker.cc b/src/lib/allocation_tracker.cc index 2f1747cd8..3a9ee70e2 100644 --- a/src/lib/allocation_tracker.cc +++ b/src/lib/allocation_tracker.cc @@ -396,7 +396,6 @@ DDRes AllocationTracker::push_alloc_sample(uintptr_t addr, event->abi = PERF_SAMPLE_REGS_ABI_64; event->sample_id.time = 0; event->addr = addr; - if (_state.pid == 0) { _state.pid = getpid(); } diff --git a/src/pprof/ddprof_pprof.cc b/src/pprof/ddprof_pprof.cc index fbba23ce5..995dd5cc1 100644 --- a/src/pprof/ddprof_pprof.cc +++ b/src/pprof/ddprof_pprof.cc @@ -169,12 +169,12 @@ static void write_line(const ddprof::Symbol &symbol, ddog_prof_Line *ffi_line) { // Assumption of API is that sample is valid in a single type DDRes pprof_aggregate(const UnwindOutput *uw_output, - const SymbolHdr *symbol_hdr, uint64_t value, + const SymbolHdr &symbol_hdr, uint64_t value, uint64_t count, const PerfWatcher *watcher, DDProfPProf *pprof) { - const ddprof::SymbolTable &symbol_table = symbol_hdr->_symbol_table; - const ddprof::MapInfoTable &mapinfo_table = symbol_hdr->_mapinfo_table; + const ddprof::SymbolTable &symbol_table = symbol_hdr._symbol_table; + const ddprof::MapInfoTable &mapinfo_table = symbol_hdr._mapinfo_table; ddog_prof_Profile *profile = pprof->_profile; int64_t values[DDPROF_PWT_LENGTH] = {}; diff --git a/src/unwind.cc b/src/unwind.cc index 3aad887a3..3f4c42031 100644 --- a/src/unwind.cc +++ b/src/unwind.cc @@ -101,11 +101,6 @@ DDRes unwindstate__unwind(UnwindState *us) { // Add a frame that identifies executable to which these belong add_virtual_base_frame(us); - if (us->_dwfl_wrapper->_inconsistent) { - // error detected on this pid - LG_WRN("(Inconsistent DWFL/DSOs)%d - Free associated objects", us->pid); - unwind_pid_free(us, us->pid); - } return res; } diff --git a/test/ddprof_exporter-ut.cc b/test/ddprof_exporter-ut.cc index 3e3e94d71..9c12177a4 100644 --- a/test/ddprof_exporter-ut.cc +++ b/test/ddprof_exporter-ut.cc @@ -160,7 +160,7 @@ TEST(DDProfExporter, simple) { res = pprof_create_profile(&pprofs, &ctx); EXPECT_TRUE(IsDDResOK(res)); - res = pprof_aggregate(&mock_output, &symbol_hdr, 1000, 1, &ctx.watchers[0], + res = pprof_aggregate(&mock_output, symbol_hdr, 1000, 1, &ctx.watchers[0], &pprofs); EXPECT_TRUE(IsDDResOK(res)); } diff --git a/test/ddprof_pprof-ut.cc b/test/ddprof_pprof-ut.cc index 31f16b464..d8f5db010 100644 --- a/test/ddprof_pprof-ut.cc +++ b/test/ddprof_pprof-ut.cc @@ -74,7 +74,7 @@ TEST(DDProfPProf, aggregate) { ctx.num_watchers = 1; DDRes res = pprof_create_profile(&pprof, &ctx); EXPECT_TRUE(IsDDResOK(res)); - res = pprof_aggregate(&mock_output, &symbol_hdr, 1000, 1, &ctx.watchers[0], + res = pprof_aggregate(&mock_output, symbol_hdr, 1000, 1, &ctx.watchers[0], &pprof); EXPECT_TRUE(IsDDResOK(res)); From d912593dbc73dad6224aa64fdf8a7069c498b6d0 Mon Sep 17 00:00:00 2001 From: r1viollet Date: Fri, 7 Apr 2023 16:04:04 +0200 Subject: [PATCH 17/21] Remove check on liveness of process --- src/ddprof_worker.cc | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/ddprof_worker.cc b/src/ddprof_worker.cc index c7c820320..534da4e91 100644 --- a/src/ddprof_worker.cc +++ b/src/ddprof_worker.cc @@ -16,7 +16,6 @@ #include "pevent_lib.hpp" #include "pprof/ddprof_pprof.hpp" #include "procutils.hpp" -#include "signal_helper.hpp" #include "stack_handler.hpp" #include "tags.hpp" #include "timer.hpp" @@ -576,9 +575,7 @@ static DDRes clear_unvisited_pids(DDProfContext *ctx) { UnwindState *us = ctx->worker_ctx.us; const std::vector pids_remove = us->dwfl_hdr.get_unvisited(); for (pid_t el : pids_remove) { - if (!process_is_alive(el)) { - DDRES_CHECK_FWD(worker_pid_free(ctx, el)); - } + DDRES_CHECK_FWD(worker_pid_free(ctx, el)); } us->dwfl_hdr.reset_unvisited(); return ddres_init(); From 4f8e1375e648c97b1739e4efc0fbd3d1af69dbb8 Mon Sep 17 00:00:00 2001 From: r1viollet Date: Fri, 7 Apr 2023 16:22:23 +0200 Subject: [PATCH 18/21] Split system allocation profiling to a different pull request --- include/ddprof_worker_context.hpp | 2 - include/live_sysallocations.hpp | 98 ---------------------- include/perf_watcher.hpp | 9 -- src/ddprof_cmdline.cc | 27 +----- src/ddprof_context_lib.cc | 10 --- src/ddprof_worker.cc | 135 +----------------------------- src/pevent_lib.cc | 92 +------------------- 7 files changed, 5 insertions(+), 368 deletions(-) delete mode 100644 include/live_sysallocations.hpp diff --git a/include/ddprof_worker_context.hpp b/include/ddprof_worker_context.hpp index 344386c5a..f7c969ace 100644 --- a/include/ddprof_worker_context.hpp +++ b/include/ddprof_worker_context.hpp @@ -6,7 +6,6 @@ #pragma once #include "live_allocation.hpp" -#include "live_sysallocations.hpp" #include "pevent.hpp" #include "proc_status.hpp" @@ -39,5 +38,4 @@ struct DDProfWorkerContext { uint32_t count_worker; // exports since last cache clear std::array lost_events_per_watcher; ddprof::LiveAllocation live_allocation; - ddprof::SystemAllocation sys_allocation; }; diff --git a/include/live_sysallocations.hpp b/include/live_sysallocations.hpp deleted file mode 100644 index b6bde8896..000000000 --- a/include/live_sysallocations.hpp +++ /dev/null @@ -1,98 +0,0 @@ -#pragma once - -#include "ddprof_defs.hpp" -#include "logger.hpp" -#include "unwind_output.hpp" - -#include -#include - -#include - -namespace ddprof { - -class SystemAllocation { -private: - template T to_page(T a) { - return ((a + T{4095ull}) & (~T{4095ull})) >> T{12ull}; - } - -public: - void add_allocs(const UnwindOutput &stack, uintptr_t addr, size_t size, - pid_t pid) { - StackMap &stack_map = _pid_map[pid]; - - // Convert addr to page idx, then page-align size and decimate - uintptr_t page_start = to_page(addr); - uintptr_t page_end = to_page(addr + size); - - for (auto i = page_start; i <= page_end; ++i) { - stack_map[i] = stack; - } - _visited_recently.insert(pid); - } - - void move_allocs(uintptr_t addr0, uintptr_t addr1, size_t size, pid_t pid) { - StackMap &stack_map = _pid_map[pid]; - - // Convert addr to page idx - uintptr_t page_start_0 = to_page(addr0); - uintptr_t page_end_0 = to_page(addr0 + size); - uintptr_t page_start_1 = to_page(addr1); - uintptr_t page_idx_max = page_end_0 - page_start_0; - - // Can ranges overlap? Better not try to delete them all at end... - for (uintptr_t i = 0; i < page_idx_max; ++i) { - stack_map[page_start_1 + i] = stack_map[page_start_0 + i]; - stack_map.erase(page_start_0 + i); - } - _visited_recently.insert(pid); - } - - void del_allocs(uintptr_t addr, size_t size, pid_t pid) { - StackMap &stack_map = _pid_map[pid]; - - // Convert addr to page idx, then page-align size and decimate - uintptr_t page_start = to_page(addr); - uintptr_t page_end = to_page(addr + size); - - for (auto i = page_start; i <= page_end; ++i) { - stack_map.erase(i); - } - _visited_recently.insert(pid); - } - - void do_mmap(const UnwindOutput &stack, uintptr_t addr, size_t size, - pid_t pid) { - add_allocs(stack, addr, size, pid); - } - - void do_munmap(uintptr_t addr, size_t size, pid_t pid) { - del_allocs(addr, size, pid); - } - - void do_madvise(uintptr_t addr, size_t size, int flags, pid_t pid) { - // No reason to worry about this yet, since it only has to do with RSS - } - - void do_mremap(const UnwindOutput &stack, uintptr_t addr0, uintptr_t addr1, - size_t size0, size_t size1, pid_t pid) { - // We could either classify these pages as belonging to the original mmap - // or to the mremap. We chose the latter for now. - // Note that we potentially duplicate a lot of work here in the case - // that addr0 == addr1 - del_allocs(addr0, size0, pid); - add_allocs(stack, addr1, size1, pid); - } - - void clear_pid(pid_t pid) { _pid_map.erase(pid); } - - using StackMap = std::unordered_map; - using PidMap = std::unordered_map; - - PidMap _pid_map; - std::unordered_set _visited_recently; - int watcher_pos; -}; - -} // namespace ddprof diff --git a/include/perf_watcher.hpp b/include/perf_watcher.hpp index 0685b5970..0f9ae8f98 100644 --- a/include/perf_watcher.hpp +++ b/include/perf_watcher.hpp @@ -24,9 +24,6 @@ struct PerfWatcherOptions { uint8_t nb_frames_to_skip; // number of bottom frames to skip in stack trace // (useful for allocation profiling to remove // frames belonging to lib_ddprofiling.so) - bool is_overloaded; // Isn't actually needed, but makes it clear from this - // file that additional state is injected into the - // watcher in ddprof_cmdline.cc }; struct PerfWatcher { @@ -86,7 +83,6 @@ enum DDProfTypeId { kDDPROF_TYPE_CUSTOM = PERF_TYPE_MAX + 100 }; enum DDProfCustomCountId { kDDPROF_COUNT_ALLOCATIONS = 0, - kDDPROF_COUNT_SYSALLOCATIONS, }; // Kernel events are necessary to get a full accounting of CPU @@ -110,9 +106,6 @@ enum DDProfCustomCountId { #define SKIP_FRAMES \ { .nb_frames_to_skip = NB_FRAMES_TO_SKIP } -#define IS_OVERLOADED \ - { .is_overloaded = true } - // Whereas tracepoints are dynamically configured and can be checked at runtime, // we lack the ability to inspect events of type other than TYPE_TRACEPOINT. // Accordingly, we maintain a list of events, even though the type of these @@ -141,8 +134,6 @@ enum DDProfCustomCountId { X(sALGN, "Align. Faults", PERF_TYPE_SOFTWARE, PERF_COUNT_SW_ALIGNMENT_FAULTS, 99, DDPROF_PWT_TRACEPOINT, IS_FREQ) \ X(sEMU, "Emu. Faults", PERF_TYPE_SOFTWARE, PERF_COUNT_SW_EMULATION_FAULTS, 99, DDPROF_PWT_TRACEPOINT, IS_FREQ) \ X(sDUM, "Dummy", PERF_TYPE_SOFTWARE, PERF_COUNT_SW_DUMMY, 1, DDPROF_PWT_NOCOUNT, {}) \ - X(tALLOCSYS1, "System Allocations", PERF_TYPE_TRACEPOINT, kDDPROF_COUNT_SYSALLOCATIONS, 1, DDPROF_PWT_ALLOC_SPACE, IS_OVERLOADED) \ - X(tALLOCSYS2, "System Al. (heavy)", PERF_TYPE_TRACEPOINT, kDDPROF_COUNT_SYSALLOCATIONS, 1, DDPROF_PWT_ALLOC_SPACE, IS_OVERLOADED) \ X(sALLOC, "Allocations", kDDPROF_TYPE_CUSTOM, kDDPROF_COUNT_ALLOCATIONS, 524288, DDPROF_PWT_ALLOC_SPACE, SKIP_FRAMES) // clang-format on diff --git a/src/ddprof_cmdline.cc b/src/ddprof_cmdline.cc index d616d3a82..08662cc73 100644 --- a/src/ddprof_cmdline.cc +++ b/src/ddprof_cmdline.cc @@ -131,35 +131,10 @@ bool watcher_from_str(const char *str, PerfWatcher *watcher) { watcher->tracepoint_group = conf->groupname; watcher->tracepoint_label = conf->label; - // Certain watcher configs get additional event information + // Allocation watcher, has an extra field to ensure we capture address if (watcher->config == kDDPROF_COUNT_ALLOCATIONS) { watcher->sample_type |= PERF_SAMPLE_ADDR; } - // Some profiling types get lots of additional state transplanted here - if (watcher->options.is_overloaded) { - if (watcher->ddprof_event_type == DDPROF_PWE_tALLOCSYS1) { - // tALLOCSY1 overrides perfopen to bind together many file descriptors - watcher->tracepoint_group = "syscalls"; - watcher->tracepoint_label = "sys_exit_mmap"; - watcher->instrument_self = true; - watcher->options.use_kernel = PerfWatcherUseKernel::kTry; - watcher->sample_stack_size /= 2; // Make this one smaller than normal - - } else if (watcher->ddprof_event_type == DDPROF_PWE_tALLOCSYS2) { - // tALLOCSYS2 captures all syscalls; used to troubleshoot 1 - watcher->tracepoint_group = "raw_syscalls"; - watcher->tracepoint_label = "sys_exit"; - long id = ddprof::tracepoint_get_id("raw_syscalls", "sys_exit"); - if (-1 == id) { - // We mutated the user's event, but it is invalid. - return false; - } - watcher->config = id; - } - watcher->sample_type |= PERF_SAMPLE_RAW; - watcher->options.use_kernel = PerfWatcherUseKernel::kTry; - } - return true; } diff --git a/src/ddprof_context_lib.cc b/src/ddprof_context_lib.cc index 9d61a4596..37b64c335 100644 --- a/src/ddprof_context_lib.cc +++ b/src/ddprof_context_lib.cc @@ -120,16 +120,6 @@ DDRes ddprof_context_set(DDProfInput *input, DDProfContext *ctx) { } ctx->num_watchers = nwatchers; - // Some profiling features, like system allocations, uses ctx storage and - // needs to associate a watcher (but only one watcher) to that storage. - for (int i = 0; i < ctx->num_watchers; ++i) { - if (ctx->watchers[i].ddprof_event_type == DDPROF_PWE_tALLOCSYS1 || - ctx->watchers[i].ddprof_event_type == DDPROF_PWE_tALLOCSYS2) { - ctx->worker_ctx.sys_allocation.watcher_pos = i; - break; - } - } - // Set defaults ctx->params.upload_period = 60.0; diff --git a/src/ddprof_worker.cc b/src/ddprof_worker.cc index 534da4e91..ceaa9c9ea 100644 --- a/src/ddprof_worker.cc +++ b/src/ddprof_worker.cc @@ -346,77 +346,6 @@ DDRes ddprof_pr_sample(DDProfContext *ctx, perf_event_sample *sample, return {}; } -DDRes ddprof_pr_sysallocation_tracking(DDProfContext *ctx, - perf_event_sample *sample, - int watcher_pos, bool &clear_pid) { - clear_pid = false; - // Syscall parameters. Suppressing nags because it's annoying to look these - // up and it isn't totally appropriate to spin out a new header just - // for this - int64_t id; - memcpy(&id, sample->data_raw + 8, sizeof(id)); - auto &sysalloc = ctx->worker_ctx.sys_allocation; - -#ifdef __x86_64__ - [[maybe_unused]] uint64_t sc_ret = sample->regs[PAM_X86_RAX]; - [[maybe_unused]] uint64_t sc_p1 = sample->regs[PAM_X86_RDI]; - [[maybe_unused]] uint64_t sc_p2 = sample->regs[PAM_X86_RSI]; - [[maybe_unused]] uint64_t sc_p3 = sample->regs[PAM_X86_RDX]; - [[maybe_unused]] uint64_t sc_p4 = sample->regs[PAM_X86_R10]; - [[maybe_unused]] uint64_t sc_p5 = sample->regs[PAM_X86_R8]; - [[maybe_unused]] uint64_t sc_p6 = sample->regs[PAM_X86_R9]; -#elif __aarch64__ - // Obviously ARM is totally broken here. - [[maybe_unused]] uint64_t sc_ret = sample->regs[PAM_ARM_X0]; - [[maybe_unused]] uint64_t sc_p1 = sample->regs[PAM_ARM_X0]; - [[maybe_unused]] uint64_t sc_p2 = sample->regs[PAM_ARM_X1]; - [[maybe_unused]] uint64_t sc_p3 = sample->regs[PAM_ARM_X2]; - [[maybe_unused]] uint64_t sc_p4 = sample->regs[PAM_ARM_X3]; - [[maybe_unused]] uint64_t sc_p5 = sample->regs[PAM_ARM_X4]; - [[maybe_unused]] uint64_t sc_p6 = sample->regs[PAM_ARM_X5]; -#else -# error Architecture not supported -#endif - if (sc_ret > -4096UL) { - // If the syscall returned error, it didn't mutate state. Skip! - // ("high" values are errors, as per standard) - return ddres_init(); - } - - // Only unwind if we will need to propagate unwinding information forward - DDRes res = {}; - UnwindOutput *uwo = NULL; - if (id == 9 || id == 25) { - auto ticks0 = ddprof::get_tsc_cycles(); - res = ddprof_unwind_sample(ctx, sample, watcher_pos); - auto unwind_ticks = ddprof::get_tsc_cycles(); - ddprof_stats_add(STATS_UNWIND_AVG_TIME, unwind_ticks - ticks0, NULL); - uwo = &ctx->worker_ctx.us->output; - - // TODO: propagate fatal - if (IsDDResFatal(res)) { - return ddres_init(); - } - } - - // hardcoded syscall numbers; these are uniform between x86/arm - if (id == 9) { - sysalloc.do_mmap(*uwo, sc_ret, sc_p2, sample->pid); - } else if (id == 11) { - sysalloc.do_munmap(sc_p1, sc_p2, sample->pid); - } else if (id == 28) { - // Unhandled, no need to handle - } else if (id == 25) { - sysalloc.do_mremap(*uwo, sc_ret, sc_p1, sc_p2, sc_p3, sample->pid); - } else if (id == 60 || id == 231 || id == 59 || id == 322 || id == 520 || - id == 545) { - // Erase upon exit or exec - clear_pid = true; - } - - return ddres_init(); -} - static void ddprof_reset_worker_stats() { for (unsigned i = 0; i < std::size(s_cycled_stats); ++i) { ddprof_stats_clear(s_cycled_stats[i]); @@ -509,55 +438,11 @@ static DDRes aggregate_live_allocations(DDProfContext *ctx) { return ddres_init(); } -DDRes aggregate_sys_allocation_stack(const UnwindOutput *uw_output, - const SymbolHdr *symbol_hdr, - const PerfWatcher *watcher, - DDProfPProf *pprof) { - DDRES_CHECK_FWD(pprof_aggregate(uw_output, *symbol_hdr, get_page_size(), 1, - watcher, pprof)); - return ddres_init(); -} - -static DDRes aggregate_sys_allocations_for_pid(DDProfContext *ctx, pid_t pid) { - struct UnwindState *us = ctx->worker_ctx.us; - SystemAllocation &sysallocs = ctx->worker_ctx.sys_allocation; - PerfWatcher *watcher = &ctx->watchers[sysallocs.watcher_pos]; - int i_export = ctx->worker_ctx.i_current_pprof; - DDProfPProf *pprof = ctx->worker_ctx.pprof[i_export]; - const auto &stack_map = sysallocs._pid_map[pid]; - for (const auto &page : stack_map) { - DDRES_CHECK_FWD(aggregate_sys_allocation_stack( - &page.second, &us->symbol_hdr, watcher, pprof)); - } - return ddres_init(); -} - -static DDRes aggregate_sys_allocations(DDProfContext *ctx) { - struct UnwindState *us = ctx->worker_ctx.us; - SystemAllocation &sysallocs = ctx->worker_ctx.sys_allocation; - PerfWatcher *watcher = &ctx->watchers[sysallocs.watcher_pos]; - int i_export = ctx->worker_ctx.i_current_pprof; - DDProfPProf *pprof = ctx->worker_ctx.pprof[i_export]; - - // Iterate through each PID - for (auto &stack_map : sysallocs._pid_map) { - // Iterate through pages... - // TODO Probably aggregate into ranges of pages or something, but once per - // page is just too much - for (const auto &page : stack_map.second) { - DDRES_CHECK_FWD(aggregate_sys_allocation_stack( - &page.second, &us->symbol_hdr, watcher, pprof)); - } - } - return ddres_init(); -} static DDRes worker_pid_free(DDProfContext *ctx, pid_t el) { - DDRES_CHECK_FWD(aggregate_sys_allocations_for_pid(ctx, el)); DDRES_CHECK_FWD(aggregate_live_allocations_for_pid(ctx, el)); UnwindState *us = ctx->worker_ctx.us; unwind_pid_free(us, el); - ctx->worker_ctx.sys_allocation.clear_pid(el); ctx->worker_ctx.live_allocation.clear_pid(el); return ddres_init(); } @@ -565,7 +450,6 @@ static DDRes worker_pid_free(DDProfContext *ctx, pid_t el) { static DDRes worker_pid_free(DDProfContext *ctx, pid_t el) { UnwindState *us = ctx->worker_ctx.us; unwind_pid_free(us, el); - ctx->worker_ctx.sys_allocation.clear_pid(el); ctx->worker_ctx.live_allocation.clear_pid(el); return ddres_init(); } @@ -589,7 +473,6 @@ DDRes ddprof_worker_cycle(DDProfContext *ctx, int64_t now, DDRES_CHECK_FWD(clear_unvisited_pids(ctx)); #ifndef DDPROF_NATIVE_LIB DDRES_CHECK_FWD(aggregate_live_allocations(ctx)); - DDRES_CHECK_FWD(aggregate_sys_allocations(ctx)); // Take the current pprof contents and ship them to the backend. This also // clears the pprof for reuse @@ -855,24 +738,8 @@ DDRes ddprof_worker_process_event(const perf_event_header *hdr, int watcher_pos, if (wpid->pid) { uint64_t mask = watcher->sample_type; perf_event_sample *sample = hdr2samp(hdr, mask); - if (sample) { - // Handle special profiling types first - if (watcher->ddprof_event_type == DDPROF_PWE_tALLOCSYS1 || - watcher->ddprof_event_type == DDPROF_PWE_tALLOCSYS2) { - // For now we have a different path for - // - mmap/munmap syscalls - bool clear_pid = false; - DDRES_CHECK_FWD(ddprof_pr_sysallocation_tracking( - ctx, sample, watcher_pos, clear_pid)); - if (clear_pid) { - LG_DBG("<%d>(SYSEXIT)%d", watcher_pos, wpid->pid); - // we could consider clearing the pid here - // though we could get other types of events - } - } else { - DDRES_CHECK_FWD(ddprof_pr_sample(ctx, sample, watcher_pos)); - } + DDRES_CHECK_FWD(ddprof_pr_sample(ctx, sample, watcher_pos)); } } break; diff --git a/src/pevent_lib.cc b/src/pevent_lib.cc index bd48df60f..b164c3742 100644 --- a/src/pevent_lib.cc +++ b/src/pevent_lib.cc @@ -65,85 +65,6 @@ static void pevent_set_info(int fd, int attr_idx, PEvent &pevent) { pevent.attr_idx = attr_idx; } -static void pevent_add_child_fd(int child_fd, PEvent &pevent) { - pevent.child_fds[pevent.current_child_fd++] = child_fd; -} - -static DDRes tallocsys1_open(PerfWatcher *watcher, int watcher_idx, pid_t pid, - int num_cpu, PEventHdr *pevent_hdr) { - PerfWatcher watcher_copy = *watcher; - PEvent *pes = pevent_hdr->pes; - - struct talloc_conf { - int fd; - bool enable_userstack; - }; - std::unordered_map kprobes{ - {"sys_exit_mmap", {-1, true}}, - {"sys_exit_munmap", {-1, false}}, - {"sys_exit_mremap", {-1, true}}}; - - // Set the IDs - for (auto &kprobe : kprobes) { - long id = ddprof::tracepoint_get_id("syscalls", kprobe.first); - if (-1 == id) { - DDRES_RETURN_ERROR_LOG(DD_WHAT_PERFOPEN, - "Error opening tracefs for tALLOCSYS1 on %s", - kprobe.first.c_str()); - } - kprobes[kprobe.first].fd = id; - } - - // Iterate - for (int cpu_idx = 0; cpu_idx < num_cpu; ++cpu_idx) { - int fd = -1; - // Create the pevent which will consolidate this watcher - size_t pevent_idx = -1; - DDRES_CHECK_FWD(pevent_create(pevent_hdr, watcher_idx, &pevent_idx)); - perf_event_attr attr = {}; - std::vector cleanup_fds; - - // This is very imperfect since failure leaves a dangling pevent - // TODO - for (auto &kprobe : kprobes) { - watcher_copy.tracepoint_group = "syscalls"; - watcher_copy.tracepoint_label = kprobe.first.c_str(); - watcher_copy.config = kprobe.second.fd; - - // THIS IS WRONG - if (kprobe.second.enable_userstack) { - watcher_copy.sample_stack_size = watcher->sample_stack_size; - } else { - watcher_copy.sample_stack_size = 0; - } - - attr = perf_config_from_watcher(&watcher_copy, true); - int fd_tmp = -1; - fd_tmp = perf_event_open(&attr, pid, cpu_idx, -1, PERF_FLAG_FD_CLOEXEC); - - if (-1 == fd_tmp) { - for (auto cleanup_fd : cleanup_fds) { - close(cleanup_fd); - } - DDRES_RETURN_ERROR_LOG(DD_WHAT_PERFOPEN, - "Error calling perfopen for tALLOCSYS1 on %s", - kprobe.first.c_str()); - } - if (-1 != fd) { - pevent_add_child_fd(fd_tmp, pes[pevent_idx]); - } else { - fd = fd_tmp; - } - cleanup_fds.push_back(fd_tmp); - } - pevent_hdr->attrs[pevent_hdr->nb_attrs] = attr; - pevent_set_info(fd, pes[pevent_idx].attr_idx, pes[pevent_idx]); - ++pevent_hdr->nb_attrs; - } - - return ddres_init(); -} - static DDRes pevent_register_cpu_0(const PerfWatcher *watcher, int watcher_idx, pid_t pid, PEventHdr *pevent_hdr, size_t &pevent_idx) { @@ -213,16 +134,9 @@ DDRes pevent_open(DDProfContext *ctx, pid_t pid, int num_cpu, assert(pevent_hdr->size == 0); // check for previous init for (int watcher_idx = 0; watcher_idx < ctx->num_watchers; ++watcher_idx) { PerfWatcher *watcher = &ctx->watchers[watcher_idx]; - if (watcher->instrument_self) { - // Here we inline a lookup for the specific handler, but in reality this - // should be defined at the level of the watcher - if (watcher->ddprof_event_type == DDPROF_PWE_tALLOCSYS1) { - DDRES_CHECK_FWD( - tallocsys1_open(watcher, watcher_idx, pid, num_cpu, pevent_hdr)); - } - } else if (watcher->type < kDDPROF_TYPE_CUSTOM) { - DDRES_CHECK_FWD( - pevent_open_all_cpus(watcher, watcher_idx, pid, num_cpu, pevent_hdr)); + if (watcher->type < kDDPROF_TYPE_CUSTOM) { + DDRES_CHECK_FWD(pevent_open_all_cpus( + &ctx->watchers[watcher_idx], watcher_idx, pid, num_cpu, pevent_hdr)); } else { // custom event, eg.allocation profiling size_t pevent_idx = 0; From ac42a3bcc89a9efbc08314bd04ee345f398d59ee Mon Sep 17 00:00:00 2001 From: r1viollet Date: Tue, 11 Apr 2023 11:40:23 +0200 Subject: [PATCH 19/21] Live allocation - unique stacks Unify the stacks when aggregating live allocations. --- include/live_allocation.hpp | 70 +++++++++++++++++--------- include/unwind_output.hpp | 5 +- include/unwind_output_hash.hpp | 29 +++++++++++ src/ddprof_worker.cc | 75 +++++++++++++--------------- src/live_allocation.cc | 82 +++++++++++++++++++++++++++++++ test/CMakeLists.txt | 2 + test/live_allocation-ut.cc | 89 ++++++++++++++++++++++++++++++++++ 7 files changed, 285 insertions(+), 67 deletions(-) create mode 100644 include/unwind_output_hash.hpp create mode 100644 src/live_allocation.cc create mode 100644 test/live_allocation-ut.cc diff --git a/include/live_allocation.hpp b/include/live_allocation.hpp index 51887d5fb..947fe3f4f 100644 --- a/include/live_allocation.hpp +++ b/include/live_allocation.hpp @@ -6,62 +6,84 @@ #pragma once #include "ddprof_defs.hpp" -#include "logger.hpp" -#include "unwind_output.hpp" #include "unlikely.hpp" +#include "unwind_output_hash.hpp" #include namespace ddprof { template -T& access_resize(std::vector& v, size_t index, const T& default_value = T()) { +T &access_resize(std::vector &v, size_t index, + const T &default_value = T()) { if (unlikely(index >= v.size())) { v.resize(index + 1, default_value); } return v[index]; } - class LiveAllocation { public: - void register_allocation(const UnwindOutput &stack, uintptr_t addr, - size_t size, int watcher_pos, pid_t pid) { + // For allocations Value is the size + // This is the cumulative value and count for a given stack + struct ValueAndCount { + int64_t _value = 0; + int64_t _count = 0; + }; + + using PprofStacks = + std::unordered_map; + struct ValuePerAddress { + int64_t _value = 0; + PprofStacks::value_type *_unique_stack = nullptr; + }; + + using AddressMap = std::unordered_map; + struct PidStacks { + AddressMap _address_map; + PprofStacks _unique_stacks; + }; + + using PidMap = std::unordered_map; + using WatcherVector = std::vector; + WatcherVector _watcher_vector; + + // Allocation should be aggregated per stack trace + // instead of a stack, we would have a total size for this unique stack trace + // and a count. + void register_allocation(const UnwindOutput &uo, uintptr_t addr, size_t size, + int watcher_pos, pid_t pid) { PidMap &pid_map = access_resize(_watcher_vector, watcher_pos); - StackMap &stack_map = pid_map[pid]; - stack_map[addr] = AllocationInfo{ - ._stack = stack, ._size = size, ._watcher_pos = watcher_pos}; + PidStacks &pid_stacks = pid_map[pid]; + register_allocation(uo, addr, size, pid_stacks._unique_stacks, + pid_stacks._address_map); } void register_deallocation(uintptr_t addr, int watcher_pos, pid_t pid) { PidMap &pid_map = access_resize(_watcher_vector, watcher_pos); - StackMap &stack_map = pid_map[pid]; - if (!stack_map.erase(addr)) { - LG_DBG("Unmatched deallocation at %lx of PID%d", addr, pid); - } + PidStacks &pid_stacks = pid_map[pid]; + register_deallocation(addr, pid_stacks._unique_stacks, + pid_stacks._address_map); } void clear_pid_for_watcher(int watcher_pos, pid_t pid) { PidMap &pid_map = access_resize(_watcher_vector, watcher_pos); - pid_map[pid].clear(); + pid_map.erase(pid); } void clear_pid(pid_t pid) { for (auto &pid_map : _watcher_vector) { - pid_map[pid].clear(); + pid_map.erase(pid); } } - struct AllocationInfo { - UnwindOutput _stack; - size_t _size; - int _watcher_pos; - }; +private: + static void register_deallocation(uintptr_t address, PprofStacks &stacks, + AddressMap &address_map); - using StackMap = std::unordered_map; - using PidMap = std::unordered_map; - using WatcherVector = std::vector; - WatcherVector _watcher_vector; + static void register_allocation(const UnwindOutput &uo, uintptr_t address, + int64_t value, PprofStacks &stacks, + AddressMap &address_map); }; } // namespace ddprof \ No newline at end of file diff --git a/include/unwind_output.hpp b/include/unwind_output.hpp index c408432c7..1076e48d6 100644 --- a/include/unwind_output.hpp +++ b/include/unwind_output.hpp @@ -11,12 +11,13 @@ #include #include "ddprof_defs.hpp" -#include "string_view.hpp" typedef struct FunLoc { uint64_t ip; // Relative to file, not VMA SymbolIdx_t _symbol_idx; MapInfoIdx_t _map_info_idx; + + auto operator<=>(const FunLoc &) const = default; } FunLoc; struct UnwindOutput { @@ -28,4 +29,6 @@ struct UnwindOutput { int pid; int tid; bool is_incomplete; + + auto operator<=>(const UnwindOutput &) const = default; }; diff --git a/include/unwind_output_hash.hpp b/include/unwind_output_hash.hpp new file mode 100644 index 000000000..604dbabb5 --- /dev/null +++ b/include/unwind_output_hash.hpp @@ -0,0 +1,29 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. This product includes software +// developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present +// Datadog, Inc. + +#include "unwind_output.hpp" + +namespace ddprof { + +template inline void hash_combine(std::size_t &seed, const T &v) { + std::hash hasher; + seed ^= hasher(v) + 0x9e3779b9 + (seed << 6) + (seed >> 2); +} + +struct UnwindOutputHash { + std::size_t operator()(const UnwindOutput &uo) const noexcept { + std::size_t seed = 0; + hash_combine(seed, uo.pid); + hash_combine(seed, uo.tid); + for (const auto &fl : uo.locs) { + hash_combine(seed, fl.ip); + hash_combine(seed, fl._symbol_idx); + hash_combine(seed, fl._map_info_idx); + } + return seed; + } +}; + +} // namespace ddprof \ No newline at end of file diff --git a/src/ddprof_worker.cc b/src/ddprof_worker.cc index ceaa9c9ea..b56ccde53 100644 --- a/src/ddprof_worker.cc +++ b/src/ddprof_worker.cc @@ -374,16 +374,15 @@ void *ddprof_worker_export_thread(void *arg) { #endif #ifndef DDPROF_NATIVE_LIB -static DDRes -aggregate_livealloc_stack(const LiveAllocation::AllocationInfo &alloc_info, - DDProfContext *ctx, - const PerfWatcher *watcher, - DDProfPProf *pprof, - const SymbolHdr &symbol_hdr) { - DDRES_CHECK_FWD(pprof_aggregate(&alloc_info._stack, symbol_hdr, - alloc_info._size, 1, watcher, pprof)); +static DDRes aggregate_livealloc_stack( + const LiveAllocation::PprofStacks::value_type &alloc_info, + DDProfContext *ctx, const PerfWatcher *watcher, DDProfPProf *pprof, + const SymbolHdr &symbol_hdr) { + DDRES_CHECK_FWD(pprof_aggregate(&alloc_info.first, symbol_hdr, + alloc_info.second._value, + alloc_info.second._count, watcher, pprof)); if (ctx->params.show_samples) { - ddprof_print_sample(alloc_info._stack, symbol_hdr, alloc_info._size, + ddprof_print_sample(alloc_info.first, symbol_hdr, alloc_info.second._value, *watcher); } return ddres_init(); @@ -399,10 +398,10 @@ static DDRes aggregate_live_allocations_for_pid(DDProfContext *ctx, pid_t pid) { watcher_pos < live_allocations._watcher_vector.size(); ++watcher_pos) { auto &pid_map = live_allocations._watcher_vector[watcher_pos]; const PerfWatcher *watcher = &ctx->watchers[watcher_pos]; - auto &stack_map = pid_map[pid]; - for (const auto &alloc_info_pair : stack_map) { - DDRES_CHECK_FWD(aggregate_livealloc_stack(alloc_info_pair.second, ctx, - watcher, pprof, symbol_hdr)); + auto &pid_stacks = pid_map[pid]; + for (const auto &alloc_info : pid_stacks._unique_stacks) { + DDRES_CHECK_FWD(aggregate_livealloc_stack(alloc_info, ctx, watcher, pprof, + symbol_hdr)); } } return ddres_init(); @@ -411,34 +410,28 @@ static DDRes aggregate_live_allocations_for_pid(DDProfContext *ctx, pid_t pid) { static DDRes aggregate_live_allocations(DDProfContext *ctx) { // this would be more efficient if we could reuse the same stacks in // libdatadog - struct UnwindState *us = ctx->worker_ctx.us; int i_export = ctx->worker_ctx.i_current_pprof; DDProfPProf *pprof = ctx->worker_ctx.pprof[i_export]; const SymbolHdr &symbol_hdr = us->symbol_hdr; - LiveAllocation &live_allocations = ctx->worker_ctx.live_allocation; - + const LiveAllocation &live_allocations = ctx->worker_ctx.live_allocation; for (unsigned watcher_pos = 0; watcher_pos < live_allocations._watcher_vector.size(); ++watcher_pos) { - auto &pid_map = live_allocations._watcher_vector[watcher_pos]; + const auto &pid_map = live_allocations._watcher_vector[watcher_pos]; const PerfWatcher *watcher = &ctx->watchers[watcher_pos]; - for (auto &stack_map : pid_map) { - for (const auto &alloc_info_pair : stack_map.second) { - DDRES_CHECK_FWD(aggregate_livealloc_stack(alloc_info_pair.second, ctx, - watcher, - pprof, - symbol_hdr)); + for (const auto &pid_vt : pid_map) { + for (const auto &alloc_info : pid_vt.second._unique_stacks) { + DDRES_CHECK_FWD(aggregate_livealloc_stack(alloc_info, ctx, watcher, + pprof, symbol_hdr)); } - LG_NTC("<%u> Number of Live allocations for PID%d = %lu ", - watcher_pos, - stack_map.first, - stack_map.second.size()); + LG_NTC("<%u> Number of Live allocations for PID%d=%lu, Unique stacks=%lu", + watcher_pos, pid_vt.first, pid_vt.second._address_map.size(), + pid_vt.second._unique_stacks.size()); } } return ddres_init(); } - static DDRes worker_pid_free(DDProfContext *ctx, pid_t el) { DDRES_CHECK_FWD(aggregate_live_allocations_for_pid(ctx, el)); UnwindState *us = ctx->worker_ctx.us; @@ -622,10 +615,9 @@ void ddprof_pr_clear_live_allocation(DDProfContext *ctx, event->sample_id.pid); } -void ddprof_pr_deallocation(DDProfContext *ctx, - const DeallocationEvent *event, int watcher_pos) { - ctx->worker_ctx.live_allocation.register_deallocation(event->ptr, - watcher_pos, +void ddprof_pr_deallocation(DDProfContext *ctx, const DeallocationEvent *event, + int watcher_pos) { + ctx->worker_ctx.live_allocation.register_deallocation(event->ptr, watcher_pos, event->sample_id.pid); } @@ -771,19 +763,18 @@ DDRes ddprof_worker_process_event(const perf_event_header *hdr, int watcher_pos, watcher_pos); break; case PERF_CUSTOM_EVENT_DEALLOCATION: - ddprof_pr_deallocation(ctx, - reinterpret_cast(hdr), - watcher_pos); + ddprof_pr_deallocation( + ctx, reinterpret_cast(hdr), watcher_pos); break; - case PERF_CUSTOM_EVENT_CLEAR_LIVE_ALLOCATION: - { - const ClearLiveAllocationEvent* event = reinterpret_cast(hdr); + case PERF_CUSTOM_EVENT_CLEAR_LIVE_ALLOCATION: { + const ClearLiveAllocationEvent *event = + reinterpret_cast(hdr); #ifndef DDPROF_NATIVE_LIB - DDRES_CHECK_FWD(aggregate_live_allocations_for_pid(ctx, event->sample_id.pid)); + DDRES_CHECK_FWD( + aggregate_live_allocations_for_pid(ctx, event->sample_id.pid)); #endif - ddprof_pr_clear_live_allocation(ctx, event,watcher_pos); - } - break; + ddprof_pr_clear_live_allocation(ctx, event, watcher_pos); + } break; default: break; } diff --git a/src/live_allocation.cc b/src/live_allocation.cc new file mode 100644 index 000000000..d7d2b96a4 --- /dev/null +++ b/src/live_allocation.cc @@ -0,0 +1,82 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. This product includes software +// developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present +// Datadog, Inc. + +#include "live_allocation.hpp" + +#include "logger.hpp" + +namespace ddprof { + +void LiveAllocation::register_deallocation(uintptr_t address, + PprofStacks &stacks, + AddressMap &address_map) { + // Find the ValuePerAddress object corresponding to the address + auto map_iter = address_map.find(address); + if (map_iter == address_map.end()) { + // No element found, nothing to do + // This means we lost previous events, leading to de-sync between + // the state of the profiler and the state of the library. + LG_DBG("Unmatched de-allocation at %lx", address); + return; + } + ValuePerAddress &v = map_iter->second; + + // Decrement count and value of the corresponding PprofStacks::value_type + // object + if (v._unique_stack) { + v._unique_stack->second._value -= v._value; + if (v._unique_stack->second._count) { + --(v._unique_stack->second._count); + } + if (!v._unique_stack->second._count) { + // If count reaches 0, remove the UnwindOutput from stacks + stacks.erase(v._unique_stack->first); + } + } + + // Remove the element from the address map + address_map.erase(map_iter); +} + +void LiveAllocation::register_allocation(const UnwindOutput &uo, + uintptr_t address, int64_t value, + PprofStacks &stacks, + AddressMap &address_map) { + if (!uo.locs.size()) { + // avoid sending empty stacks + LG_DBG("(LIVE_ALLOC) Avoid registering empty stack"); + return; + } + // Find or create the PprofStacks::value_type object corresponding to the + // UnwindOutput + auto iter = stacks.find(uo); + if (iter == stacks.end()) { + iter = stacks.emplace(uo, ValueAndCount{}).first; + } + PprofStacks::value_type &unique_stack = *iter; + + // Add the value to the address map + ValuePerAddress &v = address_map[address]; + if (v._value) { + // unexpected, we already have an allocation here + // This means we missed a previous free + LG_DBG("Existing allocation: %lx (cleaning up)", address); + if (v._unique_stack) { + // we should decrement count / value + v._unique_stack->second._value -= v._value; + if (v._unique_stack->second._count) { + --(v._unique_stack->second._count); + } + // Should we erase the element here ? + // only if we are sure it is not the same as the one we are inserting. + } + } + + v._unique_stack = &unique_stack; + v._unique_stack->second._value += value; + ++(v._unique_stack->second._count); +} + +} // namespace ddprof diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index e94229276..4b35fe2f5 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -326,6 +326,8 @@ add_unit_test(jitdump-ut jitdump-ut.cc ../src/jit/jitdump.cc) add_unit_test(tracepoint_config-ut tracepoint_config-ut.cc ../src/tracepoint_config.cc) +add_unit_test(live_allocation-ut live_allocation-ut.cc ../src/live_allocation.cc) + add_benchmark(savecontext-bench savecontext-bench.cc ../src/lib/savecontext.cc ../src/lib/saveregisters.cc) diff --git a/test/live_allocation-ut.cc b/test/live_allocation-ut.cc new file mode 100644 index 000000000..84b8f3ced --- /dev/null +++ b/test/live_allocation-ut.cc @@ -0,0 +1,89 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. This product includes software +// developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present +// Datadog, Inc. + +#include "live_allocation.hpp" +#include "loghandle.hpp" + +#include + +namespace ddprof { + +TEST(LiveAllocationTest, simple) { + LogHandle handle; + UnwindOutput uo; + uo.pid = 123; + uo.tid = 456; + uo.is_incomplete = false; + uo.locs.push_back({0x1234, 0x5678, 0x9abc}); + uo.locs.push_back({0x4321, 0x8765, 0xcba9}); + + LiveAllocation live_alloc; + int watcher_pos = 0; + pid_t pid = 12; + int64_t value = 10; + int64_t nb_registered_allocs = 10; + { // allocate 10 + uintptr_t addr = 0x10; + for (int i=0; i Date: Tue, 11 Apr 2023 15:22:20 +0200 Subject: [PATCH 20/21] Live allocation - out of order deallocations Fix behaviour in case events arrive out of order for deallocations --- include/live_allocation.hpp | 3 +++ src/live_allocation.cc | 5 ++++ test/live_allocation-ut.cc | 48 ++++++++++++++++++++++++++++++++++--- 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/include/live_allocation.hpp b/include/live_allocation.hpp index 947fe3f4f..773a6e138 100644 --- a/include/live_allocation.hpp +++ b/include/live_allocation.hpp @@ -9,6 +9,8 @@ #include "unlikely.hpp" #include "unwind_output_hash.hpp" +#include +#include #include namespace ddprof { @@ -33,6 +35,7 @@ class LiveAllocation { using PprofStacks = std::unordered_map; + struct ValuePerAddress { int64_t _value = 0; PprofStacks::value_type *_unique_stack = nullptr; diff --git a/src/live_allocation.cc b/src/live_allocation.cc index d7d2b96a4..0ed4c3f47 100644 --- a/src/live_allocation.cc +++ b/src/live_allocation.cc @@ -71,9 +71,14 @@ void LiveAllocation::register_allocation(const UnwindOutput &uo, } // Should we erase the element here ? // only if we are sure it is not the same as the one we are inserting. + if (v._unique_stack != &unique_stack && + !v._unique_stack->second._count) { + stacks.erase(v._unique_stack->first); + } } } + v._value = value; v._unique_stack = &unique_stack; v._unique_stack->second._value += value; ++(v._unique_stack->second._count); diff --git a/test/live_allocation-ut.cc b/test/live_allocation-ut.cc index 84b8f3ced..45d8de645 100644 --- a/test/live_allocation-ut.cc +++ b/test/live_allocation-ut.cc @@ -82,8 +82,50 @@ TEST(LiveAllocationTest, invalid_inputs) { // Register deallocation with invalid address EXPECT_NO_THROW(live_alloc.register_deallocation(0, watcher_pos, pid)); } + + +TEST(LiveAllocationTest, overlap_registrations) { + LogHandle handle; + LiveAllocation live_alloc; + int watcher_pos = 0; + pid_t pid = 12; + int64_t value = 10; + UnwindOutput uo; + + uintptr_t addr = 0x10; + uo.pid = 123; + uo.tid = 456; + uo.is_incomplete = false; + uo.locs.push_back({0x1234, 0x5678, 0x9abc}); + + // Register the first allocation + live_alloc.register_allocation(uo, addr, value, watcher_pos, pid); + auto &pid_map = live_alloc._watcher_vector[0]; + auto &pid_stacks = pid_map[pid]; + EXPECT_EQ(pid_stacks._address_map.size(), 1); + EXPECT_EQ(pid_stacks._unique_stacks.size(), 1); + + // Register a second allocation at the same address + // elements can arrive out of order, so this can be expected + live_alloc.register_allocation(uo, addr, value * 2, watcher_pos, pid); + EXPECT_EQ(pid_stacks._address_map.size(), 1); + EXPECT_EQ(pid_stacks._unique_stacks.size(), 1); + + // Check that the value and count have the latest value + auto &el = pid_stacks._unique_stacks[uo]; + EXPECT_EQ(el._value, value * 2); + EXPECT_EQ(el._count, 1); + + // Deallocate the first allocation + live_alloc.register_deallocation(addr, watcher_pos, pid); + EXPECT_EQ(pid_stacks._address_map.size(), 0); + EXPECT_EQ(pid_stacks._unique_stacks.size(), 0); + + // Deallocate the second allocation + live_alloc.register_deallocation(addr, watcher_pos, pid); + EXPECT_EQ(pid_stacks._address_map.size(), 0); + EXPECT_EQ(pid_stacks._unique_stacks.size(), 0); +} + } -// Other cases to consider -// -- same address registered -// From c52180aa72c7bb58cf9f66459d5bebfc8506f926 Mon Sep 17 00:00:00 2001 From: r1viollet Date: Sun, 16 Apr 2023 16:28:43 +0200 Subject: [PATCH 21/21] Minor CI fixes - format code - ensure savecontext unit test does not fail --- include/live_allocation.hpp | 2 +- src/live_allocation.cc | 3 +-- test/live_allocation-ut.cc | 14 +++++++------- test/savecontext-ut.cc | 2 +- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/include/live_allocation.hpp b/include/live_allocation.hpp index 773a6e138..f19ff339b 100644 --- a/include/live_allocation.hpp +++ b/include/live_allocation.hpp @@ -9,8 +9,8 @@ #include "unlikely.hpp" #include "unwind_output_hash.hpp" -#include #include +#include #include namespace ddprof { diff --git a/src/live_allocation.cc b/src/live_allocation.cc index 0ed4c3f47..b2822b9ba 100644 --- a/src/live_allocation.cc +++ b/src/live_allocation.cc @@ -71,8 +71,7 @@ void LiveAllocation::register_allocation(const UnwindOutput &uo, } // Should we erase the element here ? // only if we are sure it is not the same as the one we are inserting. - if (v._unique_stack != &unique_stack && - !v._unique_stack->second._count) { + if (v._unique_stack != &unique_stack && !v._unique_stack->second._count) { stacks.erase(v._unique_stack->first); } } diff --git a/test/live_allocation-ut.cc b/test/live_allocation-ut.cc index 45d8de645..2137fbd28 100644 --- a/test/live_allocation-ut.cc +++ b/test/live_allocation-ut.cc @@ -26,9 +26,9 @@ TEST(LiveAllocationTest, simple) { int64_t nb_registered_allocs = 10; { // allocate 10 uintptr_t addr = 0x10; - for (int i=0; i