Skip to content

Commit d44ad4b

Browse files
danielsnvlad-scherbich
danielsn
and
vlad-scherbich
authored
chore(profiling): enable memalloc to use sample (#15189)
## Description Refactored the memory allocator to use C++, and link to Sample. This will enable us to remove our dependency on python operations in a followon PR. ## Testing <!-- Describe your testing strategy or note what tests are included --> ## Risks <!-- Note any risks associated with this change, or "None" if no risks --> ## Additional Notes <!-- Any other information that would be helpful for reviewers --> --------- Co-authored-by: Vlad Scherbich <vlad.scherbich@datadoghq.com>
1 parent 1bceb85 commit d44ad4b

File tree

15 files changed

+152
-48
lines changed

15 files changed

+152
-48
lines changed

‎.github/workflows/build_python_3.yml‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,6 @@ jobs:
9494
CIBW_BEFORE_ALL_MACOS: rustup target add aarch64-apple-darwin
9595
CIBW_BEFORE_ALL_LINUX: |
9696
if [[ "$(uname -m)-$(uname -i)-$(uname -o | tr '[:upper:]' '[:lower:]')-$(ldd --version 2>&1 | head -n 1 | awk '{print $1}')" != "i686-unknown-linux-musl" ]]; then
97-
if command -v yum &> /dev/null; then
98-
yum install -y libatomic.i686
99-
fi
10097
curl -sSf https://sh.rustup.rs | sh -s -- -y;
10198
fi
10299
CIBW_ENVIRONMENT_LINUX: PATH=$HOME/.cargo/bin:$PATH CMAKE_BUILD_PARALLEL_LEVEL=24 CMAKE_ARGS="-DNATIVE_TESTING=OFF" SETUPTOOLS_SCM_PRETEND_VERSION_FOR_DDTRACE=${{ needs.compute_version.outputs.library_version }}

‎ddtrace/internal/datadog/profiling/cmake/AnalysisFunc.cmake‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
include(CheckIPOSupported)
22

33
function(add_ddup_config target)
4-
# Profiling native extensions are built with C++17, even though underlying repo adheres to the manylinux 2014
4+
# Profiling native extensions are built with C++20, even though underlying repo adheres to the manylinux 2014
55
# standard. This isn't currently a problem, but if it becomes one, we may have to structure the library differently.
6-
target_compile_features(${target} PUBLIC cxx_std_17)
6+
target_compile_features(${target} PUBLIC cxx_std_20)
77

88
# Common compile options
99
target_compile_options(

‎ddtrace/internal/datadog/profiling/dd_wrapper/CMakeLists.txt‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,12 @@
11
cmake_minimum_required(VERSION 3.19)
2+
3+
# Disable testing by default (can be overridden with -DBUILD_TESTING=ON) Don't use FORCE to allow command-line overrides
4+
if(NOT DEFINED BUILD_TESTING)
5+
set(BUILD_TESTING
6+
OFF
7+
CACHE BOOL "Build the testing tree")
8+
endif()
9+
210
project(
311
dd_wrapper
412
VERSION 0.1.1

‎ddtrace/internal/datadog/profiling/dd_wrapper/test/CMakeLists.txt‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,23 @@ function(dd_wrapper_add_test name)
4040
add_ddup_config(${name})
4141
target_link_options(${name} PRIVATE -Wl,--no-as-needed)
4242

43+
# Test executable needs to find libdd_wrapper in parent directory Append to INSTALL_RPATH instead of replacing to
44+
# preserve sanitizer library paths set by add_ddup_config
45+
get_target_property(EXISTING_INSTALL_RPATH ${name} INSTALL_RPATH)
46+
if(EXISTING_INSTALL_RPATH)
47+
set_target_properties(${name} PROPERTIES INSTALL_RPATH "${EXISTING_INSTALL_RPATH};$ORIGIN/.."
48+
BUILD_WITH_INSTALL_RPATH TRUE)
49+
else()
50+
set_target_properties(${name} PROPERTIES INSTALL_RPATH "$ORIGIN/.." BUILD_WITH_INSTALL_RPATH TRUE)
51+
endif()
52+
4353
gtest_discover_tests(
44-
${name}
54+
${name} DISCOVERY_MODE PRE_TEST # Delay test discovery until test execution to avoid running sanitizer-built
55+
# executables during build
4556
PROPERTIES # We start new threads after fork(), and we want to continue running the tests after that instead of
4657
# dying.
4758
ENVIRONMENT "TSAN_OPTIONS=die_after_fork=0:suppressions=${CMAKE_CURRENT_SOURCE_DIR}/TSan.supp")
4859

49-
set_target_properties(${name} PROPERTIES INSTALL_RPATH "$ORIGIN/..")
50-
5160
if(LIB_INSTALL_DIR)
5261
install(TARGETS ${name} RUNTIME DESTINATION ${LIB_INSTALL_DIR}/../test)
5362
endif()

‎ddtrace/internal/datadog/profiling/ddup/CMakeLists.txt‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,23 @@ set_target_properties(${EXTENSION_NAME} PROPERTIES PREFIX "")
6868
set_target_properties(${EXTENSION_NAME} PROPERTIES SUFFIX "")
6969

7070
# RPATH is needed for sofile discovery at runtime, since Python packages are not installed in the system path. This is
71-
# typical.
71+
# typical. Use BUILD_WITH_INSTALL_RPATH to avoid rpath manipulation during install phase which can cause conflicts For
72+
# standalone/test builds, also include LIB_INSTALL_DIR so tests can find libdd_wrapper
7273
if(APPLE)
73-
set_target_properties(${EXTENSION_NAME} PROPERTIES INSTALL_RPATH "@loader_path/..")
74+
if(BUILD_TESTING AND LIB_INSTALL_DIR)
75+
set_target_properties(${EXTENSION_NAME} PROPERTIES INSTALL_RPATH "@loader_path/..;${LIB_INSTALL_DIR}"
76+
BUILD_WITH_INSTALL_RPATH TRUE)
77+
else()
78+
set_target_properties(${EXTENSION_NAME} PROPERTIES INSTALL_RPATH "@loader_path/.." BUILD_WITH_INSTALL_RPATH
79+
TRUE)
80+
endif()
7481
elseif(UNIX)
75-
set_target_properties(${EXTENSION_NAME} PROPERTIES INSTALL_RPATH "$ORIGIN/..")
82+
if(BUILD_TESTING AND LIB_INSTALL_DIR)
83+
set_target_properties(${EXTENSION_NAME} PROPERTIES INSTALL_RPATH "$ORIGIN/..;${LIB_INSTALL_DIR}"
84+
BUILD_WITH_INSTALL_RPATH TRUE)
85+
else()
86+
set_target_properties(${EXTENSION_NAME} PROPERTIES INSTALL_RPATH "$ORIGIN/.." BUILD_WITH_INSTALL_RPATH TRUE)
87+
endif()
7688
endif()
7789
target_include_directories(
7890
${EXTENSION_NAME}

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

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
cmake_minimum_required(VERSION 3.19)
22

3+
# Disable testing by default (can be overridden with -DBUILD_TESTING=ON) Don't use FORCE to allow command-line overrides
4+
if(NOT DEFINED BUILD_TESTING)
5+
set(BUILD_TESTING
6+
OFF
7+
CACHE BOOL "Build the testing tree")
8+
endif()
9+
310
# The exact name of this extension determines aspects of the installation and build paths, which need to be kept in sync
411
# with setup.py. Accordingly, take the name passed in by the caller, defaulting to "stack_v2" if needed.
512
set(EXTENSION_NAME
@@ -109,11 +116,23 @@ set_target_properties(${EXTENSION_NAME} PROPERTIES PREFIX "")
109116
set_target_properties(${EXTENSION_NAME} PROPERTIES SUFFIX "")
110117

111118
# RPATH is needed for sofile discovery at runtime, since Python packages are not installed in the system path. This is
112-
# typical.
119+
# typical. Use BUILD_WITH_INSTALL_RPATH to avoid rpath manipulation during install phase which can cause conflicts For
120+
# standalone/test builds, also include LIB_INSTALL_DIR so tests can find libdd_wrapper
113121
if(APPLE)
114-
set_target_properties(${EXTENSION_NAME} PROPERTIES INSTALL_RPATH "@loader_path/..")
122+
if(BUILD_TESTING AND LIB_INSTALL_DIR)
123+
set_target_properties(${EXTENSION_NAME} PROPERTIES INSTALL_RPATH "@loader_path/..;${LIB_INSTALL_DIR}"
124+
BUILD_WITH_INSTALL_RPATH TRUE)
125+
else()
126+
set_target_properties(${EXTENSION_NAME} PROPERTIES INSTALL_RPATH "@loader_path/.." BUILD_WITH_INSTALL_RPATH
127+
TRUE)
128+
endif()
115129
elseif(UNIX)
116-
set_target_properties(${EXTENSION_NAME} PROPERTIES INSTALL_RPATH "$ORIGIN/..")
130+
if(BUILD_TESTING AND LIB_INSTALL_DIR)
131+
set_target_properties(${EXTENSION_NAME} PROPERTIES INSTALL_RPATH "$ORIGIN/..;${LIB_INSTALL_DIR}"
132+
BUILD_WITH_INSTALL_RPATH TRUE)
133+
else()
134+
set_target_properties(${EXTENSION_NAME} PROPERTIES INSTALL_RPATH "$ORIGIN/.." BUILD_WITH_INSTALL_RPATH TRUE)
135+
endif()
117136
endif()
118137

119138
target_link_libraries(${EXTENSION_NAME} PRIVATE dd_wrapper Threads::Threads)

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,20 @@ function(dd_wrapper_add_test name)
4040
target_link_libraries(${name} PRIVATE ${Python3_LIBRARIES})
4141
endif()
4242

43-
set_target_properties(${name} PROPERTIES INSTALL_RPATH "$ORIGIN/../stack_v2")
44-
4543
add_ddup_config(${name})
4644

47-
gtest_discover_tests(${name})
45+
# Test executable needs to find _stack_v2 in parent directory Append to INSTALL_RPATH instead of replacing to
46+
# preserve sanitizer library paths set by add_ddup_config
47+
get_target_property(EXISTING_INSTALL_RPATH ${name} INSTALL_RPATH)
48+
if(EXISTING_INSTALL_RPATH)
49+
set_target_properties(${name} PROPERTIES INSTALL_RPATH "${EXISTING_INSTALL_RPATH};$ORIGIN/.."
50+
BUILD_WITH_INSTALL_RPATH TRUE)
51+
else()
52+
set_target_properties(${name} PROPERTIES INSTALL_RPATH "$ORIGIN/.." BUILD_WITH_INSTALL_RPATH TRUE)
53+
endif()
54+
55+
gtest_discover_tests(${name} DISCOVERY_MODE PRE_TEST) # Delay test discovery until test execution to avoid running
56+
# sanitizer-built executables during build
4857

4958
# This is supplemental artifact so make sure to install it in the right place
5059
if(INPLACE_LIB_INSTALL_DIR)

‎ddtrace/profiling/collector/_memalloc.c‎ renamed to ‎ddtrace/profiling/collector/_memalloc.cpp‎

File renamed without changes.

‎ddtrace/profiling/collector/_memalloc_heap.c‎ renamed to ‎ddtrace/profiling/collector/_memalloc_heap.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ heap_tracker_thaw_no_cpython(heap_tracker_t* heap_tracker, size_t* n_to_free)
156156
*n_to_free = heap_tracker->freezer.frees.count;
157157
if (*n_to_free > 0) {
158158
/* TODO: can we put traceback_t* directly in freezer.frees so we don't need new storage? */
159-
to_free = malloc(*n_to_free * sizeof(traceback_t*));
159+
to_free = static_cast<traceback_t**>(malloc(*n_to_free * sizeof(traceback_t*)));
160160
for (size_t i = 0; i < *n_to_free; i++) {
161161
traceback_t* tb = memalloc_heap_map_remove(heap_tracker->allocs_m, heap_tracker->freezer.frees.tab[i]);
162162
to_free[i] = tb;

‎ddtrace/profiling/collector/_memalloc_heap_map.c‎ renamed to ‎ddtrace/profiling/collector/_memalloc_heap_map.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ typedef struct memalloc_heap_map_iter_t
9090
memalloc_heap_map_t*
9191
memalloc_heap_map_new()
9292
{
93-
memalloc_heap_map_t* m = calloc(sizeof(memalloc_heap_map_t), 1);
93+
memalloc_heap_map_t* m = static_cast<memalloc_heap_map_t*>(calloc(sizeof(memalloc_heap_map_t), 1));
9494
m->map = HeapSamples_new(0);
9595
return m;
9696
}
@@ -104,7 +104,7 @@ memalloc_heap_map_size(memalloc_heap_map_t* m)
104104
traceback_t*
105105
memalloc_heap_map_insert(memalloc_heap_map_t* m, void* key, traceback_t* value)
106106
{
107-
HeapSamples_Entry k = { key = key, value = value };
107+
HeapSamples_Entry k = { .key = key, .val = value };
108108
HeapSamples_Insert res = HeapSamples_insert(&m->map, &k);
109109
traceback_t* prev = NULL;
110110
if (!res.inserted) {
@@ -187,7 +187,7 @@ memalloc_heap_map_delete(memalloc_heap_map_t* m)
187187
memalloc_heap_map_iter_t*
188188
memalloc_heap_map_iter_new(memalloc_heap_map_t* m)
189189
{
190-
memalloc_heap_map_iter_t* it = malloc(sizeof(memalloc_heap_map_iter_t));
190+
memalloc_heap_map_iter_t* it = static_cast<memalloc_heap_map_iter_t*>(malloc(sizeof(memalloc_heap_map_iter_t)));
191191
if (it) {
192192
it->iter = HeapSamples_citer(&m->map);
193193
}

0 commit comments

Comments
 (0)