From 9c30987ac234e3b324a36f174767eb5280bf0688 Mon Sep 17 00:00:00 2001 From: Gregor Maier Date: Sat, 24 Oct 2026 20:58:30 +0000 Subject: [PATCH] Always apply memory profiler settings When jemalloc memory profiling was enabled by `_RJEM_MALLOC_CONF="prof:true"`, jemallocs default is start taking profile samples (because prof_active is defaulted to true by jemalloc). Buf if `telemetry.memory_profiler.enabled == false`, foundations did not apply any profiler settings, thus leaving profiling enabled with the default sampling rate of 512KB. This change always applies the settings and adds tests for it. Since testing involves spawning a new process, there's a small binary to help with that. --- foundations/Cargo.toml | 7 +++ foundations/src/telemetry/memory_profiler.rs | 29 ++++++++---- foundations/src/telemetry/mod.rs | 6 +++ foundations/src/telemetry/server/mod.rs | 7 --- .../helpers/memory_profiler_test_helper.rs | 47 +++++++++++++++++++ foundations/tests/memory_profiler.rs | 43 +++++++++++++++++ 6 files changed, 122 insertions(+), 17 deletions(-) create mode 100644 foundations/tests/helpers/memory_profiler_test_helper.rs create mode 100644 foundations/tests/memory_profiler.rs diff --git a/foundations/Cargo.toml b/foundations/Cargo.toml index 22e44d73..f483a8c0 100644 --- a/foundations/Cargo.toml +++ b/foundations/Cargo.toml @@ -233,6 +233,13 @@ rustdoc-args = ["--cfg", "foundations_docsrs", "--cfg", "tokio_unstable", "--cfg # to rustc, or else dependencies will not be enabled, and the docs build will fail. rustc-args = ["--cfg", "tokio_unstable", "--cfg", "foundations_unstable"] +[[bin]] +name = "memory-profiler-test-helper" +path = "tests/helpers/memory_profiler_test_helper.rs" +required-features = ["memory-profiling"] +test = false +bench = false + [lints] workspace = true diff --git a/foundations/src/telemetry/memory_profiler.rs b/foundations/src/telemetry/memory_profiler.rs index 32cb12c3..32455b5b 100644 --- a/foundations/src/telemetry/memory_profiler.rs +++ b/foundations/src/telemetry/memory_profiler.rs @@ -65,8 +65,9 @@ impl MemoryProfiler { /// # Enabling profiling /// /// Note that profiling needs to be explicitly enabled by setting `_RJEM_MALLOC_CONF=prof:true` - /// environment variable for the binary and with [`MemoryProfilerSettings::enabled`] being set - /// to `true`. Otherwise, this method will return `None`. + /// environment variable for the binary. When profiling is available, this method applies the + /// configured sample interval and activation state. It returns `None` when profiling is + /// unavailable or [`MemoryProfilerSettings::enabled`] is `false`. /// /// If syscall sandboxing is being used (see [`crate::security`] for more details), telemetry /// must be initialized prior to syscall sandboxing. @@ -152,23 +153,31 @@ impl MemoryProfiler { } fn init_profiler(settings: &MemoryProfilerSettings) -> BootstrapResult> { - if !settings.enabled || !control::profiling_enabled() { + if !control::profiling_enabled() { return Ok(None); } - let (request_sender, request_receiver) = mpsc::channel(); - HEAP_PROFILE_REQUEST_SENDER - .set(AsyncMutex::new(request_sender)) - .map_err(|_| anyhow::anyhow!("request sender had already been initialized"))?; + // Disable profiling before resetting it so no samples are collected using stale settings. + control::write(control::PROF_ACTIVE, false) + .map_err(|e| BootstrapError::new(e).context("failed to deactivate profiling"))?; - std::thread::spawn(move || heap_profile_thread(request_receiver)); + control::write(control::PROF_RESET, usize::from(settings.sample_interval)) + .map_err(|e| BootstrapError::new(e).context("failed to set sample interval"))?; + + if !settings.enabled { + return Ok(None); + } control::write(control::BACKGROUND_THREAD, true).map_err(|e| { BootstrapError::new(e).context("failed to activate background thread collection") })?; - control::write(control::PROF_RESET, settings.sample_interval as u64) - .map_err(|e| BootstrapError::new(e).context("failed to set sample interval"))?; + let (request_sender, request_receiver) = mpsc::channel(); + HEAP_PROFILE_REQUEST_SENDER + .set(AsyncMutex::new(request_sender)) + .map_err(|_| anyhow::anyhow!("request sender had already been initialized"))?; + + std::thread::spawn(move || heap_profile_thread(request_receiver)); control::write(control::PROF_ACTIVE, true) .map_err(|e| BootstrapError::new(e).context("failed to activate profiling"))?; diff --git a/foundations/src/telemetry/mod.rs b/foundations/src/telemetry/mod.rs index a30bf6b5..cde3e1ce 100644 --- a/foundations/src/telemetry/mod.rs +++ b/foundations/src/telemetry/mod.rs @@ -343,6 +343,12 @@ pub fn init(config: TelemetryConfig) -> BootstrapResult { ) } + // Eagerly init the memory profiler so it gets set up before syscalls are sandboxed with seccomp. + // Apply its settings even when memory_profiler.enabled or server.enabled is false + // to ensure profiling is disabled + #[cfg(all(target_os = "linux", feature = "memory-profiling"))] + let _ = MemoryProfiler::get_or_init_with(&config.settings.memory_profiler)?; + #[cfg(feature = "metrics")] self::metrics::init::init(config.service_info, &config.settings.metrics)?; diff --git a/foundations/src/telemetry/server/mod.rs b/foundations/src/telemetry/server/mod.rs index 70ee7604..f2a5b5b1 100644 --- a/foundations/src/telemetry/server/mod.rs +++ b/foundations/src/telemetry/server/mod.rs @@ -151,13 +151,6 @@ impl TelemetryServerFuture { let settings = Arc::new(settings); - // Eagerly init the memory profiler so it gets set up before syscalls are sandboxed with seccomp. - #[cfg(all(target_os = "linux", feature = "memory-profiling"))] - if settings.memory_profiler.enabled { - memory_profiling::profiler(Arc::clone(&settings)) - .map_err(|err| anyhow::anyhow!(err))?; - } - let router = Router::new(custom_routes, Arc::clone(&settings)) .context("building telemetry server router")?; diff --git a/foundations/tests/helpers/memory_profiler_test_helper.rs b/foundations/tests/helpers/memory_profiler_test_helper.rs new file mode 100644 index 00000000..063c2816 --- /dev/null +++ b/foundations/tests/helpers/memory_profiler_test_helper.rs @@ -0,0 +1,47 @@ +// Run profiler initialization in a fresh process so each test controls jemalloc's startup state. +#[cfg(target_os = "linux")] +fn main() { + use foundations::telemetry::MemoryProfiler; + use foundations::telemetry::settings::MemoryProfilerSettings; + use std::ffi::CStr; + + const PROF_ACTIVE: &CStr = cstr(b"prof.active\0"); + const PROF_LG_SAMPLE: &CStr = cstr(b"prof.lg_sample\0"); + + let enabled = match std::env::args().nth(1).as_deref() { + Some("enabled") => true, + Some("disabled") => false, + _ => panic!("expected `enabled` or `disabled` argument"), + }; + + let profiler = MemoryProfiler::get_or_init_with(&MemoryProfilerSettings { + enabled, + sample_interval: 17, + }) + .expect("failed to initialize memory profiler"); + + let active: bool = read_jemalloc_setting(PROF_ACTIVE); + let sample_interval: usize = read_jemalloc_setting(PROF_LG_SAMPLE); + + println!( + "profiler={} active={active} sample_interval={sample_interval}", + profiler.is_some() + ); +} + +#[cfg(target_os = "linux")] +fn read_jemalloc_setting(name: &std::ffi::CStr) -> T { + unsafe { tikv_jemalloc_ctl::raw::read(name.to_bytes_with_nul()) } + .expect("failed to read jemalloc setting") +} + +#[cfg(target_os = "linux")] +const fn cstr(bytes: &[u8]) -> &std::ffi::CStr { + match std::ffi::CStr::from_bytes_until_nul(bytes) { + Ok(value) => value, + Err(_) => panic!("control name should be null-terminated"), + } +} + +#[cfg(not(target_os = "linux"))] +fn main() {} diff --git a/foundations/tests/memory_profiler.rs b/foundations/tests/memory_profiler.rs new file mode 100644 index 00000000..8bc72135 --- /dev/null +++ b/foundations/tests/memory_profiler.rs @@ -0,0 +1,43 @@ +#![cfg(all(target_os = "linux", feature = "memory-profiling"))] + +use std::process::Command; + +fn run_helper(malloc_conf: &str, enabled: bool) -> String { + let mut command = Command::new(env!("CARGO_BIN_EXE_memory-profiler-test-helper")); + command + .arg(if enabled { "enabled" } else { "disabled" }) + .env("_RJEM_MALLOC_CONF", malloc_conf); + + let output = command + .output() + .expect("failed to run memory profiler test helper"); + + assert!( + output.status.success(), + "memory profiler test helper failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + String::from_utf8(output.stdout) + .expect("helper output should be UTF-8") + .trim() + .to_owned() +} + +#[test] +fn disabled_settings_are_applied() { + // Disabled application settings must override an allocator configured as active at startup. + assert_eq!( + run_helper("prof:true,prof_active:true,lg_prof_sample:19", false), + "profiler=false active=false sample_interval=17" + ); +} + +#[test] +fn enabled_settings_activate_inactive_profiler() { + // Enabled application settings must activate the allocator and replace its sample interval. + assert_eq!( + run_helper("prof:true,prof_active:false,lg_prof_sample:19", true), + "profiler=true active=true sample_interval=17" + ); +}