try to intern strings/functions

Author: KowalskiThomas

ddtrace/internal/datadog/profiling/dd_wrapper/include/ddup_interface.hpp

@@ -14,6 +14,13 @@ class Sample;
 extern "C"
 {
 #endif
+
+    using string_id_t = std::add_pointer<void>::type;
+    using function_id_t = std::add_pointer<void>::type;
+
+    string_id_t ddup_intern_string(std::string_view s);
+    function_id_t ddup_intern_function(string_id_t name, string_id_t filename);
+
     void ddup_config_env(std::string_view dd_env);
     void ddup_config_service(std::string_view service);
     void ddup_config_version(std::string_view version);
@@ -65,6 +72,7 @@ extern "C"
                          std::string_view _filename,
                          uint64_t address,
                          int64_t line);
+    void ddup_push_frame_id(Datadog::Sample* sample, function_id_t function_id, uint64_t address, int64_t line);
     void ddup_push_absolute_ns(Datadog::Sample* sample, int64_t timestamp_ns);
     void ddup_push_monotonic_ns(Datadog::Sample* sample, int64_t monotonic_ns);
     void ddup_flush_sample(Datadog::Sample* sample);

ddtrace/internal/datadog/profiling/dd_wrapper/include/libdatadog_helpers.hpp

@@ -11,6 +11,9 @@ extern "C"
 #include "datadog/profiling.h"
 }
 
+ddog_prof_ProfilesDictionaryHandle
+get_dictionary();
+
 namespace Datadog {
 
 // There's currently no need to offer custom tags, so there's no interface for

ddtrace/internal/datadog/profiling/dd_wrapper/include/profile.hpp

@@ -60,6 +60,6 @@ class Profile
     const ValueIndex& val();
 
     // collect
-    bool collect(const ddog_prof_Sample& sample, int64_t endtime_ns);
+    bool collect(const ddog_prof_Sample2& sample, int64_t endtime_ns);
 };
 } // namespace Datadog

ddtrace/internal/datadog/profiling/dd_wrapper/include/sample.hpp

@@ -6,6 +6,7 @@
 
 #include <string>
 #include <string_view>
+#include <unordered_map>
 #include <vector>
 
 extern "C"
@@ -15,6 +16,14 @@ extern "C"
 
 namespace Datadog {
 
+using string_id = ddog_prof_StringId2;
+using function_id = ddog_prof_FunctionId2;
+
+string_id
+intern_string(std::string_view s);
+function_id
+intern_function(string_id name, string_id filename);
+
 namespace internal {
 
 // StringArena holds copies of strings we need while building samples.
@@ -70,12 +79,12 @@ class Sample
     static inline bool timeline_enabled = false;
 
     // Keeps temporary buffer of frames in the stack
-    std::vector<ddog_prof_Location> locations;
+    std::vector<ddog_prof_Location2> locations;
     size_t dropped_frames = 0;
     uint64_t samples = 0;
 
     // Storage for labels
-    std::vector<ddog_prof_Label> labels{};
+    std::vector<ddog_prof_Label2> labels{};
 
     // Storage for values
     std::vector<int64_t> values = {};
@@ -91,6 +100,7 @@ class Sample
     bool push_label(ExportLabelKey key, std::string_view val);
     bool push_label(ExportLabelKey key, int64_t val);
     void push_frame_impl(std::string_view name, std::string_view filename, uint64_t address, int64_t line);
+    void push_frame_impl(function_id function_id, uint64_t address, int64_t line);
     void clear_buffers();
 
     // Add values
@@ -131,6 +141,12 @@ class Sample
                     int64_t line               // for ddog_prof_Location
     );
 
+    // Assumes frames are pushed in leaf-order
+    void push_frame(function_id function_id, // for ddog_prof_Location
+                    uint64_t address,        // for ddog_prof_Location
+                    int64_t line             // for ddog_prof_Location
+    );
+
     // Flushes the current buffer, clearing it
     bool flush_sample(bool reverse_locations = false);
 

ddtrace/internal/datadog/profiling/dd_wrapper/src/ddup_interface.cpp

@@ -9,10 +9,27 @@
 #include "uploader_builder.hpp"
 
 #include <cstdlib>
+#include <datadog/profiling.h>
 #include <iostream>
 #include <unistd.h>
 #include <unordered_map>
 
+ddog_prof_ProfilesDictionaryHandle
+get_dictionary()
+{
+    static ddog_prof_ProfilesDictionaryHandle handle = nullptr;
+
+    if (!handle) {
+        auto result = ddog_prof_ProfilesDictionary_new(&handle);
+        if (result.flags) {
+            std::cerr << "could not initialise profiles dictionary: " << result.err << std::endl;
+            return nullptr;
+        }
+    }
+
+    return handle;
+}
+
 // State
 bool is_ddup_initialized = false; // NOLINT (cppcoreguidelines-avoid-non-const-global-variables)
 std::once_flag ddup_init_flag;    // NOLINT (cppcoreguidelines-avoid-non-const-global-variables)
@@ -144,6 +161,8 @@ ddup_start() // cppcheck-suppress unusedFunction
         // Right now, only do things in the child _after_ fork
         pthread_atfork(ddup_prefork, ddup_postfork_parent, ddup_postfork_child);
 
+        get_dictionary();
+
         // Set the global initialization flag
         is_ddup_initialized = true;
         return true;
@@ -286,6 +305,28 @@ ddup_push_frame(Datadog::Sample* sample, // cppcheck-suppress unusedFunction
     sample->push_frame(_name, _filename, address, line);
 }
 
+void
+ddup_push_frame_id(Datadog::Sample* sample,
+                   function_id_t function_id,
+                   uint64_t address,
+                   int64_t line) // cppcheck-suppress unusedFunction
+{
+
+    sample->push_frame(static_cast<Datadog::function_id>(function_id), address, line);
+}
+
+string_id_t
+ddup_intern_string(std::string_view s) // cppcheck-suppress unusedFunction
+{
+    return Datadog::intern_string(s);
+}
+
+function_id_t
+ddup_intern_function(string_id_t name, string_id_t filename) // cppcheck-suppress unusedFunction
+{
+    return Datadog::intern_function(static_cast<Datadog::string_id>(name), static_cast<Datadog::string_id>(filename));
+}
+
 void
 ddup_push_absolute_ns(Datadog::Sample* sample, int64_t timestamp_ns) // cppcheck-suppress unusedFunction
 {

ddtrace/internal/datadog/profiling/dd_wrapper/src/profile.cpp

@@ -2,6 +2,7 @@
 
 #include "libdatadog_helpers.hpp"
 
+#include <datadog/profiling.h>
 #include <functional>
 #include <iostream>
 
@@ -196,11 +197,19 @@ Datadog::Profile::val()
 }
 
 bool
-Datadog::Profile::collect(const ddog_prof_Sample& sample, int64_t endtime_ns)
+Datadog::Profile::collect(const ddog_prof_Sample2& sample, int64_t endtime_ns)
 {
+    auto set_dictionary_res = ddog_prof_Profile_set_profiles_dictionary(&cur_profile, get_dictionary());
+    if (set_dictionary_res.tag != DDOG_VOID_RESULT_OK) {
+        auto err = set_dictionary_res.err;
+        std::cerr << "Error setting profiles dictionary: " << err_to_msg(&err, "") << std::endl;
+        ddog_Error_drop(&err);
+        return false;
+    }
+
     static bool already_warned = false; // cppcheck-suppress threadsafety-threadsafety
     const std::lock_guard<std::mutex> lock(profile_mtx);
-    auto res = ddog_prof_Profile_add(&cur_profile, sample, endtime_ns);
+    auto res = ddog_prof_Profile_add2(&cur_profile, sample, endtime_ns);
     if (!res.ok) {          // NOLINT (cppcoreguidelines-pro-type-union-access)
         auto err = res.err; // NOLINT (cppcoreguidelines-pro-type-union-access)
         if (!already_warned) {

ddtrace/internal/datadog/profiling/dd_wrapper/src/sample.cpp

@@ -1,11 +1,51 @@
 #include "sample.hpp"
 
-#include "code_provenance.hpp"
+#include "libdatadog_helpers.hpp"
 
 #include <algorithm>
 #include <chrono>
+#include <datadog/common.h>
+#include <datadog/profiling.h>
 #include <string_view>
-#include <thread>
+
+Datadog::string_id
+Datadog::intern_string(std::string_view s)
+{
+    auto dict = get_dictionary();
+
+    ddog_prof_StringId2 string_id;
+    auto insert_str_res = ddog_prof_ProfilesDictionary_insert_str(
+      &string_id, dict, to_slice(s), ddog_prof_Utf8Option::DDOG_PROF_UTF8_OPTION_CONVERT_LOSSY);
+
+    if (insert_str_res.flags) {
+        std::cerr << "Error inserting string: " << insert_str_res.err << std::endl;
+        return nullptr;
+    }
+
+    return string_id;
+}
+
+Datadog::function_id
+Datadog::intern_function(string_id name, string_id filename)
+{
+    static auto empty_string_id = intern_string("");
+
+    auto dict = get_dictionary();
+    ddog_prof_Function2 my_function = {
+        .name = name,
+        .system_name = empty_string_id, // No support for system_name in Python
+        .file_name = filename,
+    };
+
+    ddog_prof_FunctionId2 function_id;
+    auto insert_function_res = ddog_prof_ProfilesDictionary_insert_function(&function_id, dict, &my_function);
+    if (insert_function_res.flags) {
+        std::cerr << "Error inserting function: " << insert_function_res.err << std::endl;
+        return nullptr;
+    }
+
+    return function_id;
+}
 
 Datadog::internal::StringArena::StringArena()
 {
@@ -55,25 +95,27 @@ Datadog::Sample::Sample(SampleType _type_mask, unsigned int _max_nframes)
 void
 Datadog::Sample::push_frame_impl(std::string_view name, std::string_view filename, uint64_t address, int64_t line)
 {
-    static const ddog_prof_Mapping null_mapping = { 0, 0, 0, to_slice(""), { 0 }, to_slice(""), { 0 } };
-    name = string_storage.insert(name);
-    filename = string_storage.insert(filename);
-
-    const ddog_prof_Location loc = {
-        .mapping = null_mapping, // No support for mappings in Python
-        .function = {
-          .name = to_slice(name),
-          .name_id = { 0 },
-          .system_name = {}, // No support for system_name in Python
-          .system_name_id = { 0 },
-          .filename = to_slice(filename),
-          .filename_id = { 0 },
-        },
-        .address = address,
-        .line = line,
-    };
+    // static const ddog_prof_Mapping null_mapping = { 0, 0, 0, to_slice(""), { 0 }, to_slice(""), { 0 } };
+    // name = string_storage.insert(name);
+    // filename = string_storage.insert(filename);
 
-    locations.emplace_back(loc);
+    auto name_id = intern_string(name);
+    auto filename_id = intern_string(filename);
+
+    auto function_id = intern_function(name_id, filename_id);
+
+    push_frame_impl(function_id, address, line);
+}
+
+void
+Datadog::Sample::push_frame_impl(function_id function_id, uint64_t address, int64_t line)
+{
+    locations.push_back({
+      .mapping = nullptr, // No support for mappings in Python
+      .function = function_id,
+      .address = address,
+      .line = line,
+    });
 }
 
 void
@@ -87,6 +129,16 @@ Datadog::Sample::push_frame(std::string_view name, std::string_view filename, ui
     }
 }
 
+void
+Datadog::Sample::push_frame(function_id function_id, uint64_t address, int64_t line)
+{
+    if (locations.size() <= max_nframes) {
+        push_frame_impl(function_id, address, line);
+    } else {
+        ++dropped_frames;
+    }
+}
+
 bool
 Datadog::Sample::push_label(const ExportLabelKey key, std::string_view val)
 {
@@ -100,11 +152,18 @@ Datadog::Sample::push_label(const ExportLabelKey key, std::string_view val)
         return true;
     }
 
+    std::string_view val_str = string_storage.insert(val);
+    const static std::string unit_str = "";
+
     // Otherwise, persist the val string and add the label
-    val = string_storage.insert(val);
-    auto& label = labels.emplace_back();
-    label.key = to_slice(key_sv);
-    label.str = to_slice(val);
+    labels.push_back({
+      .key = intern_string(key_sv),
+      // do not intern this because it could be a memory leak if values are many-valued
+      .str = to_slice(val_str),
+      .num = 0,
+      // do not intern this because it could be a memory leak if values are many-valued
+      .num_unit = to_slice(unit_str.c_str()),
+    });
     return true;
 }
 
@@ -119,11 +178,13 @@ Datadog::Sample::push_label(const ExportLabelKey key, int64_t val)
         return true;
     }
 
-    auto& label = labels.emplace_back();
-    label.key = to_slice(key_sv);
-    label.str = to_slice("");
-    label.num = val;
-    label.num_unit = to_slice("");
+    auto key_id = intern_string(key_sv);
+    labels.push_back({
+      .key = key_id,
+      .str = to_slice(""),
+      .num = val,
+      .num_unit = to_slice(""),
+    });
     return true;
 }
 
@@ -150,7 +211,7 @@ Datadog::Sample::flush_sample(bool reverse_locations)
         std::reverse(locations.begin(), locations.end());
     }
 
-    const ddog_prof_Sample sample = {
+    const ddog_prof_Sample2 sample = {
         .locations = { locations.data(), locations.size() },
         .values = { values.data(), values.size() },
         .labels = { labels.data(), labels.size() },