From 47112d4b368c24cec5135b3d2a3347180038628b Mon Sep 17 00:00:00 2001 From: Xinyi Zou Date: Tue, 17 Dec 2024 20:55:04 +0800 Subject: [PATCH] [revert](memory) Revert disable Jemalloc Hook (#45534) ### What problem does this PR solve? It is not easy to remove `with-jemalloc-prefix`, which may affect the compatibility between third-party and old version codes. Also, will building failed on Mac, it said can't find mallctl symbol. because jemalloc's default prefix on macOS is "je_", not "". Maybe can use alias instead of override. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label --- be/src/http/default_path_handlers.cpp | 2 +- be/src/runtime/CMakeLists.txt | 4 +++- be/src/runtime/memory/heap_profiler.cpp | 8 ++++---- be/src/util/mem_info.cpp | 2 +- be/src/util/mem_info.h | 10 +++++----- be/src/vec/common/allocator.h | 2 +- build.sh | 20 +++++++++++++++++++- cloud/src/common/CMakeLists.txt | 6 ++++++ thirdparty/build-thirdparty.sh | 7 ++++--- 9 files changed, 44 insertions(+), 17 deletions(-) diff --git a/be/src/http/default_path_handlers.cpp b/be/src/http/default_path_handlers.cpp index b26ba67b54625c..04e1121cab63ba 100644 --- a/be/src/http/default_path_handlers.cpp +++ b/be/src/http/default_path_handlers.cpp @@ -146,7 +146,7 @@ void memory_info_handler(std::stringstream* output) { auto* _opaque = static_cast(opaque); _opaque->append(buf); }; - malloc_stats_print(write_cb, &tmp, "a"); + jemalloc_stats_print(write_cb, &tmp, "a"); boost::replace_all(tmp, "\n", "
"); (*output) << tmp; #else diff --git a/be/src/runtime/CMakeLists.txt b/be/src/runtime/CMakeLists.txt index ab380f9711f1ac..a0b3b799a764cb 100644 --- a/be/src/runtime/CMakeLists.txt +++ b/be/src/runtime/CMakeLists.txt @@ -25,7 +25,9 @@ set(EXECUTABLE_OUTPUT_PATH "${BUILD_DIR}/src/runtime") file(GLOB_RECURSE RUNTIME_FILES CONFIGURE_DEPENDS *.cpp *.cc) -list(REMOVE_ITEM RUNTIME_FILES ${CMAKE_CURRENT_SOURCE_DIR}/memory/jemalloc_hook.cpp) +if (NOT USE_JEMALLOC OR NOT USE_MEM_TRACKER) + list(REMOVE_ITEM RUNTIME_FILES ${CMAKE_CURRENT_SOURCE_DIR}/memory/jemalloc_hook.cpp) +endif() add_library(Runtime STATIC ${RUNTIME_FILES} diff --git a/be/src/runtime/memory/heap_profiler.cpp b/be/src/runtime/memory/heap_profiler.cpp index 0b0448ce0eaaf6..01ed82f76ef6d1 100644 --- a/be/src/runtime/memory/heap_profiler.cpp +++ b/be/src/runtime/memory/heap_profiler.cpp @@ -30,8 +30,8 @@ void HeapProfiler::set_prof_active(bool prof) { #ifdef USE_JEMALLOC std::lock_guard guard(_mutex); try { - int err = mallctl("prof.active", nullptr, nullptr, &prof, 1); - err |= mallctl("prof.thread_active_init", nullptr, nullptr, &prof, 1); + int err = jemallctl("prof.active", nullptr, nullptr, &prof, 1); + err |= jemallctl("prof.thread_active_init", nullptr, nullptr, &prof, 1); if (err) { LOG(WARNING) << "jemalloc heap profiling start failed, " << err; } else { @@ -48,7 +48,7 @@ bool HeapProfiler::get_prof_dump(const std::string& profile_file_name) { std::lock_guard guard(_mutex); const char* file_name_ptr = profile_file_name.c_str(); try { - int err = mallctl("prof.dump", nullptr, nullptr, &file_name_ptr, sizeof(const char*)); + int err = jemallctl("prof.dump", nullptr, nullptr, &file_name_ptr, sizeof(const char*)); if (err) { LOG(WARNING) << "dump heap profile failed, " << err; return false; @@ -93,7 +93,7 @@ bool HeapProfiler::check_heap_profiler() { #ifdef USE_JEMALLOC size_t value = 0; size_t sz = sizeof(value); - mallctl("prof.active", &value, &sz, nullptr, 0); + jemallctl("prof.active", &value, &sz, nullptr, 0); return value; #else return false; diff --git a/be/src/util/mem_info.cpp b/be/src/util/mem_info.cpp index 97e529ac6c72e1..fe9cf84b2aed54 100644 --- a/be/src/util/mem_info.cpp +++ b/be/src/util/mem_info.cpp @@ -101,7 +101,7 @@ void MemInfo::refresh_allocator_mem() { // the current epoch number, which might be useful to log as a sanity check. uint64_t epoch = 0; size_t sz = sizeof(epoch); - mallctl("epoch", &epoch, &sz, &epoch, sz); + jemallctl("epoch", &epoch, &sz, &epoch, sz); // Number of extents of the given type in this arena in the bucket corresponding to page size index. // Large size class starts at 16384, the extents have three sizes before 16384: 4096, 8192, and 12288, so + 3 diff --git a/be/src/util/mem_info.h b/be/src/util/mem_info.h index 0e14b64bd8f965..39ae9eb0b79cfb 100644 --- a/be/src/util/mem_info.h +++ b/be/src/util/mem_info.h @@ -103,7 +103,7 @@ class MemInfo { #ifdef USE_JEMALLOC size_t value = 0; size_t sz = sizeof(value); - if (mallctl(name.c_str(), &value, &sz, nullptr, 0) == 0) { + if (jemallctl(name.c_str(), &value, &sz, nullptr, 0) == 0) { return value; } #endif @@ -114,7 +114,7 @@ class MemInfo { #ifdef USE_JEMALLOC unsigned value = 0; size_t sz = sizeof(value); - if (mallctl(name.c_str(), &value, &sz, nullptr, 0) == 0) { + if (jemallctl(name.c_str(), &value, &sz, nullptr, 0) == 0) { return value; } #endif @@ -146,8 +146,8 @@ class MemInfo { if (config::enable_je_purge_dirty_pages) { try { // Purge all unused dirty pages for arena , or for all arenas if equals MALLCTL_ARENAS_ALL. - int err = mallctl(fmt::format("arena.{}.purge", MALLCTL_ARENAS_ALL).c_str(), - nullptr, nullptr, nullptr, 0); + int err = jemallctl(fmt::format("arena.{}.purge", MALLCTL_ARENAS_ALL).c_str(), + nullptr, nullptr, nullptr, 0); if (err) { LOG(WARNING) << "Jemalloc purge all unused dirty pages failed"; } @@ -166,7 +166,7 @@ class MemInfo { #ifdef USE_JEMALLOC constexpr size_t TCACHE_LIMIT = (1ULL << 30); // 1G if (allocator_cache_mem() - je_dirty_pages_mem() > TCACHE_LIMIT) { - int err = mallctl("thread.tcache.flush", nullptr, nullptr, nullptr, 0); + int err = jemallctl("thread.tcache.flush", nullptr, nullptr, nullptr, 0); if (err) { LOG(WARNING) << "Jemalloc thread.tcache.flush failed"; } diff --git a/be/src/vec/common/allocator.h b/be/src/vec/common/allocator.h index a0ab9c8c0a8207..b05128bc6933cc 100644 --- a/be/src/vec/common/allocator.h +++ b/be/src/vec/common/allocator.h @@ -109,7 +109,7 @@ class DefaultMemoryAllocator { static void release_unused() { #if defined(USE_JEMALLOC) - mallctl(fmt::format("arena.{}.purge", MALLCTL_ARENAS_ALL).c_str(), NULL, NULL, NULL, 0); + jemallctl(fmt::format("arena.{}.purge", MALLCTL_ARENAS_ALL).c_str(), NULL, NULL, NULL, 0); #endif // defined(USE_JEMALLOC) } }; diff --git a/build.sh b/build.sh index 89a22842b7d100..5a09b7a0a165e3 100755 --- a/build.sh +++ b/build.sh @@ -375,7 +375,25 @@ BUILD_TYPE_LOWWER=$(echo "${BUILD_TYPE}" | tr '[:upper:]' '[:lower:]') if [[ "${BUILD_TYPE_LOWWER}" == "asan" ]]; then USE_JEMALLOC='OFF' elif [[ -z "${USE_JEMALLOC}" ]]; then - USE_JEMALLOC='ON' + if [[ "$(uname -s)" != 'Darwin' ]]; then + USE_JEMALLOC='ON' + else + USE_JEMALLOC='OFF' + fi +fi +if [[ -f "${TP_INCLUDE_DIR}/jemalloc/jemalloc_doris_with_prefix.h" ]]; then + # compatible with old thirdparty + rm -rf "${TP_INCLUDE_DIR}/jemalloc/jemalloc.h" + rm -rf "${TP_LIB_DIR}/libjemalloc_doris.a" + rm -rf "${TP_LIB_DIR}/libjemalloc_doris_pic.a" + rm -rf "${TP_INCLUDE_DIR}/rocksdb" + rm -rf "${TP_LIB_DIR}/librocksdb.a" + + mv "${TP_INCLUDE_DIR}/jemalloc/jemalloc_doris_with_prefix.h" "${TP_INCLUDE_DIR}/jemalloc/jemalloc.h" + mv "${TP_LIB_DIR}/libjemalloc_doris_with_prefix.a" "${TP_LIB_DIR}/libjemalloc_doris.a" + mv "${TP_LIB_DIR}/libjemalloc_doris_with_prefix_pic.a" "${TP_LIB_DIR}/libjemalloc_doris_pic.a" + mv "${TP_LIB_DIR}/librocksdb_jemalloc_with_prefix.a" "${TP_LIB_DIR}/librocksdb.a" + mv -f "${TP_INCLUDE_DIR}/rocksdb_jemalloc_with_prefix" "${TP_INCLUDE_DIR}/rocksdb" fi if [[ -z "${USE_BTHREAD_SCANNER}" ]]; then USE_BTHREAD_SCANNER='OFF' diff --git a/cloud/src/common/CMakeLists.txt b/cloud/src/common/CMakeLists.txt index 008e8906044d51..429748909bebd9 100644 --- a/cloud/src/common/CMakeLists.txt +++ b/cloud/src/common/CMakeLists.txt @@ -15,6 +15,12 @@ set(COMMON_FILES network_util.cpp ) +if (USE_JEMALLOC) + set(COMMON_FILES ${COMMON_FILES} + jemalloc_hook.cpp + ) +endif() + add_library(Common STATIC ${COMMON_FILES} ) diff --git a/thirdparty/build-thirdparty.sh b/thirdparty/build-thirdparty.sh index a209977769c278..333787cb64d808 100755 --- a/thirdparty/build-thirdparty.sh +++ b/thirdparty/build-thirdparty.sh @@ -1529,10 +1529,11 @@ build_jemalloc_doris() { WITH_LG_PAGE='' fi - # CFLAGS="${cflags}" ../configure --prefix="${TP_INSTALL_DIR}" --with-install-suffix="_doris" "${WITH_LG_PAGE}" \ - # --with-jemalloc-prefix=je --enable-prof --disable-cxx --disable-libdl --disable-shared + # It is not easy to remove `with-jemalloc-prefix`, which may affect the compatibility between third-party and old version codes. + # Also, will building failed on Mac, it said can't find mallctl symbol. because jemalloc's default prefix on macOS is "je_", not "". + # Maybe can use alias instead of overwrite. CFLAGS="${cflags}" ../configure --prefix="${TP_INSTALL_DIR}" --with-install-suffix="_doris" "${WITH_LG_PAGE}" \ - --enable-prof --disable-libdl --disable-shared + --with-jemalloc-prefix=je --enable-prof --disable-cxx --disable-libdl --disable-shared make -j "${PARALLEL}" make install