perf(profiling): intern strings/functions into libdatadog

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

@@ -4,7 +4,6 @@
 #include <iostream>
 #include <string>
 #include <string_view>
-#include <variant>
 
 extern "C"
 {
@@ -139,6 +138,13 @@ add_tag(ddog_Vec_Tag& tags, const ExportTagKey key, std::string_view val, std::s
     return add_tag(tags, key_sv, val, errmsg);
 }
 
+namespace internal {
+
+ddog_prof_ProfilesDictionaryHandle
+get_profiles_dictionary();
+
+} // namespace internal
+
 // Keep macros from propagating
 #undef X_STR
 #undef X_ENUM

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

@@ -53,6 +53,14 @@ struct StringArena
 
 } // namespace internal
 
+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);
+
 class SampleManager; // friend
 
 class Sample
@@ -70,12 +78,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 +99,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 +140,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,38 @@
 #include "uploader_builder.hpp"
 
 #include <cstdlib>
+#include <datadog/profiling.h>
 #include <iostream>
 #include <unistd.h>
 #include <unordered_map>
 
+// Dictionary state - needs to be at file scope so postfork can reset it
+namespace {
+
+std::once_flag dict_init_flag;
+ddog_prof_ProfilesDictionaryHandle dict_handle = nullptr;
+
+} // namespace
+
+namespace Datadog::internal {
+
+ddog_prof_ProfilesDictionaryHandle
+get_profiles_dictionary()
+{
+    std::call_once(dict_init_flag, []() {
+        auto result = ddog_prof_ProfilesDictionary_new(&dict_handle);
+        if (result.flags) {
+            std::cerr << "could not initialise profiles dictionary: " << result.err << std::endl;
+        }
+
+        // TODO: how should we handle this case? Return a nullptr? If it happens, we're stuck.
+    });
+
+    return dict_handle;
+}
+
+} // namespace Datadog::internal
+
 // 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)
@@ -22,6 +50,16 @@ std::once_flag ddup_init_flag;    // NOLINT (cppcoreguidelines-avoid-non-const-g
 void
 ddup_postfork_child()
 {
+    // Drop the dictionary handle from the parent before resetting
+    // The ProfilesDictionary is reference-counted, so we must drop our reference
+    if (dict_handle != nullptr) {
+        ddog_prof_ProfilesDictionary_drop(&dict_handle);
+    }
+
+    // We need to re-initialize std::once_flag to allow re-creation
+    new (&dict_init_flag) std::once_flag();
+    dict_handle = nullptr;
+
     Datadog::Uploader::postfork_child();
     Datadog::SampleManager::postfork_child();
 }
@@ -32,6 +70,16 @@ ddup_postfork_parent()
     Datadog::Uploader::postfork_parent();
 }
 
+// Cleanup function to free the profiles dictionary on exit
+void
+ddup_cleanup()
+{
+    if (dict_handle != nullptr) {
+        ddog_prof_ProfilesDictionary_drop(&dict_handle);
+        dict_handle = nullptr;
+    }
+}
+
 // Since we don't control the internal state of libdatadog's exporter and we want to prevent state-tearing
 // after a `fork()`, we just outright prevent uploads from happening during a `fork()` operation.  Since a
 // new upload may be scheduled _just_ as `fork()` is entered, we also need to prevent new uploads from happening.
@@ -144,6 +192,11 @@ 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);
 
+        Datadog::internal::get_profiles_dictionary();
+
+        // Register cleanup function to free resources on exit
+        std::atexit(ddup_cleanup);
+
         // Set the global initialization flag
         is_ddup_initialized = true;
         return true;
@@ -286,6 +339,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>
 
@@ -55,6 +56,20 @@ Datadog::Profile::reset_profile()
         ddog_Error_drop(&err);
         return false;
     }
+
+    auto set_dictionary_res =
+      ddog_prof_Profile_set_profiles_dictionary(&cur_profile, Datadog::internal::get_profiles_dictionary());
+    if (set_dictionary_res.tag != DDOG_VOID_RESULT_OK) {
+        auto err = set_dictionary_res.err;
+        if (!already_warned) {
+            already_warned = true;
+            const std::string errmsg = err_to_msg(&err, "Error setting profiles dictionary");
+            std::cerr << errmsg << std::endl;
+        }
+        ddog_Error_drop(&err);
+        return false;
+    }
+
     return true;
 }
 
@@ -185,6 +200,19 @@ Datadog::Profile::one_time_init(SampleType type, unsigned int _max_nframes)
         return;
     }
 
+    auto set_dictionary_res =
+      ddog_prof_Profile_set_profiles_dictionary(&cur_profile, Datadog::internal::get_profiles_dictionary());
+    if (set_dictionary_res.tag != DDOG_VOID_RESULT_OK) {
+        auto err = set_dictionary_res.err;
+        if (!already_warned) {
+            already_warned = true;
+            const std::string errmsg = err_to_msg(&err, "Error setting profiles dictionary");
+            std::cerr << errmsg << std::endl;
+        }
+        ddog_Error_drop(&err);
+        return;
+    }
+
     // We're done. Don't do this again.
     first_time.store(false);
 }
@@ -196,11 +224,12 @@ 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)
 {
     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) {