perf: reduce memory usage in profiling

Utilizes ThinStr in the ZendFrame to shrink memory.

Author: morrisonlevi

Cargo.lock

@@ -21,6 +21,7 @@ chrono = { version = "0.4" }
 crossbeam-channel = { version = "0.5", default-features = false, features = ["std"] }
 datadog-alloc = { git = "https://github.com/DataDog/libdatadog", tag = "v10.0.0" }
 datadog-profiling = { git = "https://github.com/DataDog/libdatadog", tag = "v10.0.0" }
+datadog-thin-str = { version = "1", path = "../thin-str", features = ["std"] }
 ddcommon = { git = "https://github.com/DataDog/libdatadog", tag = "v10.0.0" }
 env_logger = { version = "0.11", default-features = false }
 indexmap = { version = "2.2" }

profiling/Cargo.toml

@@ -6,8 +6,8 @@ mod logging;
 mod pcntl;
 pub mod profiling;
 mod sapi;
-mod thin_str;
 mod wall_time;
+mod well_known;
 
 #[cfg(php_run_time_cache)]
 mod string_set;

profiling/src/lib.rs

@@ -17,6 +17,7 @@ use crate::bindings::ddog_php_prof_get_active_fiber_test as ddog_php_prof_get_ac
 
 use crate::bindings::{datadog_php_profiling_get_profiling_context, zend_execute_data};
 use crate::config::SystemSettings;
+use crate::well_known::WellKnown;
 use crate::{CLOCKS, TAGS};
 use chrono::Utc;
 use core::{ptr, str};
@@ -26,6 +27,7 @@ use datadog_profiling::api::{
 };
 use datadog_profiling::exporter::Tag;
 use datadog_profiling::internal::Profile as InternalProfile;
+use datadog_thin_str::ThinString;
 use log::{debug, info, trace, warn};
 use once_cell::sync::OnceCell;
 use std::borrow::Cow;
@@ -49,6 +51,8 @@ use datadog_profiling::api::UpscalingInfo;
 
 #[cfg(feature = "exception_profiling")]
 use crate::exception::EXCEPTION_PROFILING_INTERVAL;
+#[cfg(feature = "timeline")]
+use crate::timeline::IncludeType;
 
 const UPLOAD_PERIOD: Duration = Duration::from_secs(67);
 
@@ -514,9 +518,6 @@ pub enum UploadMessage {
     Upload(Box<UploadRequest>),
 }
 
-#[cfg(feature = "timeline")]
-const COW_EVAL: Cow<str> = Cow::Borrowed("[eval]");
-
 const DDPROF_TIME: &str = "ddprof_time";
 const DDPROF_UPLOAD: &str = "ddprof_upload";
 
@@ -907,14 +908,14 @@ impl Profiler {
     }];
 
     #[cfg(feature = "timeline")]
-    pub fn collect_compile_string(&self, now: i64, duration: i64, filename: String, line: u32) {
+    pub fn collect_compile_string(&self, now: i64, duration: i64, filename: ThinString, line: u32) {
         let mut labels = Profiler::common_labels(Self::TIMELINE_COMPILE_FILE_LABELS.len());
         labels.extend_from_slice(Self::TIMELINE_COMPILE_FILE_LABELS);
         let n_labels = labels.len();
 
         match self.prepare_and_send_message(
             vec![ZendFrame {
-                function: COW_EVAL,
+                function: WellKnown::Eval.into(),
                 file: Some(filename),
                 line,
             }],
@@ -942,7 +943,7 @@ impl Profiler {
         now: i64,
         duration: i64,
         filename: String,
-        include_type: &str,
+        include_type: IncludeType,
     ) {
         let mut labels = Profiler::common_labels(Self::TIMELINE_COMPILE_FILE_LABELS.len() + 1);
         labels.extend_from_slice(Self::TIMELINE_COMPILE_FILE_LABELS);
@@ -955,7 +956,11 @@ impl Profiler {
 
         match self.prepare_and_send_message(
             vec![ZendFrame {
-                function: format!("[{include_type}]").into(),
+                function: match include_type {
+                    IncludeType::Include => WellKnown::Include.into(),
+                    IncludeType::Require => WellKnown::Require.into(),
+                    IncludeType::Unknown => WellKnown::UnknownIncludeType.into(),
+                },
                 file: None,
                 line: 0,
             }],
@@ -984,7 +989,7 @@ impl Profiler {
 
         labels.push(Label {
             key: "event",
-            value: LabelValue::Str(std::borrow::Cow::Borrowed(event)),
+            value: LabelValue::Str(Cow::Borrowed(event)),
         });
 
         let n_labels = labels.len();
@@ -1013,7 +1018,7 @@ impl Profiler {
 
     /// This function can be called to collect any fatal errors
     #[cfg(feature = "timeline")]
-    pub fn collect_fatal(&self, now: i64, file: String, line: u32, message: String) {
+    pub fn collect_fatal(&self, now: i64, file: ThinString, line: u32, message: String) {
         let mut labels = Profiler::common_labels(2);
 
         labels.push(Label {
@@ -1029,7 +1034,7 @@ impl Profiler {
 
         match self.prepare_and_send_message(
             vec![ZendFrame {
-                function: "[fatal]".into(),
+                function: WellKnown::Fatal.into(),
                 file: Some(file),
                 line,
             }],
@@ -1056,7 +1061,7 @@ impl Profiler {
     pub(crate) fn collect_opcache_restart(
         &self,
         now: i64,
-        file: String,
+        file: ThinString,
         line: u32,
         reason: &'static str,
     ) {
@@ -1109,7 +1114,7 @@ impl Profiler {
 
         match self.prepare_and_send_message(
             vec![ZendFrame {
-                function: "[idle]".into(),
+                function: WellKnown::Idle.into(),
                 file: None,
                 line: 0,
             }],
@@ -1165,7 +1170,7 @@ impl Profiler {
 
         match self.prepare_and_send_message(
             vec![ZendFrame {
-                function: "[gc]".into(),
+                function: WellKnown::Gc.into(),
                 file: None,
                 line: 0,
             }],
@@ -1233,7 +1238,7 @@ impl Profiler {
             if let Some(functionname) = extract_function_name(func) {
                 labels.push(Label {
                     key: "fiber",
-                    value: LabelValue::Str(functionname.into()),
+                    value: LabelValue::Str(Cow::from(functionname)),
                 });
             }
         }
@@ -1287,12 +1292,16 @@ mod tests {
     use super::*;
     use crate::config::AgentEndpoint;
     use datadog_profiling::exporter::Uri;
+    use datadog_thin_str::ConstStorage;
     use log::LevelFilter;
 
     fn get_frames() -> Vec<ZendFrame> {
+        static FOOBAR: ConstStorage<8> = ConstStorage::from_str("foobar()");
+        static FOOBAR_PHP: ConstStorage<10> = ConstStorage::from_str("foobar.php");
+
         vec![ZendFrame {
-            function: "foobar()".into(),
-            file: Some("foobar.php".into()),
+            function: ThinString::from(&FOOBAR),
+            file: Some(ThinString::from(&FOOBAR_PHP)),
             line: 42,
         }]
     }

profiling/src/profiling/mod.rs

@@ -1,23 +1,24 @@
 use crate::bindings::{zai_str_from_zstr, zend_execute_data, zend_function};
-use std::borrow::Cow;
+use crate::well_known::WellKnown;
+use datadog_alloc::Global;
+use datadog_thin_str::ThinString;
+use std::ops::Deref;
 use std::str::Utf8Error;
 
 #[cfg(php_frameless)]
 use crate::bindings::zend_flf_functions;
+
 #[cfg(php_frameless)]
 use crate::bindings::{
     ZEND_FRAMELESS_ICALL_0, ZEND_FRAMELESS_ICALL_1, ZEND_FRAMELESS_ICALL_2, ZEND_FRAMELESS_ICALL_3,
 };
 
-const COW_PHP_OPEN_TAG: Cow<str> = Cow::Borrowed("<?php");
-const COW_TRUNCATED: Cow<str> = Cow::Borrowed("[truncated]");
-
 #[derive(Default, Debug)]
 pub struct ZendFrame {
     // Most tools don't like frames that don't have function names, so use a
     // fake name if you need to like "<?php".
-    pub function: Cow<'static, str>,
-    pub file: Option<String>,
+    pub function: ThinString,
+    pub file: Option<ThinString>,
     pub line: u32, // use 0 for no line info
 }
 
@@ -30,7 +31,7 @@ pub struct ZendFrame {
 /// Namespaces are part of the class_name or function_name respectively.
 /// Closures and anonymous classes get reformatted by the backend (or maybe
 /// frontend, either way it's not our concern, at least not right now).
-pub fn extract_function_name(func: &zend_function) -> Option<String> {
+pub fn extract_function_name(func: &zend_function) -> Option<ThinString> {
     let method_name: &[u8] = func.name().unwrap_or(b"");
 
     /* The top of the stack seems to reasonably often not have a function, but
@@ -59,15 +60,17 @@ pub fn extract_function_name(func: &zend_function) -> Option<String> {
 
     buffer.extend_from_slice(method_name);
 
-    Some(String::from_utf8_lossy(buffer.as_slice()).into_owned())
+    let lossy = String::from_utf8_lossy(buffer.as_slice());
+    Some(ThinString::from_str_in(&lossy, Global))
 }
 
-unsafe fn extract_file_and_line(execute_data: &zend_execute_data) -> (Option<String>, u32) {
+unsafe fn extract_file_and_line(execute_data: &zend_execute_data) -> (Option<ThinString>, u32) {
     // This should be Some, just being cautious.
     match execute_data.func.as_ref() {
         Some(func) if !func.is_internal() => {
             // Safety: zai_str_from_zstr will return a valid ZaiStr.
-            let file = zai_str_from_zstr(func.op_array.filename.as_mut()).into_string();
+            let file_lossy = zai_str_from_zstr(func.op_array.filename.as_mut()).into_string_lossy();
+            let file = ThinString::from_str_in(file_lossy.deref(), Global);
             let lineno = match execute_data.opline.as_ref() {
                 Some(opline) => opline.lineno,
                 None => 0,
@@ -82,9 +85,10 @@ unsafe fn extract_file_and_line(execute_data: &zend_execute_data) -> (Option<Str
 mod detail {
     use super::*;
     use crate::string_set::StringSet;
-    use crate::thin_str::ThinStr;
+    use datadog_thin_str::ThinStr;
     use log::{debug, trace};
     use std::cell::RefCell;
+    use std::ops::Deref;
     use std::ptr::NonNull;
 
     struct StringCache<'a> {
@@ -100,9 +104,9 @@ mod detail {
         /// string in the slot currently, then create one by calling the
         /// provided function, store it in the string cache and cache slot,
         /// and return it.
-        fn get_or_insert<F>(&mut self, slot: usize, f: F) -> Option<String>
+        fn get_or_insert<F>(&mut self, slot: usize, f: F) -> Option<ThinString>
         where
-            F: FnOnce() -> Option<String>,
+            F: FnOnce() -> Option<ThinString>,
         {
             debug_assert!(slot < self.cache_slots.len());
             let cached = unsafe { self.cache_slots.get_unchecked_mut(slot) };
@@ -116,7 +120,7 @@ mod detail {
                     // so this ThinStr points into the same string set that
                     // created it.
                     let str = unsafe { self.string_set.get_thin_str(thin_str) };
-                    Some(str.to_string())
+                    Some(ThinString::from_str_in(str, Global))
                 }
                 None => {
                     let string = f()?;
@@ -221,7 +225,7 @@ mod detail {
                                     &**zend_flf_functions.offset(opline.extended_value as isize)
                                 };
                                 samples.push(ZendFrame {
-                                    function: extract_function_name(func).map(Cow::Owned).unwrap(),
+                                    function: extract_function_name(func).unwrap(),
                                     file: None,
                                     line: 0,
                                 });
@@ -240,7 +244,7 @@ mod detail {
                          */
                         if samples.len() == max_depth - 1 {
                             samples.push(ZendFrame {
-                                function: COW_TRUNCATED,
+                                function: ThinString::from(WellKnown::Truncated),
                                 file: None,
                                 line: 0,
                             });
@@ -292,15 +296,15 @@ mod detail {
                     let mut stats = cell.borrow_mut();
                     stats.not_applicable += 1;
                 });
-                let function = extract_function_name(func).map(Cow::Owned);
+                let function = extract_function_name(func);
                 let (file, line) = extract_file_and_line(execute_data);
                 (function, file, line)
             }
         };
 
         if function.is_some() || file.is_some() {
             Some(ZendFrame {
-                function: function.unwrap_or(COW_PHP_OPEN_TAG),
+                function: function.unwrap_or(WellKnown::PhpOpenTag.into()),
                 file,
                 line,
             })
@@ -312,16 +316,16 @@ mod detail {
     fn handle_function_cache_slot(
         func: &zend_function,
         string_cache: &mut StringCache,
-    ) -> Option<Cow<'static, str>> {
+    ) -> Option<ThinString> {
         let fname = string_cache.get_or_insert(0, || extract_function_name(func))?;
-        Some(Cow::Owned(fname))
+        Some(ThinString::from_str_in(&fname, Global))
     }
 
     unsafe fn handle_file_cache_slot(
         execute_data: &zend_execute_data,
         string_cache: &mut StringCache,
-    ) -> (Option<String>, u32) {
-        let option = string_cache.get_or_insert(1, || -> Option<String> {
+    ) -> (Option<ThinString>, u32) {
+        let option = string_cache.get_or_insert(1, || -> Option<ThinString> {
             unsafe {
                 // Safety: if we have cache slots, we definitely have a func.
                 let func = &*execute_data.func;
@@ -330,7 +334,9 @@ mod detail {
                 };
 
                 // SAFETY: calling C function with correct args.
-                let file = zai_str_from_zstr(func.op_array.filename.as_mut()).into_string();
+                let file_lossy =
+                    zai_str_from_zstr(func.op_array.filename.as_mut()).into_string_lossy();
+                let file = ThinString::from_str_in(file_lossy.deref(), Global);
                 Some(file)
             }
         });
@@ -341,7 +347,7 @@ mod detail {
                     Some(opline) => opline.lineno,
                     None => 0,
                 };
-                (Some(filename), lineno)
+                (Some(ThinString::from_str_in(&filename, Global)), lineno)
             }
             None => (None, 0),
         }
@@ -380,7 +386,7 @@ mod detail {
                  */
                 if samples.len() == max_depth - 1 {
                     samples.push(ZendFrame {
-                        function: COW_TRUNCATED,
+                        function: WellKnown::Truncated.into(),
                         file: None,
                         line: 0,
                     });
@@ -401,7 +407,7 @@ mod detail {
             // Only create a new frame if there's file or function info.
             if file.is_some() || function.is_some() {
                 // If there's no function name, use a fake name.
-                let function = function.map(Cow::Owned).unwrap_or(COW_PHP_OPEN_TAG);
+                let function = function.unwrap_or(WellKnown::PhpOpenTag.into());
                 return Some(ZendFrame {
                     function,
                     file,
@@ -415,6 +421,17 @@ mod detail {
 
 pub use detail::*;
 
+#[cfg(test)]
+mod size_tests {
+    use super::*;
+    use core::mem::size_of;
+
+    #[test]
+    fn test_frame_size() {
+        assert_eq!(size_of::<ZendFrame>(), size_of::<usize>() * 3);
+    }
+}
+
 #[cfg(all(test, stack_walking_tests))]
 mod tests {
     use super::*;