Skip to content

Commit

Permalink
perf: reduce memory usage in profiling
Browse files Browse the repository at this point in the history 
Utilizes ThinStr in the ZendFrame to shrink memory.
  • Loading branch information
@morrisonlevi
morrisonlevi committed Dec 15, 2024
1 parent 0fff208 commit b067c30
Show file tree
Hide file tree
Showing 9 changed files with 178 additions and 284 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions profiling/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
2 changes: 1 addition & 1 deletion profiling/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
41 changes: 25 additions & 16 deletions profiling/src/profiling/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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;
Expand All @@ -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);

Expand Down Expand Up @@ -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";

Expand Down Expand Up @@ -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,
}],
Expand Down Expand Up @@ -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);
Expand All @@ -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,
}],
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
}],
Expand All @@ -1056,7 +1061,7 @@ impl Profiler {
pub(crate) fn collect_opcache_restart(
&self,
now: i64,
file: String,
file: ThinString,
line: u32,
reason: &'static str,
) {
Expand Down Expand Up @@ -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,
}],
Expand Down Expand Up @@ -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,
}],
Expand Down Expand Up @@ -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)),
});
}
}
Expand Down Expand Up @@ -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,
}]
}
Expand Down
69 changes: 43 additions & 26 deletions profiling/src/profiling/stack_walking.rs
View file
Original file line number Diff line number Diff line change
@@ -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
}

Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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> {
Expand All @@ -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) };
Expand All @@ -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()?;
Expand Down Expand Up @@ -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,
});
Expand All @@ -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,
});
Expand Down Expand Up @@ -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,
})
Expand All @@ -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;
Expand All @@ -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)
}
});
Expand All @@ -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),
}
Expand Down Expand Up @@ -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,
});
Expand All @@ -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,
Expand All @@ -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::*;
Expand Down
Loading

0 comments on commit b067c30

Please sign in to comment.