From b393823f368811f6711820c37e499d26bd042a23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Renault?= Date: Tue, 6 Feb 2024 14:41:14 +0100 Subject: [PATCH] Replace stats_alloc with procfs --- Cargo.lock | 33 ++++-- meilisearch/Cargo.toml | 1 - meilisearch/src/main.rs | 5 - meilisearch/src/routes/logs.rs | 2 +- tracing-trace/Cargo.toml | 3 +- tracing-trace/src/entry.rs | 74 +++++------- tracing-trace/src/layer.rs | 45 ++------ .../src/processor/firefox_profiler.rs | 106 ++++-------------- tracing-trace/src/processor/fmt.rs | 22 +--- 9 files changed, 99 insertions(+), 192 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4ad454e80..8f8fd1ff1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3667,7 +3667,6 @@ dependencies = [ "siphasher 1.0.0", "slice-group-by", "static-files", - "stats_alloc", "sysinfo", "tar", "temp-env", @@ -4433,6 +4432,29 @@ dependencies = [ "rustix 0.36.16", ] +[[package]] +name = "procfs" +version = "0.16.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "731e0d9356b0c25f16f33b5be79b1c57b562f141ebfcdb0ad8ac2c13a24293b4" +dependencies = [ + "bitflags 2.4.1", + "hex", + "lazy_static", + "procfs-core", + "rustix 0.38.26", +] + +[[package]] +name = "procfs-core" +version = "0.16.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2d3554923a69f4ce04c4a754260c338f505ce22642d3830e049a399fc2059a29" +dependencies = [ + "bitflags 2.4.1", + "hex", +] + [[package]] name = "prometheus" version = "0.13.3" @@ -4445,7 +4467,7 @@ dependencies = [ "libc", "memchr", "parking_lot", - "procfs", + "procfs 0.14.2", "protobuf", "thiserror", ] @@ -5222,11 +5244,6 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" -[[package]] -name = "stats_alloc" -version = "0.1.10" -source = "git+https://github.com/Kerollmops/stats_alloc?branch=stable-const-fn-trait#6f83c52160c7d0550fdf770e1f73d239b0ff9a97" - [[package]] name = "strsim" version = "0.10.0" @@ -5705,9 +5722,9 @@ dependencies = [ "byte-unit", "color-spantrace", "fxprof-processed-profile", + "procfs 0.16.0", "serde", "serde_json", - "stats_alloc", "tokio", "tracing", "tracing-error", diff --git a/meilisearch/Cargo.toml b/meilisearch/Cargo.toml index 21136b6e8..2a7b5ade1 100644 --- a/meilisearch/Cargo.toml +++ b/meilisearch/Cargo.toml @@ -107,7 +107,6 @@ url = { version = "2.5.0", features = ["serde"] } tracing = "0.1.40" tracing-subscriber = "0.3.18" tracing-trace = { version = "0.1.0", path = "../tracing-trace" } -stats_alloc = { git = "https://github.com/Kerollmops/stats_alloc", branch = "stable-const-fn-trait", optional = true } [dev-dependencies] actix-rt = "2.9.0" diff --git a/meilisearch/src/main.rs b/meilisearch/src/main.rs index 94a28eb32..734f50de3 100644 --- a/meilisearch/src/main.rs +++ b/meilisearch/src/main.rs @@ -20,14 +20,9 @@ use tracing::level_filters::LevelFilter; use tracing_subscriber::layer::SubscriberExt as _; use tracing_subscriber::Layer; -#[cfg(not(feature = "stats_alloc"))] #[global_allocator] static ALLOC: MiMalloc = MiMalloc; -#[cfg(feature = "stats_alloc")] -#[global_allocator] -static ALLOC: stats_alloc::StatsAlloc = stats_alloc::StatsAlloc::new(MiMalloc); - fn default_layer() -> LogRouteType { None.with_filter(tracing_subscriber::filter::Targets::new().with_target("", LevelFilter::OFF)) } diff --git a/meilisearch/src/routes/logs.rs b/meilisearch/src/routes/logs.rs index c22ca2129..a62f6d648 100644 --- a/meilisearch/src/routes/logs.rs +++ b/meilisearch/src/routes/logs.rs @@ -14,7 +14,7 @@ use index_scheduler::IndexScheduler; use meilisearch_types::deserr::DeserrJsonError; use meilisearch_types::error::deserr_codes::*; use meilisearch_types::error::{Code, ResponseError}; -use tokio::sync::mpsc::{self}; +use tokio::sync::mpsc; use tracing_subscriber::filter::Targets; use tracing_subscriber::Layer; diff --git a/tracing-trace/Cargo.toml b/tracing-trace/Cargo.toml index da5e2b36c..4fe3ca735 100644 --- a/tracing-trace/Cargo.toml +++ b/tracing-trace/Cargo.toml @@ -13,10 +13,11 @@ serde_json = "1.0.111" tracing = "0.1.40" tracing-error = "0.2.0" tracing-subscriber = "0.3.18" -stats_alloc = { git = "https://github.com/Kerollmops/stats_alloc", branch = "stable-const-fn-trait" } byte-unit = { version = "4.0.19", default-features = false, features = [ "std", "serde", ] } tokio = { version = "1.35.1", features = ["sync"] } +[target.'cfg(target_os = "linux")'.dependencies] +procfs = { version = "0.16.0", default-features = false } diff --git a/tracing-trace/src/entry.rs b/tracing-trace/src/entry.rs index 61151b04c..f0136c18c 100644 --- a/tracing-trace/src/entry.rs +++ b/tracing-trace/src/entry.rs @@ -101,58 +101,46 @@ pub struct SpanClose { } /// A struct with a lot of memory allocation stats akin -/// to the `stats_alloc::Stats` one but implements the -/// `Serialize/Deserialize` serde traits. +/// to the `procfs::Process::StatsM` one plus the OOM score. +/// +/// Note that all the values are in bytes not in pages. #[derive(Debug, Default, Clone, Copy, Serialize, Deserialize)] pub struct MemoryStats { - pub allocations: usize, - pub deallocations: usize, - pub reallocations: usize, - pub bytes_allocated: usize, - pub bytes_deallocated: usize, - pub bytes_reallocated: isize, -} - -impl From for MemoryStats { - fn from(stats: stats_alloc::Stats) -> Self { - let stats_alloc::Stats { - allocations, - deallocations, - reallocations, - bytes_allocated, - bytes_deallocated, - bytes_reallocated, - } = stats; - MemoryStats { - allocations, - deallocations, - reallocations, - bytes_allocated, - bytes_deallocated, - bytes_reallocated, - } - } + /// Resident set size, measured in bytes. + /// (same as VmRSS in /proc//status). + pub resident: u64, + /// Number of resident shared bytes (i.e., backed by a file). + /// (same as RssFile+RssShmem in /proc//status). + pub shared: u64, + /// The current score that the kernel gives to this process + /// for the purpose of selecting a process for the OOM-killer + /// + /// A higher score means that the process is more likely to be selected + /// by the OOM-killer. The basis for this score is the amount of memory used + /// by the process, plus other factors. + /// + /// (Since linux 2.6.11) + pub oom_score: u32, } impl MemoryStats { + #[cfg(target_os = "linux")] + pub fn fetch() -> procfs::ProcResult { + let process = procfs::process::Process::myself().unwrap(); + let procfs::process::StatM { resident, shared, .. } = process.statm()?; + let oom_score = process.oom_score()?; + let page_size = procfs::page_size(); + + Ok(MemoryStats { resident: resident * page_size, shared: shared * page_size, oom_score }) + } + pub fn checked_sub(self, other: Self) -> Option { Some(Self { - allocations: self.allocations.checked_sub(other.allocations)?, - deallocations: self.deallocations.checked_sub(other.deallocations)?, - reallocations: self.reallocations.checked_sub(other.reallocations)?, - bytes_allocated: self.bytes_allocated.checked_sub(other.bytes_allocated)?, - bytes_deallocated: self.bytes_deallocated.checked_sub(other.bytes_deallocated)?, - bytes_reallocated: self.bytes_reallocated.checked_sub(other.bytes_reallocated)?, + resident: self.resident.checked_sub(other.resident)?, + shared: self.shared.checked_sub(other.shared)?, + oom_score: self.oom_score.checked_sub(other.oom_score)?, }) } - - pub fn usage(&self) -> isize { - (self.bytes_allocated - self.bytes_deallocated) as isize + self.bytes_reallocated - } - - pub fn operations(&self) -> usize { - self.allocations + self.deallocations + self.reallocations - } } #[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Hash)] diff --git a/tracing-trace/src/layer.rs b/tracing-trace/src/layer.rs index aa2908304..96690ff1f 100644 --- a/tracing-trace/src/layer.rs +++ b/tracing-trace/src/layer.rs @@ -1,11 +1,9 @@ -use std::alloc::{GlobalAlloc, System}; use std::borrow::Cow; use std::collections::HashMap; use std::io::Write; use std::ops::ControlFlow; use std::sync::RwLock; -use stats_alloc::StatsAlloc; use tracing::span::{Attributes, Id as TracingId}; use tracing::{Metadata, Subscriber}; use tracing_subscriber::layer::Context; @@ -18,55 +16,31 @@ use crate::entry::{ use crate::{Error, Trace, TraceWriter}; /// Layer that measures the time spent in spans. -pub struct TraceLayer { +pub struct TraceLayer { sender: tokio::sync::mpsc::UnboundedSender, callsites: RwLock>, start_time: std::time::Instant, - memory_allocator: Option<&'static StatsAlloc>, } impl Trace { - pub fn new() -> (Self, TraceLayer) { + pub fn new() -> (Self, TraceLayer) { let (sender, receiver) = tokio::sync::mpsc::unbounded_channel(); let trace = Trace { receiver }; let layer = TraceLayer { sender, callsites: Default::default(), start_time: std::time::Instant::now(), - memory_allocator: None, - }; - (trace, layer) - } - - pub fn with_stats_alloc( - stats_alloc: &'static StatsAlloc, - ) -> (Self, TraceLayer) { - let (sender, receiver) = tokio::sync::mpsc::unbounded_channel(); - let trace = Trace { receiver }; - let layer = TraceLayer { - sender, - callsites: Default::default(), - start_time: std::time::Instant::now(), - memory_allocator: Some(stats_alloc), }; (trace, layer) } } impl TraceWriter { - pub fn new(writer: W) -> (Self, TraceLayer) { + pub fn new(writer: W) -> (Self, TraceLayer) { let (trace, layer) = Trace::new(); (trace.into_writer(writer), layer) } - pub fn with_stats_alloc( - writer: W, - stats_alloc: &'static StatsAlloc, - ) -> (Self, TraceLayer) { - let (trace, layer) = Trace::with_stats_alloc(stats_alloc); - (trace.into_writer(writer), layer) - } - pub async fn receive(&mut self) -> Result, Error> { let Some(entry) = self.receiver.recv().await else { return Ok(ControlFlow::Break(())); @@ -107,7 +81,7 @@ enum OpaqueIdentifier { Call(tracing::callsite::Identifier), } -impl TraceLayer { +impl TraceLayer { fn resource_id(&self, opaque: OpaqueIdentifier) -> Option { self.callsites.read().unwrap().get(&opaque).copied() } @@ -122,8 +96,14 @@ impl TraceLayer { self.start_time.elapsed() } + #[cfg(target_os = "linux")] fn memory_stats(&self) -> Option { - self.memory_allocator.map(|ma| ma.stats().into()) + Some(MemoryStats::fetch().unwrap()) + } + + #[cfg(not(target_os = "linux"))] + fn memory_stats(&self) -> Option { + None } fn send(&self, entry: Entry) { @@ -160,10 +140,9 @@ impl TraceLayer { } } -impl Layer for TraceLayer +impl Layer for TraceLayer where S: Subscriber, - A: GlobalAlloc, { fn on_new_span(&self, attrs: &Attributes<'_>, id: &TracingId, _ctx: Context<'_, S>) { let call_id = self diff --git a/tracing-trace/src/processor/firefox_profiler.rs b/tracing-trace/src/processor/firefox_profiler.rs index 126b4af1a..5daf202bd 100644 --- a/tracing-trace/src/processor/firefox_profiler.rs +++ b/tracing-trace/src/processor/firefox_profiler.rs @@ -227,8 +227,8 @@ fn add_memory_samples( profile.add_counter_sample( memory_counters.usage, last_timestamp, - stats.usage() as f64 - last_memory.usage() as f64, - stats.operations().checked_sub(last_memory.operations()).unwrap_or_default() as u32, + stats.resident as f64 - last_memory.resident as f64, + 0, ); let delta = stats.checked_sub(*last_memory); @@ -317,39 +317,21 @@ impl<'a> ProfilerMarker for SpanMarker<'a> { searchable: true, }), MarkerSchemaField::Dynamic(MarkerDynamicField { - key: "allocations", - label: "Number of allocation operations while this function was executing", - format: MarkerFieldFormat::Integer, - searchable: false, - }), - MarkerSchemaField::Dynamic(MarkerDynamicField { - key: "deallocations", - label: "Number of deallocation operations while this function was executing", - format: MarkerFieldFormat::Integer, - searchable: false, - }), - MarkerSchemaField::Dynamic(MarkerDynamicField { - key: "reallocations", - label: "Number of reallocation operations while this function was executing", - format: MarkerFieldFormat::Integer, - searchable: false, - }), - MarkerSchemaField::Dynamic(MarkerDynamicField { - key: "allocated_bytes", - label: "Number of allocated bytes while this function was executing", + key: "resident", + label: "Resident set size, measured in bytes while this function was executing", format: MarkerFieldFormat::Bytes, searchable: false, }), MarkerSchemaField::Dynamic(MarkerDynamicField { - key: "deallocated_bytes", - label: "Number of deallocated bytes while this function was executing", + key: "shared", + label: "Number of resident shared pages (i.e., backed by a file) while this function was executing", format: MarkerFieldFormat::Bytes, searchable: false, }), MarkerSchemaField::Dynamic(MarkerDynamicField { - key: "reallocated_bytes", - label: "Number of reallocated bytes while this function was executing", - format: MarkerFieldFormat::Bytes, + key: "oom_score", + label: "The current score that the kernel gives to this process for the purpose of selecting a process for the OOM-killer while this function was executing", + format: MarkerFieldFormat::Integer, searchable: false, }), ]; @@ -384,21 +366,10 @@ impl<'a> ProfilerMarker for SpanMarker<'a> { "thread_id": thread_id, }); - if let Some(MemoryStats { - allocations, - deallocations, - reallocations, - bytes_allocated, - bytes_deallocated, - bytes_reallocated, - }) = self.memory_delta - { - value["allocations"] = json!(allocations); - value["deallocations"] = json!(deallocations); - value["reallocations"] = json!(reallocations); - value["allocated_bytes"] = json!(bytes_allocated); - value["deallocated_bytes"] = json!(bytes_deallocated); - value["reallocated_bytes"] = json!(bytes_reallocated); + if let Some(MemoryStats { resident, shared, oom_score }) = self.memory_delta { + value["resident"] = json!(resident); + value["shared"] = json!(shared); + value["oom_score"] = json!(oom_score); } value @@ -447,39 +418,21 @@ impl<'a> ProfilerMarker for EventMarker<'a> { searchable: true, }), MarkerSchemaField::Dynamic(MarkerDynamicField { - key: "allocations", - label: "Number of allocation operations since last measure", - format: MarkerFieldFormat::Integer, - searchable: false, - }), - MarkerSchemaField::Dynamic(MarkerDynamicField { - key: "deallocations", - label: "Number of deallocation operations since last measure", - format: MarkerFieldFormat::Integer, - searchable: false, - }), - MarkerSchemaField::Dynamic(MarkerDynamicField { - key: "reallocations", - label: "Number of reallocation operations since last measure", - format: MarkerFieldFormat::Integer, - searchable: false, - }), - MarkerSchemaField::Dynamic(MarkerDynamicField { - key: "allocated_bytes", - label: "Number of allocated bytes since last measure", + key: "resident", + label: "Resident set size, measured in bytes while this function was executing", format: MarkerFieldFormat::Bytes, searchable: false, }), MarkerSchemaField::Dynamic(MarkerDynamicField { - key: "deallocated_bytes", - label: "Number of deallocated bytes since last measure", + key: "shared", + label: "Number of resident shared pages (i.e., backed by a file) while this function was executing", format: MarkerFieldFormat::Bytes, searchable: false, }), MarkerSchemaField::Dynamic(MarkerDynamicField { - key: "reallocated_bytes", - label: "Number of reallocated bytes since last measure", - format: MarkerFieldFormat::Bytes, + key: "oom_score", + label: "The current score that the kernel gives to this process for the purpose of selecting a process for the OOM-killer while this function was executing", + format: MarkerFieldFormat::Integer, searchable: false, }), ]; @@ -514,21 +467,10 @@ impl<'a> ProfilerMarker for EventMarker<'a> { "thread_id": thread_id, }); - if let Some(MemoryStats { - allocations, - deallocations, - reallocations, - bytes_allocated, - bytes_deallocated, - bytes_reallocated, - }) = self.memory_delta - { - value["allocations"] = json!(allocations); - value["deallocations"] = json!(deallocations); - value["reallocations"] = json!(reallocations); - value["allocated_bytes"] = json!(bytes_allocated); - value["deallocated_bytes"] = json!(bytes_deallocated); - value["reallocated_bytes"] = json!(bytes_reallocated); + if let Some(MemoryStats { resident, shared, oom_score }) = self.memory_delta { + value["resident"] = json!(resident); + value["shared"] = json!(shared); + value["oom_score"] = json!(oom_score); } value diff --git a/tracing-trace/src/processor/fmt.rs b/tracing-trace/src/processor/fmt.rs index 166930dfc..8c6af1640 100644 --- a/tracing-trace/src/processor/fmt.rs +++ b/tracing-trace/src/processor/fmt.rs @@ -188,23 +188,9 @@ fn print_duration(duration: std::time::Duration) -> String { } /// Format only the allocated bytes, deallocated bytes and reallocated bytes in GiB, MiB, KiB, Bytes. -fn print_memory(memory: MemoryStats) -> String { +fn print_memory(MemoryStats { resident, shared, oom_score }: MemoryStats) -> String { use byte_unit::Byte; - - let allocated_bytes = Byte::from_bytes(memory.bytes_allocated.try_into().unwrap()); - let deallocated_bytes = Byte::from_bytes(memory.bytes_deallocated.try_into().unwrap()); - - let reallocated_sign = if memory.bytes_reallocated < 0 { "-" } else { "" }; - let reallocated_bytes = - Byte::from_bytes(memory.bytes_reallocated.abs_diff(0).try_into().unwrap()); - - let adjusted_allocated_bytes = allocated_bytes.get_appropriate_unit(true); - let adjusted_deallocated_bytes = deallocated_bytes.get_appropriate_unit(true); - let adjusted_reallocated_bytes = reallocated_bytes.get_appropriate_unit(true); - - format!( - "Allocated {adjusted_allocated_bytes:.2}, \ - Deallocated {adjusted_deallocated_bytes:.2}, \ - Reallocated {reallocated_sign}{adjusted_reallocated_bytes:.2}" - ) + let rss_bytes = Byte::from_bytes(resident).get_appropriate_unit(true); + let shared_bytes = Byte::from_bytes(shared).get_appropriate_unit(true); + format!("RSS {rss_bytes:.2}, Shared {shared_bytes:.2}, OOM score {oom_score}") }