From 88ff7b3b18afecf6c41e007ac0528b93fa289756 Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Tue, 23 Jul 2024 13:04:26 +0800 Subject: [PATCH 1/2] Revert "Enable heap profiling (#376)" This reverts commit 87b668286174b152df0f7ef4b738a5f46857eeff. --- Cargo.lock | 1 - .../proxy_ffi/src/jemalloc_utils.rs | 55 +++++++++---------- proxy_components/proxy_ffi/src/lib.rs | 2 +- proxy_components/proxy_server/Cargo.toml | 1 - .../proxy_server/src/status_server/mod.rs | 7 +-- .../proxy_server/src/status_server/profile.rs | 4 +- .../src/status_server/vendored_utils.rs | 50 ----------------- 7 files changed, 33 insertions(+), 87 deletions(-) delete mode 100644 proxy_components/proxy_server/src/status_server/vendored_utils.rs diff --git a/Cargo.lock b/Cargo.lock index e652e2c7cba..2a8fa5d7d63 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4695,7 +4695,6 @@ dependencies = [ "pprof", "prometheus", "protobuf", - "proxy_ffi", "raft", "raft_log_engine", "raftstore", diff --git a/proxy_components/proxy_ffi/src/jemalloc_utils.rs b/proxy_components/proxy_ffi/src/jemalloc_utils.rs index 210169ef535..cd6e4673c45 100644 --- a/proxy_components/proxy_ffi/src/jemalloc_utils.rs +++ b/proxy_components/proxy_ffi/src/jemalloc_utils.rs @@ -23,16 +23,13 @@ extern "C" { #[allow(unused_variables)] #[allow(unused_mut)] #[allow(unused_unsafe)] -pub fn issue_mallctl_args( - command: &str, - oldptr: *mut ::std::os::raw::c_void, - oldsize: *mut u64, - newptr: *mut ::std::os::raw::c_void, - newsize: u64, -) { +fn issue_mallctl(command: &str) -> u64 { + type PtrUnderlying = u64; + let mut ptr: PtrUnderlying = 0; + let mut size = std::mem::size_of::() as u64; + let c_str = std::ffi::CString::new(command).unwrap(); + let c_ptr: *const ::std::os::raw::c_char = c_str.as_ptr() as *const ::std::os::raw::c_char; unsafe { - let c_str = std::ffi::CString::new(command).unwrap(); - let c_ptr: *const ::std::os::raw::c_char = c_str.as_ptr() as *const ::std::os::raw::c_char; // See unprefixed_malloc_on_supported_platforms in tikv-jemalloc-sys. #[cfg(any(test, feature = "testexport"))] { @@ -40,13 +37,25 @@ pub fn issue_mallctl_args( { // See NO_UNPREFIXED_MALLOC #[cfg(any(target_os = "android", target_os = "dragonfly", target_os = "macos"))] - _rjem_mallctl(c_ptr, oldptr, oldsize, newptr, newsize); + _rjem_mallctl( + c_ptr, + &mut ptr as *mut _ as *mut ::std::os::raw::c_void, + &mut size as *mut u64, + std::ptr::null_mut(), + 0, + ); #[cfg(not(any( target_os = "android", target_os = "dragonfly", target_os = "macos" )))] - mallctl(c_ptr, oldptr, oldsize, newptr, newsize); + mallctl( + c_ptr, + &mut ptr as *mut _ as *mut ::std::os::raw::c_void, + &mut size as *mut u64, + std::ptr::null_mut(), + 0, + ); } } @@ -54,25 +63,15 @@ pub fn issue_mallctl_args( { // Must linked to tiflash. #[cfg(feature = "external-jemalloc")] - let r = mallctl(c_ptr, oldptr, oldsize, newptr, newsize); + mallctl( + c_ptr, + &mut ptr as *mut _ as *mut ::std::os::raw::c_void, + &mut size as *mut u64, + std::ptr::null_mut(), + 0, + ); } } -} - -#[allow(unused_variables)] -#[allow(unused_mut)] -#[allow(unused_unsafe)] -fn issue_mallctl(command: &str) -> u64 { - type PtrUnderlying = u64; - let mut ptr: PtrUnderlying = 0; - let mut size = std::mem::size_of::() as u64; - issue_mallctl_args( - command, - &mut ptr as *mut _ as *mut ::std::os::raw::c_void, - &mut size as *mut u64, - std::ptr::null_mut(), - 0, - ); ptr } diff --git a/proxy_components/proxy_ffi/src/lib.rs b/proxy_components/proxy_ffi/src/lib.rs index 31a28a701ca..fd61492e800 100644 --- a/proxy_components/proxy_ffi/src/lib.rs +++ b/proxy_components/proxy_ffi/src/lib.rs @@ -1,5 +1,5 @@ // Copyright 2022 TiKV Project Authors. Licensed under Apache-2.0. -#![feature(rustc_private)] + #![feature(thread_id_value)] /// All mods end up with `_impls` impl structs defined in interface. diff --git a/proxy_components/proxy_server/Cargo.toml b/proxy_components/proxy_server/Cargo.toml index f5053a7aaa9..57b89d7b8b0 100644 --- a/proxy_components/proxy_server/Cargo.toml +++ b/proxy_components/proxy_server/Cargo.toml @@ -55,7 +55,6 @@ encryption_export = { workspace = true, default-features = false } engine_rocks = { workspace = true, default-features = false } engine_rocks_helper = { workspace = true } service = { workspace = true } -proxy_ffi = { workspace = true, default-features = false } engine_store_ffi = { workspace = true, default-features = false } engine_tiflash = { workspace = true, default-features = false } engine_traits = { workspace = true, default-features = false } diff --git a/proxy_components/proxy_server/src/status_server/mod.rs b/proxy_components/proxy_server/src/status_server/mod.rs index d829f46a607..6542ffcf8ef 100644 --- a/proxy_components/proxy_server/src/status_server/mod.rs +++ b/proxy_components/proxy_server/src/status_server/mod.rs @@ -1,7 +1,6 @@ // Copyright 2018 TiKV Project Authors. Licensed under Apache-2.0. mod profile; -mod vendored_utils; use std::{ error::Error as StdError, @@ -691,9 +690,9 @@ where (Method::GET, "/debug/pprof/heap_deactivate") => { Self::deactivate_heap_prof(req) } - (Method::GET, "/debug/pprof/heap") => { - Self::dump_heap_prof_to_resp(req).await - } + // (Method::GET, "/debug/pprof/heap") => { + // Self::dump_heap_prof_to_resp(req).await + // } (Method::GET, "/config") => { Self::get_config(req, &cfg_controller, engine_store_server_helper) .await diff --git a/proxy_components/proxy_server/src/status_server/profile.rs b/proxy_components/proxy_server/src/status_server/profile.rs index 4a1400d65d6..b3d91d3bea6 100644 --- a/proxy_components/proxy_server/src/status_server/profile.rs +++ b/proxy_components/proxy_server/src/status_server/profile.rs @@ -21,14 +21,14 @@ use lazy_static::lazy_static; use pprof::protos::Message; use regex::Regex; use tempfile::{NamedTempFile, TempDir}; +#[cfg(not(test))] +use tikv_alloc::{activate_prof, deactivate_prof, dump_prof}; use tokio::sync::{Mutex, MutexGuard}; #[cfg(test)] pub use self::test_utils::TEST_PROFILE_MUTEX; #[cfg(test)] use self::test_utils::{activate_prof, deactivate_prof, dump_prof}; -#[cfg(not(test))] -use super::vendored_utils::{activate_prof, deactivate_prof, dump_prof}; // File name suffix for periodically dumped heap profiles. const HEAP_PROFILE_SUFFIX: &str = ".heap"; diff --git a/proxy_components/proxy_server/src/status_server/vendored_utils.rs b/proxy_components/proxy_server/src/status_server/vendored_utils.rs deleted file mode 100644 index c09391a4e4e..00000000000 --- a/proxy_components/proxy_server/src/status_server/vendored_utils.rs +++ /dev/null @@ -1,50 +0,0 @@ -// Copyright 2024 TiKV Project Authors. Licensed under Apache-2.0. - -use proxy_ffi::jemalloc_utils::issue_mallctl_args; -use tikv_alloc::error::ProfResult; - -pub fn activate_prof() -> ProfResult<()> { - { - let mut value: bool = true; - let len = std::mem::size_of_val(&value) as u64; - issue_mallctl_args( - "prof.active", - std::ptr::null_mut(), - std::ptr::null_mut(), - &mut value as *mut _ as *mut _, - len, - ); - } - Ok(()) -} - -pub fn deactivate_prof() -> ProfResult<()> { - { - let mut value: bool = false; - let len = std::mem::size_of_val(&value) as u64; - issue_mallctl_args( - "prof.active", - std::ptr::null_mut(), - std::ptr::null_mut(), - &mut value as *mut _ as *mut _, - len, - ); - } - Ok(()) -} - -pub fn dump_prof(path: &str) -> tikv_alloc::error::ProfResult<()> { - { - let mut bytes = std::ffi::CString::new(path)?.into_bytes_with_nul(); - let mut ptr = bytes.as_mut_ptr() as *mut ::std::os::raw::c_char; - let len = std::mem::size_of_val(&ptr) as u64; - issue_mallctl_args( - "prof.dump", - std::ptr::null_mut(), - std::ptr::null_mut(), - &mut ptr as *mut _ as *mut _, - len, - ); - } - Ok(()) -} From f1d6158de20a7e23613d1a468507503fd1b42612 Mon Sep 17 00:00:00 2001 From: CalvinNeo Date: Wed, 24 Jul 2024 22:49:35 +0800 Subject: [PATCH 2/2] Reapply "Enable heap profiling (#376)" This reverts commit 88ff7b3b18afecf6c41e007ac0528b93fa289756. --- Cargo.lock | 1 + .../proxy_ffi/src/jemalloc_utils.rs | 55 ++++++++++--------- proxy_components/proxy_ffi/src/lib.rs | 2 +- proxy_components/proxy_server/Cargo.toml | 1 + .../proxy_server/src/status_server/mod.rs | 7 ++- .../proxy_server/src/status_server/profile.rs | 4 +- .../src/status_server/vendored_utils.rs | 50 +++++++++++++++++ 7 files changed, 87 insertions(+), 33 deletions(-) create mode 100644 proxy_components/proxy_server/src/status_server/vendored_utils.rs diff --git a/Cargo.lock b/Cargo.lock index 2a8fa5d7d63..e652e2c7cba 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4695,6 +4695,7 @@ dependencies = [ "pprof", "prometheus", "protobuf", + "proxy_ffi", "raft", "raft_log_engine", "raftstore", diff --git a/proxy_components/proxy_ffi/src/jemalloc_utils.rs b/proxy_components/proxy_ffi/src/jemalloc_utils.rs index cd6e4673c45..210169ef535 100644 --- a/proxy_components/proxy_ffi/src/jemalloc_utils.rs +++ b/proxy_components/proxy_ffi/src/jemalloc_utils.rs @@ -23,13 +23,16 @@ extern "C" { #[allow(unused_variables)] #[allow(unused_mut)] #[allow(unused_unsafe)] -fn issue_mallctl(command: &str) -> u64 { - type PtrUnderlying = u64; - let mut ptr: PtrUnderlying = 0; - let mut size = std::mem::size_of::() as u64; - let c_str = std::ffi::CString::new(command).unwrap(); - let c_ptr: *const ::std::os::raw::c_char = c_str.as_ptr() as *const ::std::os::raw::c_char; +pub fn issue_mallctl_args( + command: &str, + oldptr: *mut ::std::os::raw::c_void, + oldsize: *mut u64, + newptr: *mut ::std::os::raw::c_void, + newsize: u64, +) { unsafe { + let c_str = std::ffi::CString::new(command).unwrap(); + let c_ptr: *const ::std::os::raw::c_char = c_str.as_ptr() as *const ::std::os::raw::c_char; // See unprefixed_malloc_on_supported_platforms in tikv-jemalloc-sys. #[cfg(any(test, feature = "testexport"))] { @@ -37,25 +40,13 @@ fn issue_mallctl(command: &str) -> u64 { { // See NO_UNPREFIXED_MALLOC #[cfg(any(target_os = "android", target_os = "dragonfly", target_os = "macos"))] - _rjem_mallctl( - c_ptr, - &mut ptr as *mut _ as *mut ::std::os::raw::c_void, - &mut size as *mut u64, - std::ptr::null_mut(), - 0, - ); + _rjem_mallctl(c_ptr, oldptr, oldsize, newptr, newsize); #[cfg(not(any( target_os = "android", target_os = "dragonfly", target_os = "macos" )))] - mallctl( - c_ptr, - &mut ptr as *mut _ as *mut ::std::os::raw::c_void, - &mut size as *mut u64, - std::ptr::null_mut(), - 0, - ); + mallctl(c_ptr, oldptr, oldsize, newptr, newsize); } } @@ -63,15 +54,25 @@ fn issue_mallctl(command: &str) -> u64 { { // Must linked to tiflash. #[cfg(feature = "external-jemalloc")] - mallctl( - c_ptr, - &mut ptr as *mut _ as *mut ::std::os::raw::c_void, - &mut size as *mut u64, - std::ptr::null_mut(), - 0, - ); + let r = mallctl(c_ptr, oldptr, oldsize, newptr, newsize); } } +} + +#[allow(unused_variables)] +#[allow(unused_mut)] +#[allow(unused_unsafe)] +fn issue_mallctl(command: &str) -> u64 { + type PtrUnderlying = u64; + let mut ptr: PtrUnderlying = 0; + let mut size = std::mem::size_of::() as u64; + issue_mallctl_args( + command, + &mut ptr as *mut _ as *mut ::std::os::raw::c_void, + &mut size as *mut u64, + std::ptr::null_mut(), + 0, + ); ptr } diff --git a/proxy_components/proxy_ffi/src/lib.rs b/proxy_components/proxy_ffi/src/lib.rs index fd61492e800..31a28a701ca 100644 --- a/proxy_components/proxy_ffi/src/lib.rs +++ b/proxy_components/proxy_ffi/src/lib.rs @@ -1,5 +1,5 @@ // Copyright 2022 TiKV Project Authors. Licensed under Apache-2.0. - +#![feature(rustc_private)] #![feature(thread_id_value)] /// All mods end up with `_impls` impl structs defined in interface. diff --git a/proxy_components/proxy_server/Cargo.toml b/proxy_components/proxy_server/Cargo.toml index 57b89d7b8b0..f5053a7aaa9 100644 --- a/proxy_components/proxy_server/Cargo.toml +++ b/proxy_components/proxy_server/Cargo.toml @@ -55,6 +55,7 @@ encryption_export = { workspace = true, default-features = false } engine_rocks = { workspace = true, default-features = false } engine_rocks_helper = { workspace = true } service = { workspace = true } +proxy_ffi = { workspace = true, default-features = false } engine_store_ffi = { workspace = true, default-features = false } engine_tiflash = { workspace = true, default-features = false } engine_traits = { workspace = true, default-features = false } diff --git a/proxy_components/proxy_server/src/status_server/mod.rs b/proxy_components/proxy_server/src/status_server/mod.rs index 6542ffcf8ef..d829f46a607 100644 --- a/proxy_components/proxy_server/src/status_server/mod.rs +++ b/proxy_components/proxy_server/src/status_server/mod.rs @@ -1,6 +1,7 @@ // Copyright 2018 TiKV Project Authors. Licensed under Apache-2.0. mod profile; +mod vendored_utils; use std::{ error::Error as StdError, @@ -690,9 +691,9 @@ where (Method::GET, "/debug/pprof/heap_deactivate") => { Self::deactivate_heap_prof(req) } - // (Method::GET, "/debug/pprof/heap") => { - // Self::dump_heap_prof_to_resp(req).await - // } + (Method::GET, "/debug/pprof/heap") => { + Self::dump_heap_prof_to_resp(req).await + } (Method::GET, "/config") => { Self::get_config(req, &cfg_controller, engine_store_server_helper) .await diff --git a/proxy_components/proxy_server/src/status_server/profile.rs b/proxy_components/proxy_server/src/status_server/profile.rs index b3d91d3bea6..4a1400d65d6 100644 --- a/proxy_components/proxy_server/src/status_server/profile.rs +++ b/proxy_components/proxy_server/src/status_server/profile.rs @@ -21,14 +21,14 @@ use lazy_static::lazy_static; use pprof::protos::Message; use regex::Regex; use tempfile::{NamedTempFile, TempDir}; -#[cfg(not(test))] -use tikv_alloc::{activate_prof, deactivate_prof, dump_prof}; use tokio::sync::{Mutex, MutexGuard}; #[cfg(test)] pub use self::test_utils::TEST_PROFILE_MUTEX; #[cfg(test)] use self::test_utils::{activate_prof, deactivate_prof, dump_prof}; +#[cfg(not(test))] +use super::vendored_utils::{activate_prof, deactivate_prof, dump_prof}; // File name suffix for periodically dumped heap profiles. const HEAP_PROFILE_SUFFIX: &str = ".heap"; diff --git a/proxy_components/proxy_server/src/status_server/vendored_utils.rs b/proxy_components/proxy_server/src/status_server/vendored_utils.rs new file mode 100644 index 00000000000..c09391a4e4e --- /dev/null +++ b/proxy_components/proxy_server/src/status_server/vendored_utils.rs @@ -0,0 +1,50 @@ +// Copyright 2024 TiKV Project Authors. Licensed under Apache-2.0. + +use proxy_ffi::jemalloc_utils::issue_mallctl_args; +use tikv_alloc::error::ProfResult; + +pub fn activate_prof() -> ProfResult<()> { + { + let mut value: bool = true; + let len = std::mem::size_of_val(&value) as u64; + issue_mallctl_args( + "prof.active", + std::ptr::null_mut(), + std::ptr::null_mut(), + &mut value as *mut _ as *mut _, + len, + ); + } + Ok(()) +} + +pub fn deactivate_prof() -> ProfResult<()> { + { + let mut value: bool = false; + let len = std::mem::size_of_val(&value) as u64; + issue_mallctl_args( + "prof.active", + std::ptr::null_mut(), + std::ptr::null_mut(), + &mut value as *mut _ as *mut _, + len, + ); + } + Ok(()) +} + +pub fn dump_prof(path: &str) -> tikv_alloc::error::ProfResult<()> { + { + let mut bytes = std::ffi::CString::new(path)?.into_bytes_with_nul(); + let mut ptr = bytes.as_mut_ptr() as *mut ::std::os::raw::c_char; + let len = std::mem::size_of_val(&ptr) as u64; + issue_mallctl_args( + "prof.dump", + std::ptr::null_mut(), + std::ptr::null_mut(), + &mut ptr as *mut _ as *mut _, + len, + ); + } + Ok(()) +}