Skip to content

Commit fc7bb21

Browse files
authored
perf(profiling): remove utf8 check (#13054)
This will remove ~160ms per minute from stack v2 cpu time as measured on a test environment. The strings passed from echion are already validated to utf-8, via https://github.com/taegyunkim/echion/blob/taegyunkim/unwind-task-fix/echion/strings.h#L48 and we don't seem to be deleting any items in the string table, from which the strings are coming from. So I don't think there's any more need for validation. Also, libdatadog does utf8 validation too. ![Screenshot 2025-04-17 at 4 32 34 PM](https://github.com/user-attachments/assets/2354d19e-efde-4a4a-aa3e-18aff407d74e) ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
1 parent dcfc4ef commit fc7bb21

File tree

4 files changed

+7
-47
lines changed

4 files changed

+7
-47
lines changed

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ add_cppcheck_target(
6565
${CMAKE_CURRENT_SOURCE_DIR}
6666
INCLUDE
6767
${CMAKE_CURRENT_SOURCE_DIR}/include
68-
${CMAKE_CURRENT_SOURCE_DIR}/include/vendored
6968
${CMAKE_CURRENT_SOURCE_DIR}/include/util
7069
${CMAKE_CURRENT_SOURCE_DIR}/..
7170
${echion_SOURCE_DIR}

ddtrace/internal/datadog/profiling/stack_v2/include/vendored/utf8_validate.hpp

Lines changed: 0 additions & 34 deletions
This file was deleted.

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

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
#include "stack_renderer.hpp"
22

33
#include "thread_span_links.hpp"
4-
#include "utf8_validate.hpp"
54

65
#include "echion/strings.h"
76

@@ -144,17 +143,6 @@ StackRenderer::render_frame(Frame& frame)
144143

145144
auto line = frame.location.line;
146145

147-
// Normally, further utf-8 validation would be pointless here, but we may be reading data where the
148-
// string pointer was valid, but the string is actually garbage data at the exact time of the read.
149-
// This is rare, but blowing some cycles on early validation allows the sample to be retained by
150-
// libdatadog, so we can evaluate the actual impact of this scenario in live scenarios.
151-
static const std::string_view invalid = "<invalid_utf8>";
152-
if (!utf8_check_is_valid(name_str.data(), name_str.size())) {
153-
name_str = invalid;
154-
}
155-
if (!utf8_check_is_valid(filename_str.data(), filename_str.size())) {
156-
filename_str = invalid;
157-
}
158146
// DEV: Echion pushes a dummy frame containing task name, and its line
159147
// number is set to 0.
160148
if (!pushed_task_name and line == 0) {
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
#instructions:
3+
fixes:
4+
- |
5+
profiling: reduces overhead of the v2 stack sampler by removing unnecessary
6+
and expensive utf-8 validation
7+

0 commit comments

Comments
 (0)