From 3405b4e980c26b360f49dd998262af540799253a Mon Sep 17 00:00:00 2001 From: houseme Date: Tue, 4 Aug 2026 23:20:49 +0800 Subject: [PATCH] perf(observability): avoid sampler stat allocations (#5708) * perf(observability): avoid cgroup stat key allocations Replace the memory.stat HashMap parser with a fixed-field parser so the memory observability sampler does not allocate String keys or hash every cgroup field on each interval. Co-Authored-By: heihutu * perf(observability): parse mimalloc stats without copying Parse the mimalloc stats JSON while the mimalloc-owned buffer is still alive, then free it immediately. This avoids allocating an owned String on each allocator memory sample. Co-Authored-By: heihutu --------- Co-authored-by: heihutu --- rustfs/src/memory_observability.rs | 99 ++++++++++++++++++------------ 1 file changed, 61 insertions(+), 38 deletions(-) diff --git a/rustfs/src/memory_observability.rs b/rustfs/src/memory_observability.rs index a5e930985..13c6f1ea0 100644 --- a/rustfs/src/memory_observability.rs +++ b/rustfs/src/memory_observability.rs @@ -19,7 +19,6 @@ use rustfs_io_metrics::{ use serde::Serialize; #[cfg(any(test, not(target_os = "windows")))] use serde_json::Value; -use std::collections::HashMap; #[cfg(not(target_os = "windows"))] use std::ffi::CStr; use std::path::Path; @@ -131,6 +130,20 @@ struct CgroupMemorySnapshot { inactive_file_bytes: Option, } +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +struct CgroupMemoryStatFields { + anon: Option, + file: Option, + active_file: Option, + inactive_file: Option, + rss: Option, + cache: Option, + total_rss: Option, + total_cache: Option, + total_active_file: Option, + total_inactive_file: Option, +} + #[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] struct AllocatorMemorySnapshot { backend: &'static str, @@ -156,16 +169,32 @@ fn read_optional_u64(path: &Path) -> Option { trimmed.parse::().ok() } -fn parse_kv_stats(content: &str) -> HashMap { - content - .lines() - .filter_map(|line| { - let mut parts = line.split_whitespace(); - let key = parts.next()?; - let value = parts.next()?.parse::().ok()?; - Some((key.to_string(), value)) - }) - .collect() +fn parse_cgroup_memory_stat(content: &str) -> CgroupMemoryStatFields { + let mut fields = CgroupMemoryStatFields::default(); + for line in content.lines() { + let mut parts = line.split_whitespace(); + let Some(key) = parts.next() else { + continue; + }; + let Some(value) = parts.next().and_then(|value| value.parse::().ok()) else { + continue; + }; + + match key { + "anon" => fields.anon = Some(value), + "file" => fields.file = Some(value), + "active_file" => fields.active_file = Some(value), + "inactive_file" => fields.inactive_file = Some(value), + "rss" => fields.rss = Some(value), + "cache" => fields.cache = Some(value), + "total_rss" => fields.total_rss = Some(value), + "total_cache" => fields.total_cache = Some(value), + "total_active_file" => fields.total_active_file = Some(value), + "total_inactive_file" => fields.total_inactive_file = Some(value), + _ => {} + } + } + fields } fn read_cgroup_v2() -> Option { @@ -175,14 +204,14 @@ fn read_cgroup_v2() -> Option { return None; } - let stats = parse_kv_stats(&std::fs::read_to_string(&stat_path).ok()?); + let stats = parse_cgroup_memory_stat(&std::fs::read_to_string(&stat_path).ok()?); Some(CgroupMemorySnapshot { current_bytes: read_optional_u64(&root.join("memory.current")), limit_bytes: read_optional_u64(&root.join("memory.max")), - anon_bytes: stats.get("anon").copied(), - file_bytes: stats.get("file").copied(), - active_file_bytes: stats.get("active_file").copied(), - inactive_file_bytes: stats.get("inactive_file").copied(), + anon_bytes: stats.anon, + file_bytes: stats.file, + active_file_bytes: stats.active_file, + inactive_file_bytes: stats.inactive_file, }) } @@ -193,20 +222,14 @@ fn read_cgroup_v1() -> Option { return None; } - let stats = parse_kv_stats(&std::fs::read_to_string(&stat_path).ok()?); + let stats = parse_cgroup_memory_stat(&std::fs::read_to_string(&stat_path).ok()?); Some(CgroupMemorySnapshot { current_bytes: read_optional_u64(&root.join("memory.usage_in_bytes")), limit_bytes: read_optional_u64(&root.join("memory.limit_in_bytes")), - anon_bytes: stats.get("total_rss").copied().or_else(|| stats.get("rss").copied()), - file_bytes: stats.get("total_cache").copied().or_else(|| stats.get("cache").copied()), - active_file_bytes: stats - .get("total_active_file") - .copied() - .or_else(|| stats.get("active_file").copied()), - inactive_file_bytes: stats - .get("total_inactive_file") - .copied() - .or_else(|| stats.get("inactive_file").copied()), + anon_bytes: stats.total_rss.or(stats.rss), + file_bytes: stats.total_cache.or(stats.cache), + active_file_bytes: stats.total_active_file.or(stats.active_file), + inactive_file_bytes: stats.total_inactive_file.or(stats.inactive_file), }) } @@ -300,18 +323,17 @@ fn parse_mimalloc_stats_json(stats_json: &str) -> Option Option { // SAFETY: `mi_stats_get_json` returns a null-terminated JSON buffer owned by // mimalloc when called with a null input buffer. The mimalloc API requires - // freeing that buffer with `mi_free`, which is done before returning. - let stats = unsafe { + // freeing that buffer with `mi_free`; parsing finishes before the buffer is freed. + let observation = unsafe { let stats_ptr = libmimalloc_sys::mi_stats_get_json(0, std::ptr::null_mut()); if stats_ptr.is_null() { return None; } - let stats = CStr::from_ptr(stats_ptr).to_str().ok().map(str::to_owned); + let observation = CStr::from_ptr(stats_ptr).to_str().ok().and_then(parse_mimalloc_stats_json); libmimalloc_sys::mi_free(stats_ptr.cast()); - stats? + observation? }; - let observation = parse_mimalloc_stats_json(&stats)?; Some(AllocatorMemorySnapshot { backend: crate::allocator_reclaim::allocator_backend(), observation, @@ -456,17 +478,18 @@ mod tests { CgroupMemorySnapshot, MEMORY_OBSERVABILITY_SERVICE_NAME, MemoryObservabilityCancellationSource, MemoryObservabilityController, MemoryObservabilityDesiredState, MemoryObservabilityServiceState, MemoryObservabilityShutdownHandle, MemoryObservabilityWorkerMutation, build_memory_observability_controller_snapshot, - build_memory_observability_status_snapshot, parse_kv_stats, parse_mimalloc_stats_json, read_optional_u64, + build_memory_observability_status_snapshot, parse_cgroup_memory_stat, parse_mimalloc_stats_json, read_optional_u64, }; use std::fs; use std::path::PathBuf; #[test] - fn parse_kv_stats_extracts_numeric_pairs() { - let parsed = parse_kv_stats("anon 12\nfile 34\nactive_file 56\n"); - assert_eq!(parsed.get("anon").copied(), Some(12)); - assert_eq!(parsed.get("file").copied(), Some(34)); - assert_eq!(parsed.get("active_file").copied(), Some(56)); + fn parse_cgroup_memory_stat_extracts_tracked_numeric_fields() { + let parsed = parse_cgroup_memory_stat("anon 12\nfile 34\nactive_file 56\nignored 78\nmalformed nope\n"); + assert_eq!(parsed.anon, Some(12)); + assert_eq!(parsed.file, Some(34)); + assert_eq!(parsed.active_file, Some(56)); + assert_eq!(parsed.inactive_file, None); } #[test]