From 6142f125e75a240d8289eeaa045b4cec3755e4ba Mon Sep 17 00:00:00 2001 From: Taegyun Kim Date: Fri, 20 Sep 2024 19:45:47 +0000 Subject: [PATCH] avoid frequent lock acquire and release --- .../dd_wrapper/include/ddup_interface.hpp | 5 +- .../dd_wrapper/src/ddup_interface.cpp | 46 +++++++++---------- .../internal/datadog/profiling/ddup/_ddup.pyx | 32 +++++++------ 3 files changed, 44 insertions(+), 39 deletions(-) diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/include/ddup_interface.hpp b/ddtrace/internal/datadog/profiling/dd_wrapper/include/ddup_interface.hpp index 4d7dc5ada4..b09a727fa8 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/include/ddup_interface.hpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/include/ddup_interface.hpp @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -31,8 +32,8 @@ extern "C" bool ddup_is_initialized(); void ddup_start(); void ddup_set_runtime_id(std::string_view runtime_id); - void ddup_profile_set_endpoint(uint64_t local_root_span_id, std::string_view trace_endpoint); - void ddup_profile_add_endpoint_count(std::string_view trace_endpoint, int64_t count); + void ddup_profile_set_endpoints(std::map span_ids_to_endpoints); + void ddup_profile_add_endpoint_counts(std::map trace_endpoints_to_counts); bool ddup_upload(); // Proxy functions to the underlying sample diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/src/ddup_interface.cpp b/ddtrace/internal/datadog/profiling/dd_wrapper/src/ddup_interface.cpp index 5b7a2773c9..684f4278bf 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/src/ddup_interface.cpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/src/ddup_interface.cpp @@ -309,36 +309,36 @@ ddup_upload() // cppcheck-suppress unusedFunction } void -ddup_profile_set_endpoint(uint64_t local_root_span_id, - std::string_view trace_endpoint) // cppcheck-suppress unusedFunction +ddup_profile_set_endpoints( + std::map span_ids_to_endpoints) // cppcheck-suppress unusedFunction { - ddog_CharSlice trace_endpoint_slice = Datadog::to_slice(trace_endpoint); - ddog_prof_Profile& profile = Datadog::Sample::profile_borrow(); - auto res = ddog_prof_Profile_set_endpoint(&profile, local_root_span_id, trace_endpoint_slice); - Datadog::Sample::profile_release(); - - if (!res.ok) { - auto err = res.err; - const std::string errmsg = Datadog::err_to_msg(&err, "Error setting endpoint"); - std::cerr << errmsg << std::endl; - ddog_Error_drop(&err); + for (const auto& [span_id, trace_endpoint] : span_ids_to_endpoints) { + ddog_CharSlice trace_endpoint_slice = Datadog::to_slice(trace_endpoint); + auto res = ddog_prof_Profile_set_endpoint(&profile, span_id, trace_endpoint_slice); + if (!res.ok) { + auto err = res.err; + const std::string errmsg = Datadog::err_to_msg(&err, "Error setting endpoint"); + std::cerr << errmsg << std::endl; + ddog_Error_drop(&err); + } } + Datadog::Sample::profile_release(); } void -ddup_profile_add_endpoint_count(std::string_view trace_endpoint, int64_t count) +ddup_profile_add_endpoint_counts(std::map trace_endpoints_to_counts) { - ddog_CharSlice trace_endpoint_slice = Datadog::to_slice(trace_endpoint); - ddog_prof_Profile& profile = Datadog::Sample::profile_borrow(); - auto res = ddog_prof_Profile_add_endpoint_count(&profile, trace_endpoint_slice, count); - Datadog::Sample::profile_release(); - - if (!res.ok) { - auto err = res.err; - const std::string errmsg = Datadog::err_to_msg(&err, "Error adding endpoint count"); - std::cerr << errmsg << std::endl; - ddog_Error_drop(&err); + for (const auto& [trace_endpoint, count] : trace_endpoints_to_counts) { + ddog_CharSlice trace_endpoint_slice = Datadog::to_slice(trace_endpoint); + auto res = ddog_prof_Profile_add_endpoint_count(&profile, trace_endpoint_slice, count); + if (!res.ok) { + auto err = res.err; + const std::string errmsg = Datadog::err_to_msg(&err, "Error adding endpoint count"); + std::cerr << errmsg << std::endl; + ddog_Error_drop(&err); + } } + Datadog::Sample::profile_release(); } diff --git a/ddtrace/internal/datadog/profiling/ddup/_ddup.pyx b/ddtrace/internal/datadog/profiling/ddup/_ddup.pyx index bdf160d7ea..63f8437d93 100644 --- a/ddtrace/internal/datadog/profiling/ddup/_ddup.pyx +++ b/ddtrace/internal/datadog/profiling/ddup/_ddup.pyx @@ -5,6 +5,9 @@ from typing import Dict from typing import Optional from typing import Union +from libcpp.map cimport map +from libcpp.utility cimport pair + import ddtrace import platform from .._types import StringType @@ -47,8 +50,8 @@ cdef extern from "ddup_interface.hpp": void ddup_start() void ddup_set_runtime_id(string_view _id) - void ddup_profile_set_endpoint(uint64_t local_root_span_id, string_view trace_endpoint) - void ddup_profile_add_endpoint_count(string_view trace_endpoint, int64_t count) + void ddup_profile_set_endpoints(map[int64_t, string_view] span_ids_to_endpoints) + void ddup_profile_add_endpoint_counts(map[string_view, int64_t] trace_endpoints_to_counts) bint ddup_upload() nogil Sample *ddup_start_sample() @@ -175,26 +178,27 @@ def upload() -> None: processor = ddtrace.tracer._endpoint_call_counter_span_processor endpoint_counts, endpoint_to_span_ids = processor.reset() + + cdef map[int64_t, string_view] span_ids_to_endpoints = map[int64_t, string_view]() for endpoint, span_ids in endpoint_to_span_ids.items(): endpoint_bytes = ensure_binary_or_empty(endpoint) for span_id in span_ids: - # TODO(taegyunkim): Pass the mapping directly to the native - # code to avoid the need to iterate over the mapping and call - # the native function multiple times which leads to frequent - # lock acquire and release inside the native code. - ddup_profile_set_endpoint( - clamp_to_uint64_unsigned(span_id), - string_view(endpoint_bytes, len(endpoint_bytes)), + span_ids_to_endpoints.insert( + pair[int64_t, string_view]( + clamp_to_uint64_unsigned(span_id), + string_view(endpoint_bytes, len(endpoint_bytes)) + ) ) + ddup_profile_set_endpoints(span_ids_to_endpoints) + + cdef map[string_view, int64_t] trace_endpoints_to_counts = map[string_view, int64_t]() for endpoint, cnt in endpoint_counts.items(): endpoint_bytes = ensure_binary_or_empty(endpoint) - # Same comment as above applies here, avoid frequent lock - # acquire and release by passing the mapping directly to the - # native code. - ddup_profile_add_endpoint_count( + trace_endpoints_to_counts.insert(pair[string_view, int64_t]( string_view(endpoint_bytes, len(endpoint_bytes)), clamp_to_int64_unsigned(cnt) - ) + )) + ddup_profile_add_endpoint_counts(trace_endpoints_to_counts) with nogil: ddup_upload()