Linter fixes

- Clang tidy
- Cleanup of unused fields

Author: r1viollet

include/live_allocation.hpp

@@ -82,11 +82,6 @@ class LiveAllocation {
   using PidMap = std::unordered_map<pid_t, PidStacks>;
   using WatcherVector = std::vector<PidMap>;
 
-  WatcherVector _watcher_vector;
-
-  int64_t upscale_with_mapping(const PprofStacks::value_type &stack,
-                               PidStacks &pid_stacks);
-
   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);
@@ -122,12 +117,13 @@ class LiveAllocation {
 
   static std::vector<SmapsEntry> parse_smaps(pid_t pid);
 
-  void cycle() { _stats = {}; }
+  static int64_t upscale_with_mapping(const PprofStacks::value_type &stack,
+                                      PidStacks &pid_stacks);
 
-  void set_default_unwind_output(const UnwindOutput &uo) {
-    _default_uw_output = uo;
-  }
+  void cycle() { _stats = {}; }
 
+  // no lint to avoid warning about member being public (should be refactored)
+  WatcherVector _watcher_vector; // NOLINT
 private:
   static bool register_deallocation_internal(uintptr_t address,
                                              PidStacks &pid_stacks);
@@ -139,7 +135,6 @@ class LiveAllocation {
   struct {
     unsigned _unmatched_deallocations = {};
   } _stats;
-  UnwindOutput _default_uw_output;
 };
 
 } // namespace ddprof

src/ddprof_worker.cc

@@ -253,9 +253,10 @@ DDRes aggregate_livealloc_stack(
     int64_t upscalled_value, DDProfContext &ctx, const PerfWatcher *watcher,
     DDProfPProf *pprof, const SymbolHdr &symbol_hdr) {
 
-  if (upscalled_value)
+  if (upscalled_value) {
     LG_DBG("Upscaling from %ld to %ld", alloc_info.second._value,
            upscalled_value);
+  }
   // default to the sampled value
   const DDProfValuePack pack{
       upscalled_value ? upscalled_value : alloc_info.second._value,
@@ -269,7 +270,6 @@ DDRes aggregate_livealloc_stack(
 }
 
 DDRes aggregate_live_allocations_common(DDProfContext &ctx,
-                                        LiveAllocation &live_allocations,
                                         unsigned watcher_pos, pid_t pid,
                                         LiveAllocation::PidStacks &pid_stacks) {
   UnwindState *us = ctx.worker_ctx.us;
@@ -289,8 +289,8 @@ DDRes aggregate_live_allocations_common(DDProfContext &ctx,
 
   // Step 1: Process existing samples
   for (const auto &alloc_info : pid_stacks._unique_stacks) {
-    int64_t upscaled_value =
-        live_allocations.upscale_with_mapping(alloc_info, pid_stacks);
+    const int64_t upscaled_value =
+        ddprof::LiveAllocation::upscale_with_mapping(alloc_info, pid_stacks);
     DDRES_CHECK_FWD(aggregate_livealloc_stack(alloc_info, upscaled_value, ctx,
                                               watcher, pprof, symbol_hdr));
   }
@@ -307,7 +307,7 @@ DDRes aggregate_live_allocations_common(DDProfContext &ctx,
   for (const auto &el : pid_stacks.entries) {
     if (el.accounted_size == 0) {
       // Use the RSS value for this mapping as the "unsampled" value
-      int64_t unsampled_value = el.rss_kb * 1000; // RSS in bytes
+      const int64_t unsampled_value = el.rss_kb * 1000; // RSS in bytes
       // Add the frame using the fake UnwindOutput
       const DDProfValuePack pack{unsampled_value, 1, 0};
       DDRES_CHECK_FWD(
@@ -335,8 +335,8 @@ DDRes aggregate_live_allocations_for_pid(DDProfContext &ctx, pid_t pid) {
     auto &pid_map = live_allocations._watcher_vector[watcher_pos];
     auto &pid_stacks = pid_map[pid]; // Get PidStacks for the specific pid
     // Call the common function to handle the aggregation
-    DDRES_CHECK_FWD(aggregate_live_allocations_common(
-        ctx, live_allocations, watcher_pos, pid, pid_stacks));
+    DDRES_CHECK_FWD(
+        aggregate_live_allocations_common(ctx, watcher_pos, pid, pid_stacks));
   }
 
   return {};
@@ -353,7 +353,7 @@ DDRes aggregate_live_allocations(DDProfContext &ctx) {
       auto &pid_stacks = pid_vt.second;
       // Call the common function to handle the aggregation for each PID
       DDRES_CHECK_FWD(aggregate_live_allocations_common(
-          ctx, live_allocations, watcher_pos, pid_vt.first, pid_stacks));
+          ctx, watcher_pos, pid_vt.first, pid_stacks));
     }
   }
 
@@ -440,12 +440,6 @@ DDRes worker_library_init(DDProfContext &ctx,
     ctx.worker_ctx.exp[1] = nullptr;
     ctx.worker_ctx.pprof[0] = nullptr;
     ctx.worker_ctx.pprof[1] = nullptr;
-
-    // live allocation - create default unwind output
-    add_common_frame(ctx.worker_ctx.us, SymbolErrors::unsampled_mapping);
-    ctx.worker_ctx.live_allocation.set_default_unwind_output(
-        ctx.worker_ctx.us->output);
-    ctx.worker_ctx.us->output.clear();
   }
   CatchExcept2DDRes();
   return {};

src/live_allocation.cc

@@ -36,7 +36,7 @@ bool LiveAllocation::register_deallocation_internal(uintptr_t address,
   // object
   if (v._unique_stack) {
     // Adjust the mapping values map for the allocation being deallocated
-    ProcessAddress_t mapping_start = v._unique_stack->first.start_mmap;
+    const ProcessAddress_t mapping_start = v._unique_stack->first.start_mmap;
     mapping_values[mapping_start]._value -= v._value;
     if (mapping_values[mapping_start]._count > 0) {
       --mapping_values[mapping_start]._count;
@@ -88,17 +88,14 @@ bool LiveAllocation::register_allocation_internal(const UnwindOutput &uo,
   if (entry == pid_stacks.entries.end() || address < entry->start) {
     // Address not within any known mapping
     LG_DBG("(LIVE_ALLOC) Address not within any known mapping: %lx", address);
-    if (entry != pid_stacks.entries.end())
-      LG_DBG("(LIVE_ALLOC) matched entry start: %lx, end: %lx", entry->start,
-             entry->end);
   } else {
     start_addr = entry->start;
   }
 
   // Create or find the PprofStacks::value_type object corresponding to the
   // UnwindOutput
   auto &stacks = pid_stacks._unique_stacks;
-  StackAndMapping stack_key{uo_ptr, start_addr};
+  const StackAndMapping stack_key{uo_ptr, start_addr};
   auto iter = stacks.find(stack_key);
   if (iter == stacks.end()) {
     iter = stacks.emplace(stack_key, ValueAndCount{}).first;
@@ -151,7 +148,7 @@ bool LiveAllocation::register_allocation_internal(const UnwindOutput &uo,
 }
 
 std::vector<SmapsEntry> LiveAllocation::parse_smaps(pid_t pid) {
-  std::string smaps_file = absl::StrFormat("%s/%d/smaps", "/proc", pid);
+  const std::string smaps_file = absl::StrFormat("%s/%d/smaps", "/proc", pid);
 
   std::vector<SmapsEntry> entries;
   FILE *file = fopen(smaps_file.c_str(), "r");
@@ -164,7 +161,7 @@ std::vector<SmapsEntry> LiveAllocation::parse_smaps(pid_t pid) {
   SmapsEntry current_entry;
 
   while (fgets(buffer, sizeof(buffer), file)) {
-    std::string_view line(buffer);
+    const std::string_view line(buffer);
 
     if (line.find("Rss:") == 0) {
       // Extract RSS value (take characters after "Rss:    ")
@@ -180,12 +177,12 @@ std::vector<SmapsEntry> LiveAllocation::parse_smaps(pid_t pid) {
       }
       current_entry.rss_kb = rss;
       // push back as we are not parsing other values
-      entries.push_back(std::move(current_entry));
+      entries.push_back(current_entry);
       current_entry = SmapsEntry(); // Reset for next entry
     } else if (line.find('-') != std::string_view::npos) {
       // Extract address
-      std::string_view address = line.substr(0, line.find(' '));
-      size_t dash_position = address.find('-');
+      const std::string_view address = line.substr(0, line.find(' '));
+      const size_t dash_position = address.find('-');
       unsigned long long start;
       unsigned long long end;
       // Convert start address
@@ -268,7 +265,9 @@ LiveAllocation::upscale_with_mapping(const PprofStacks::value_type &stack,
   //
   // What if we have a different profile type to show this?
   //
-  return stack.second._value * entry->rss_kb * 1000 / accounted_value._value;
+  constexpr int KILOBYTES_TO_BYTES = 1000;
+  return stack.second._value * entry->rss_kb * KILOBYTES_TO_BYTES /
+      accounted_value._value;
 }
 
 } // namespace ddprof

src/pprof/ddprof_pprof.cc

@@ -251,11 +251,6 @@ size_t prepare_labels(const UnwindOutput &uw_output, const PerfWatcher &watcher,
     labels[labels_num].str = to_CharSlice(uw_output.container_id);
     ++labels_num;
   }
-  if (mapping_addr) {
-    labels[labels_num].key = to_CharSlice(k_mapping_label);
-    labels[labels_num].str = to_CharSlice(pid_str(mapping_addr, pid_strs));
-    ++labels_num;
-  }
 
   // Add any configured labels.  Note that TID alone has the same cardinality as
   // (TID;PID) tuples, so except for symbol table overhead it doesn't matter