Skip to content

Commit

Permalink
avoid frequent lock acquire and release
Browse files Browse the repository at this point in the history
  • Loading branch information
taegyunkim committed Sep 20, 2024
1 parent a95e522 commit 6142f12
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 39 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#pragma once

#include <map>
#include <stddef.h>
#include <stdint.h>
#include <string_view>
Expand Down Expand Up @@ -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<int64_t, std::string_view> span_ids_to_endpoints);
void ddup_profile_add_endpoint_counts(std::map<std::string_view, int64_t> trace_endpoints_to_counts);
bool ddup_upload();

// Proxy functions to the underlying sample
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<int64_t, std::string_view> 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<std::string_view, int64_t> 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();
}
32 changes: 18 additions & 14 deletions ddtrace/internal/datadog/profiling/ddup/_ddup.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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(<const char*>endpoint_bytes, len(endpoint_bytes)),
span_ids_to_endpoints.insert(
pair[int64_t, string_view](
clamp_to_uint64_unsigned(span_id),
string_view(<const char*>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(<const char*>endpoint_bytes, len(endpoint_bytes)),
clamp_to_int64_unsigned(cnt)
)
))
ddup_profile_add_endpoint_counts(trace_endpoints_to_counts)

with nogil:
ddup_upload()
Expand Down

0 comments on commit 6142f12

Please sign in to comment.