Skip to content

Commit 93dc0ba

Browse files
dd-octo-sts[bot]KowalskiThomastaegyunkim
authored
fix(profiling): adapt Stack V2 to exception-free Echion [backport 3.17] (#15052)
Backport 2c72bca from #14933 to 3.17. ## What does this PR do? This PR updates Echion to use the latest version. Included Echion PRs - [refactor: stop using exceptions for reporting errors](P403n1x87/echion#156) - [misc: remove Error classes](P403n1x87/echion#164) - [bug: avoid possibly unbounded memory allocations](P403n1x87/echion#159) - [fix: remove extra loop in Frame::read() for 3.13+](P403n1x87/echion#167) - [refactor: return std::reference_wrapper over raw pointer](P403n1x87/echion#168) See comparison here P403n1x87/echion@3ebeb3e...39d74a3 Co-authored-by: T. Kowalski <[email protected]> Co-authored-by: Taegyun Kim <[email protected]>
1 parent 4f94fd3 commit 93dc0ba

File tree

6 files changed

+32
-20
lines changed

6 files changed

+32
-20
lines changed

ddtrace/internal/datadog/profiling/stack_v2/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ endif()
4141

4242
# Add echion
4343
set(ECHION_COMMIT
44-
"3ebeb3e975239f252fa0d6bb739344f35eaf1657" # https://github.com/kowalskithomas/echion/commit/3ebeb3e975239f252fa0d6bb739344f35eaf1657
44+
"39d74a33a3f3abe810e6a29132721871e3127472" # https://github.com/P403n1x87/echion/commit/39d74a33a3f3abe810e6a29132721871e3127472
4545
CACHE STRING "Commit hash of echion to use")
4646
FetchContent_Declare(
4747
echion

ddtrace/internal/datadog/profiling/stack_v2/include/stack_renderer.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ class StackRenderer : public RendererInterface
4040
// the sample is created, this has to be reset.
4141
bool pushed_task_name = false;
4242

43-
void open() override {}
43+
Result<void> open() override { return Result<void>::ok(); }
4444
void close() override {}
4545
void header() override {}
4646
void metadata(const std::string&, const std::string&) override {}

ddtrace/internal/datadog/profiling/stack_v2/src/sampler.cpp

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#include "thread_span_links.hpp"
44

5+
#include "echion/errors.h"
56
#include "echion/greenlets.h"
67
#include "echion/interp.h"
78
#include "echion/tasks.h"
@@ -153,7 +154,7 @@ Sampler::sampling_thread(const uint64_t seq_num)
153154
// Perform the sample
154155
for_each_interp([&](InterpreterInfo& interp) -> void {
155156
for_each_thread(interp, [&](PyThreadState* tstate, ThreadInfo& thread) {
156-
thread.sample(interp.id, tstate, wall_time_us);
157+
(void)thread.sample(interp.id, tstate, wall_time_us);
157158
});
158159
});
159160

@@ -245,19 +246,21 @@ Sampler::register_thread(uint64_t id, uint64_t native_id, const char* name)
245246
static bool has_errored = false;
246247
auto it = thread_info_map.find(id);
247248
if (it == thread_info_map.end()) {
248-
try {
249-
thread_info_map.emplace(id, std::make_unique<ThreadInfo>(id, native_id, name));
250-
} catch (const ThreadInfo::Error& e) {
249+
auto maybe_thread_info = ThreadInfo::create(id, native_id, name);
250+
if (maybe_thread_info) {
251+
thread_info_map.emplace(id, std::move(*maybe_thread_info));
252+
} else {
251253
if (!has_errored) {
252254
has_errored = true;
253255
std::cerr << "Failed to register thread: " << std::hex << id << std::dec << " (" << native_id << ") "
254256
<< name << std::endl;
255257
}
256258
}
257259
} else {
258-
try {
259-
it->second = std::make_unique<ThreadInfo>(id, native_id, name);
260-
} catch (const ThreadInfo::Error& e) {
260+
auto maybe_thread_info = ThreadInfo::create(id, native_id, name);
261+
if (maybe_thread_info) {
262+
it->second = std::move(*maybe_thread_info);
263+
} else {
261264
if (!has_errored) {
262265
has_errored = true;
263266
std::cerr << "Failed to register thread: " << std::hex << id << std::dec << " (" << native_id << ") "

ddtrace/internal/datadog/profiling/stack_v2/src/stack_renderer.cpp

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -131,15 +131,18 @@ StackRenderer::render_frame(Frame& frame)
131131
static constexpr std::string_view missing_name = "<unknown function>";
132132
std::string_view filename_str;
133133
std::string_view name_str;
134-
try {
135-
filename_str = string_table.lookup(frame.filename);
136-
} catch (StringTable::Error&) {
134+
135+
auto maybe_filename_str = string_table.lookup(frame.filename);
136+
if (maybe_filename_str) {
137+
filename_str = maybe_filename_str->get();
138+
} else {
137139
filename_str = missing_filename;
138140
}
139141

140-
try {
141-
name_str = string_table.lookup(frame.name);
142-
} catch (StringTable::Error&) {
142+
auto maybe_name_str = string_table.lookup(frame.name);
143+
if (maybe_name_str) {
144+
name_str = maybe_name_str->get();
145+
} else {
143146
name_str = missing_name;
144147
}
145148

ddtrace/internal/datadog/profiling/stack_v2/src/stack_v2.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -213,16 +213,15 @@ track_greenlet(PyObject* Py_UNUSED(m), PyObject* args)
213213
if (!PyArg_ParseTuple(args, "lOO", &greenlet_id, &name, &frame))
214214
return NULL;
215215

216-
StringTable::Key greenlet_name;
217-
218-
try {
219-
greenlet_name = string_table.key(name);
220-
} catch (StringTable::Error&) {
216+
auto maybe_greenlet_name = string_table.key(name);
217+
if (!maybe_greenlet_name) {
221218
// We failed to get this task but we keep going
222219
PyErr_SetString(PyExc_RuntimeError, "Failed to get greenlet name from the string table");
223220
return NULL;
224221
}
225222

223+
auto greenlet_name = *maybe_greenlet_name;
224+
226225
Py_BEGIN_ALLOW_THREADS;
227226
Sampler::get().track_greenlet(greenlet_id, greenlet_name, frame);
228227
Py_END_ALLOW_THREADS;
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
fixes:
3+
- |
4+
profiling: Upgrades echion to resolve an issue where stack profiler can
5+
allocate a large amount of memory unnecessarily. Resolves another issue
6+
where the profiler can loop infinitely on Python 3.13.
7+

0 commit comments

Comments
 (0)