From cdcba7c45735a6d94d5a0e7605fd59325744aeb7 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Tue, 28 Feb 2023 21:54:33 -0700 Subject: [PATCH 01/13] perf(profiling): speed up stalk walking by using function run_time_cache --- profiling-store/Cargo.toml | 17 ++ profiling-store/rust-toolchain.toml | 2 + profiling-store/src/lib.rs | 3 + profiling-store/src/string_table/borrowed.rs | 87 ++++++++++ .../src/string_table/bump_owned.rs | 70 ++++++++ profiling-store/src/string_table/mod.rs | 74 +++++++++ profiling-store/src/string_table/owned.rs | 109 ++++++++++++ profiling/Cargo.lock | 32 ++++ profiling/Cargo.toml | 5 +- profiling/src/bindings/mod.rs | 28 ++++ profiling/src/lib.rs | 25 ++- profiling/src/php_ffi.c | 57 ++++++- profiling/src/php_ffi.h | 7 +- profiling/src/profiling/mod.rs | 8 +- profiling/src/profiling/stalk_walking.rs | 156 +++++++++++++++--- 15 files changed, 639 insertions(+), 41 deletions(-) create mode 100644 profiling-store/Cargo.toml create mode 100644 profiling-store/rust-toolchain.toml create mode 100644 profiling-store/src/lib.rs create mode 100644 profiling-store/src/string_table/borrowed.rs create mode 100644 profiling-store/src/string_table/bump_owned.rs create mode 100644 profiling-store/src/string_table/mod.rs create mode 100644 profiling-store/src/string_table/owned.rs diff --git a/profiling-store/Cargo.toml b/profiling-store/Cargo.toml new file mode 100644 index 00000000000..9784a1d1252 --- /dev/null +++ b/profiling-store/Cargo.toml @@ -0,0 +1,17 @@ +# Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. +# This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present Datadog, Inc. + +[package] +name = "datadog-profiling-store" +version = "2.0.0" +edition = "2021" +license = "Apache-2.0" + +[dependencies] +ahash = "0.8.3" +anyhow = "1.0.68" +bumpalo = { version = "3.12.0", features = ["collections"] } +crossbeam-channel = "0.5.6" +derivative = "2.2.0" +prost = "0.11.6" +self_cell = "0.10.2" diff --git a/profiling-store/rust-toolchain.toml b/profiling-store/rust-toolchain.toml new file mode 100644 index 00000000000..292fe499e3b --- /dev/null +++ b/profiling-store/rust-toolchain.toml @@ -0,0 +1,2 @@ +[toolchain] +channel = "stable" diff --git a/profiling-store/src/lib.rs b/profiling-store/src/lib.rs new file mode 100644 index 00000000000..0e99c839b18 --- /dev/null +++ b/profiling-store/src/lib.rs @@ -0,0 +1,3 @@ +mod string_table; + +pub use string_table::*; diff --git a/profiling-store/src/string_table/borrowed.rs b/profiling-store/src/string_table/borrowed.rs new file mode 100644 index 00000000000..ab20ad727b1 --- /dev/null +++ b/profiling-store/src/string_table/borrowed.rs @@ -0,0 +1,87 @@ +use ahash::RandomState; +use std::collections::HashMap; +use std::ops::Range; + +pub struct BorrowedStringTable<'a> { + pub(super) vec: Vec<&'a str>, + pub(super) map: HashMap<&'a str, usize, RandomState>, +} + +impl<'a> BorrowedStringTable<'a> { + #[inline] + pub fn new() -> Self { + Self::default() + } +} + +impl<'a> Default for BorrowedStringTable<'a> { + fn default() -> Self { + /// The initial size of the Vec. At the time of writing, Vec would + /// choose size 4. This is expected to be much too small for the + /// use-case, so use a larger initial capacity to save a few + /// re-allocations in the beginning. + /// This is just an educated estimate, not a finely tuned value. + const INITIAL_VEC_CAPACITY: usize = 1024 / std::mem::size_of::<&str>(); + + /// A HashMap is less straight-forward, but it uses more memory for + /// the same number of elements compared to a Vec, but not twice as + /// much for our situation, so dividing by 2 should be okay, at least + /// until further measurement is done. + const INITIAL_MAP_CAPACITY: usize = INITIAL_VEC_CAPACITY / 2; + + let mut vec = Vec::with_capacity(INITIAL_VEC_CAPACITY); + vec.push(""); + let mut map = HashMap::with_capacity_and_hasher(INITIAL_MAP_CAPACITY, Default::default()); + map.insert("", 0); + Self { vec, map } + } +} + +impl<'a> super::StringTable for BorrowedStringTable<'a> { + #[inline] + fn len(&self) -> usize { + self.vec.len() + } + + #[inline] + fn is_empty(&self) -> bool { + self.vec.is_empty() + } + + fn insert_full(&mut self, str: &str) -> (usize, bool) { + match self.map.get(str) { + None => { + let id = self.vec.len(); + // Safety: DEFINITELY NOT SAFE. The caller _must_ + let borrowed = unsafe { std::mem::transmute(str) }; + self.vec.push(borrowed); + + self.map.insert(borrowed, id); + debug_assert_eq!(self.map.len(), self.vec.len()); + (id, true) + } + Some(offset) => (*offset, false), + } + } + + #[inline] + fn get_offset(&self, offset: usize) -> &str { + self.vec[offset] + } + + #[inline] + fn get_range(&self, range: Range) -> &[&str] { + &self.vec[range] + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + pub fn borrowed_string_table() { + let set = BorrowedStringTable::<'static>::new(); + super::super::tests::basic(set); + } +} diff --git a/profiling-store/src/string_table/bump_owned.rs b/profiling-store/src/string_table/bump_owned.rs new file mode 100644 index 00000000000..c49ea06ef26 --- /dev/null +++ b/profiling-store/src/string_table/bump_owned.rs @@ -0,0 +1,70 @@ +use bumpalo::{collections, Bump}; +use std::ops::Range; + +pub(super) struct BorrowedStringTable<'b> { + arena: &'b Bump, + pub(super) set: super::borrowed::BorrowedStringTable<'b>, +} + +impl<'b> BorrowedStringTable<'b> { + #[inline] + pub(super) fn new(arena: &'b Bump) -> Self { + Self { + arena, + set: Default::default(), + } + } +} + +impl<'b> super::StringTable for BorrowedStringTable<'b> { + #[inline] + fn len(&self) -> usize { + self.set.len() + } + + fn insert_full(&mut self, str: &str) -> (usize, bool) { + match self.set.map.get(str) { + None => { + let owned = collections::String::from_str_in(str, self.arena); + + /* Consume the string but retain a reference to its data in + * the arena. The reference is valid as long as the arena + * doesn't get reset. This is partly the reason for the unsafe + * marker on `StringTable::new`. + */ + let bumped_str = owned.into_bump_str(); + + let id = self.set.vec.len(); + self.set.vec.push(bumped_str); + + self.set.map.insert(bumped_str, id); + assert_eq!(self.set.vec.len(), self.set.map.len()); + (id, true) + } + Some(offset) => (*offset, false), + } + } + + #[inline] + fn get_offset(&self, offset: usize) -> &str { + self.set.get_offset(offset) + } + + #[inline] + fn get_range(&self, range: Range) -> &[&str] { + self.set.get_range(range) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + pub fn bump_owned_string_table() { + // small size, to allow testing re-alloc. + let bump = Bump::with_capacity(16); + let set = BorrowedStringTable::new(&bump); + super::super::tests::basic(set); + } +} diff --git a/profiling-store/src/string_table/mod.rs b/profiling-store/src/string_table/mod.rs new file mode 100644 index 00000000000..88641d4eea4 --- /dev/null +++ b/profiling-store/src/string_table/mod.rs @@ -0,0 +1,74 @@ +use std::ops::Range; + +mod borrowed; +mod bump_owned; +mod owned; + +pub use borrowed::*; +pub use owned::*; + +pub trait StringTable { + fn len(&self) -> usize; + + #[inline] + fn is_empty(&self) -> bool { + self.len() == 0 + } + + #[inline] + fn insert(&mut self, item: &str) -> usize { + self.insert_full(item).0 + } + + fn insert_full(&mut self, item: &str) -> (usize, bool); + + fn get_offset(&self, offset: usize) -> &str; + fn get_range(&self, range: Range) -> &[&str]; +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Pass in an empty set, which should only include the empty string at 0. + pub(crate) fn basic(mut set: S) { + // the empty string must always be included in the set at 0. + let empty_str = set.get_offset(0); + assert_eq!("", empty_str); + + let cases = &[ + (0, ""), + (1, "local root span id"), + (2, "span id"), + (3, "trace endpoint"), + (4, "samples"), + (5, "count"), + (6, "wall-time"), + (7, "nanoseconds"), + (8, "cpu-time"), + (9, " Self { + let bytes = self + .inner + .with_dependent(|arena, _table| arena.allocated_bytes()); + + use super::StringTable as StringTableTrait; + let mut table = OwnedStringTable::with_capacity(bytes); + let len = self.len(); + table.reserve(len); + for str in self.get_range(0..len) { + table.insert(str); + } + table + } +} + +impl OwnedStringTable { + #[inline] + pub fn new() -> Self { + Self::with_capacity(4000) + } + + #[inline] + pub fn with_capacity(capacity: usize) -> Self { + let inner = StringTableCell::new(Bump::with_capacity(capacity), |arena| { + BorrowedStringTable::new(arena) + }); + Self { inner } + } + + #[inline] + fn reserve(&mut self, additional: usize) { + self.inner.with_dependent_mut(|_arena, table| { + table.set.vec.reserve(additional); + table.set.map.reserve(additional); + }) + } +} + +impl Default for OwnedStringTable { + fn default() -> Self { + Self::new() + } +} + +impl super::StringTable for OwnedStringTable { + #[inline] + fn len(&self) -> usize { + self.inner.with_dependent(|_arena, set| set.len()) + } + + #[inline] + fn insert_full(&mut self, str: &str) -> (usize, bool) { + self.inner + .with_dependent_mut(|_arena, set| set.insert_full(str)) + } + + #[inline] + fn get_offset(&self, offset: usize) -> &str { + self.inner + .with_dependent(|_arena, set| set.get_offset(offset)) + } + + #[inline] + fn get_range(&self, range: Range) -> &[&str] { + self.inner + .with_dependent(|_arena, set| set.get_range(range)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// If this fails, bumpalo may have changed its allocation patterns, and + /// [OwnedStringTable::new] may need adjusted. + #[test] + fn test_bump() { + let arena = Bump::with_capacity(4000); + assert_eq!(4096 - 64, arena.chunk_capacity()); + } + + #[test] + fn owned_string_table() { + // small size, to allow testing re-alloc. + let set = OwnedStringTable::with_capacity(64); + super::super::tests::basic(set); + } +} diff --git a/profiling/Cargo.lock b/profiling/Cargo.lock index 82169d544a8..2ff46188fab 100644 --- a/profiling/Cargo.lock +++ b/profiling/Cargo.lock @@ -2,6 +2,18 @@ # It is not intended for manual editing. version = 3 +[[package]] +name = "ahash" +version = "0.8.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2c99f64d1e06488f620f932677e24bc6e2897582980441ae90a671415bd7ec2f" +dependencies = [ + "cfg-if", + "getrandom", + "once_cell", + "version_check", +] + [[package]] name = "aho-corasick" version = "0.7.20" @@ -284,6 +296,7 @@ dependencies = [ "cpu-time", "crossbeam-channel", "datadog-profiling", + "datadog-profiling-store", "env_logger", "indexmap", "lazy_static", @@ -326,6 +339,19 @@ dependencies = [ "tokio-util", ] +[[package]] +name = "datadog-profiling-store" +version = "2.0.0" +dependencies = [ + "ahash", + "anyhow", + "bumpalo", + "crossbeam-channel", + "derivative", + "prost", + "self_cell", +] + [[package]] name = "ddcommon" version = "2.0.0" @@ -1059,6 +1085,12 @@ dependencies = [ "libc", ] +[[package]] +name = "self_cell" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1ef965a420fe14fdac7dd018862966a4c14094f900e1650bbc71ddd7d580c8af" + [[package]] name = "serde" version = "1.0.152" diff --git a/profiling/Cargo.toml b/profiling/Cargo.toml index 2eefe3f4fb1..f4e0ef775c8 100644 --- a/profiling/Cargo.toml +++ b/profiling/Cargo.toml @@ -13,9 +13,10 @@ crate-type = ["cdylib"] [dependencies] anyhow = { version = "1.0" } cfg-if = { version = "1.0" } -crossbeam-channel = { version = "0.5", default-features = false, features = ["std"] } cpu-time = { version = "1.0" } +crossbeam-channel = { version = "0.5", default-features = false, features = ["std"] } datadog-profiling = { git = "https://github.com/DataDog/libdatadog", tag = "v2.0.0" } +datadog-profiling-store = { path = "../profiling-store" } env_logger = { version = "0.9.3" } indexmap = { version = "1.8" } lazy_static = { version = "1.4" } @@ -23,9 +24,9 @@ libc = "0.2" # TRACE set to max to support runtime configuration. log = { version = "0.4", features = ["max_level_trace", "release_max_level_trace"]} once_cell = { version = "1.12" } -uuid = { version = "1.0", features = ["v4"] } rand = { version = "0.8.5" } rand_distr = { version = "0.4.3" } +uuid = { version = "1.0", features = ["v4"] } [features] default = ["allocation_profiling"] diff --git a/profiling/src/bindings/mod.rs b/profiling/src/bindings/mod.rs index 01ce36af4e9..58d0c86051b 100644 --- a/profiling/src/bindings/mod.rs +++ b/profiling/src/bindings/mod.rs @@ -268,6 +268,16 @@ extern "C" { /// strings will be converted into a string view to a static empty string /// (single byte of null, len of 0). pub fn ddog_php_prof_zend_string_view(zstr: Option<&mut zend_string>) -> zai_string_view; + + /// Registers the run_time_cache slot with the engine. Must be done in + /// module init or extension startup. + pub fn ddog_php_prof_function_run_time_cache_init(module_name: *const c_char); + + /// Gets the address of a function's run_time_cache slot. May return None + /// if it detects incomplete initialization, which is always a bug but + /// none-the-less has been seen in the wild. It may also return None if + /// the run_time_cache is not available on this function type. + pub fn ddog_php_prof_function_run_time_cache(func: &zend_function) -> Option<&mut [usize; 2]>; } pub use zend_module_dep as ModuleDep; @@ -490,3 +500,21 @@ pub struct ZaiConfigMemoizedEntry { ) -> c_int, >, } + +#[cfg(test)] +mod tests { + + // If this fails, then ddog_php_prof_function_run_time_cache needs to be + // adjusted accordingly. + #[test] + fn test_sizeof_fixed_size_slice_is_same_as_pointer() { + assert_eq!( + std::mem::size_of::<&[usize; 2]>(), + std::mem::size_of::<*mut usize>() + ); + assert_eq!( + std::mem::align_of::<&[usize; 2]>(), + std::mem::align_of::<*mut usize>() + ); + } +} diff --git a/profiling/src/lib.rs b/profiling/src/lib.rs index 5fb252030c8..0e46c744774 100644 --- a/profiling/src/lib.rs +++ b/profiling/src/lib.rs @@ -49,16 +49,18 @@ static PROFILER: Mutex> = Mutex::new(None); /// interior null bytes and must be null terminated. static PROFILER_NAME: &[u8] = b"datadog-profiling\0"; +/// Name of the profiling module and zend_extension, but as a &CStr. +// Safety: null terminated, contains no interior null bytes. +static PROFILER_NAME_CSTR: &CStr = unsafe { CStr::from_bytes_with_nul_unchecked(PROFILER_NAME) }; + /// Version of the profiling module and zend_extension. Must not contain any /// interior null bytes and must be null terminated. static PROFILER_VERSION: &[u8] = concat!(env!("CARGO_PKG_VERSION"), "\0").as_bytes(); lazy_static! { // Safety: PROFILER_NAME is a byte slice that satisfies the safety requirements. - static ref PROFILER_NAME_STR: &'static str = unsafe { CStr::from_ptr(PROFILER_NAME.as_ptr() as *const c_char) } - .to_str() - // Panic: we own this string and it should be UTF8 (see PROFILER_NAME above). - .unwrap(); + // Panic: we own this string and it should be UTF8 (see PROFILER_NAME above). + static ref PROFILER_NAME_STR: &'static str = PROFILER_NAME_CSTR.to_str().unwrap(); // Safety: PROFILER_VERSION is a byte slice that satisfies the safety requirements. static ref PROFILER_VERSION_STR: &'static str = unsafe { CStr::from_ptr(PROFILER_VERSION.as_ptr() as *const c_char) } @@ -106,7 +108,7 @@ pub extern "C" fn get_module() -> &'static mut zend::ModuleEntry { ]; let module = zend::ModuleEntry { - name: PROFILER_NAME.as_ptr() as *const u8, + name: PROFILER_NAME.as_ptr(), module_startup_func: Some(minit), module_shutdown_func: Some(mshutdown), request_startup_func: Some(rinit), @@ -225,7 +227,7 @@ extern "C" fn minit(r#type: c_int, module_number: c_int) -> ZendResult { * At the time of this writing, PHP 8.2 isn't out yet so it's possible * it may get reverted if issues are found. */ - let str = PROFILER_NAME.as_ptr(); + let str = PROFILER_NAME_CSTR.as_ptr(); let len = PROFILER_NAME.len() - 1; // ignore trailing null byte // Safety: str is valid for at least len values. @@ -706,6 +708,14 @@ extern "C" fn rshutdown(r#type: c_int, module_number: c_int) -> ZendResult { #[cfg(debug_assertions)] trace!("RSHUTDOWN({}, {})", r#type, module_number); + #[cfg(php8)] + { + profiling::FUNCTION_CACHE_STATS.with(|cell| { + let stats = cell.borrow(); + debug!("Process cumulative {stats:?}"); + }); + } + REQUEST_LOCALS.with(|cell| { let mut locals = cell.borrow_mut(); @@ -908,6 +918,9 @@ extern "C" fn startup(extension: *mut ZendExtension) -> ZendResult { // Safety: called during startup hook with correct params. unsafe { zend::datadog_php_profiling_startup(extension) }; + // Safety: calling this in startup/minit as required. + unsafe { bindings::ddog_php_prof_function_run_time_cache_init(PROFILER_NAME_CSTR.as_ptr()) }; + // Ignore a failure as ZEND_VERSION.get() will return an Option if it's not set. let _ = ZEND_VERSION.get_or_try_init(|| { // Safety: CStr string is null-terminated without any interior null bytes. diff --git a/profiling/src/php_ffi.c b/profiling/src/php_ffi.c index 9392a940c3d..5acca6e97de 100644 --- a/profiling/src/php_ffi.c +++ b/profiling/src/php_ffi.c @@ -42,8 +42,8 @@ void datadog_php_profiling_startup(zend_extension *extension) { void *datadog_php_profiling_vm_interrupt_addr(void) { return &EG(vm_interrupt); } -zend_module_entry *datadog_get_module_entry(const uint8_t *str, uintptr_t len) { - return zend_hash_str_find_ptr(&module_registry, (const char *)str, len); +zend_module_entry *datadog_get_module_entry(const char *str, uintptr_t len) { + return zend_hash_str_find_ptr(&module_registry, str, len); } ddtrace_profiling_context (*datadog_php_profiling_get_profiling_context)(void) = @@ -95,8 +95,7 @@ zai_string_view ddog_php_prof_zend_string_view(zend_string *zstr) { void ddog_php_prof_zend_mm_set_custom_handlers(zend_mm_heap *heap, void* (*_malloc)(size_t), void (*_free)(void*), - void* (*_realloc)(void*, size_t)) -{ + void* (*_realloc)(void*, size_t)) { zend_mm_set_custom_handlers(heap, _malloc, _free, _realloc); #if PHP_VERSION_ID < 70300 if (!_malloc && !_free && !_realloc) { @@ -105,7 +104,53 @@ void ddog_php_prof_zend_mm_set_custom_handlers(zend_mm_heap *heap, #endif } -zend_execute_data* ddog_php_prof_get_current_execute_data() -{ +zend_execute_data* ddog_php_prof_get_current_execute_data() { return EG(current_execute_data); } + +#if PHP_VERSION_ID >= 80000 +static int ddog_php_prof_run_time_cache_handle = -1; +#endif + +void ddog_php_prof_function_run_time_cache_init(const char *module_name) { +#if PHP_VERSION_ID >= 80000 + // Grab 2, one for function name and one for filename. + ddog_php_prof_run_time_cache_handle = + zend_get_op_array_extension_handles(module_name, 2); +#endif + + /* It's possible to work on PHP 7.4 as well, but there are opcache bugs + * that weren't truly fixed until PHP 8: + * https://github.com/php/php-src/pull/5871 + * I would rather avoid these bugs for now. + */ +} + +uintptr_t *ddog_php_prof_function_run_time_cache(zend_function *func) { +#if PHP_VERSION_ID < 80000 + /* It's possible to work on PHP 7.4 as well, but there are opcache bugs + * that weren't truly fixed until PHP 8: + * https://github.com/php/php-src/pull/5871 + * I would rather avoid these bugs for now. + */ + return NULL; +#else + + // It should be initialized by this point, or we failed. + if (ddog_php_prof_run_time_cache_handle < 0) return NULL; + +#if PHP_VERSION_ID < 80200 + // internal functions don't have a runtime cache until PHP 8.2 + if (func->type == ZEND_INTERNAL_FUNCTION) return NULL; + + uintptr_t *cache_addr = RUN_TIME_CACHE(&func->op_array); +#else + uintptr_t *cache_addr = RUN_TIME_CACHE(&func->common); +#endif + + // To my knowledge, this is always a bug, but it has happened. + if (!cache_addr) return 0; + + return cache_addr + ddog_php_prof_run_time_cache_handle; +#endif +} diff --git a/profiling/src/php_ffi.h b/profiling/src/php_ffi.h index 571e86475ff..27e04255e5a 100644 --- a/profiling/src/php_ffi.h +++ b/profiling/src/php_ffi.h @@ -35,11 +35,10 @@ const char *datadog_module_build_id(void); /** * Lookup module by name in the module registry. Returns NULL if not found. - * This is meant to be called from Rust, so it uses types that are easy to use - * in Rust. In Rust, strings are validated byte-slices instead of `char` slices - * and array lengths use uintptr_t, not size_t. + * This is meant to be called from Rust, so it uses uintptr_t, not size_t, for + * the length for convenience. */ -zend_module_entry *datadog_get_module_entry(const uint8_t *str, uintptr_t len); +zend_module_entry *datadog_get_module_entry(const char *str, uintptr_t len); /** * Fetches the VM interrupt address of the calling PHP thread. diff --git a/profiling/src/profiling/mod.rs b/profiling/src/profiling/mod.rs index a68fb21e1cc..a94c192d618 100644 --- a/profiling/src/profiling/mod.rs +++ b/profiling/src/profiling/mod.rs @@ -4,7 +4,7 @@ mod thread_utils; mod uploader; pub use interrupts::*; -use stalk_walking::*; +pub use stalk_walking::*; use uploader::*; use crate::bindings::{datadog_php_profiling_get_profiling_context, zend_execute_data}; @@ -280,7 +280,7 @@ impl TimeCollector { let location = Location { lines: vec![Line { function: Function { - name: frame.function.as_str(), + name: frame.function.as_ref(), system_name: "", filename: frame.file.as_deref().unwrap_or(""), start_line: 0, @@ -695,8 +695,8 @@ mod tests { fn get_frames() -> Vec { vec![ZendFrame { - function: "foobar()".to_string(), - file: Some("foobar.php".to_string()), + function: "foobar()".into(), + file: Some("foobar.php".into()), line: 42, }] } diff --git a/profiling/src/profiling/stalk_walking.rs b/profiling/src/profiling/stalk_walking.rs index 36650c3c621..d205942d725 100644 --- a/profiling/src/profiling/stalk_walking.rs +++ b/profiling/src/profiling/stalk_walking.rs @@ -1,15 +1,32 @@ use crate::bindings::{ - ddog_php_prof_zend_string_view, zend_execute_data, zend_function, zend_string, - ZEND_USER_FUNCTION, + ddog_php_prof_function_run_time_cache, ddog_php_prof_zend_string_view, zend_execute_data, + zend_function, zend_string, ZEND_USER_FUNCTION, }; +use datadog_profiling_store::{OwnedStringTable, StringTable}; +use log::debug; +use std::borrow::Cow; +use std::cell::{RefCell, RefMut}; +use std::mem::transmute; use std::str::Utf8Error; +#[derive(Debug, Default)] +pub struct FunctionRunTimeCacheStats { + hit: usize, + missed: usize, + not_applicable: usize, +} + +thread_local! { + static CACHED_STRINGS: RefCell = RefCell::new(OwnedStringTable::new()); + pub static FUNCTION_CACHE_STATS: RefCell = RefCell::new(Default::default()) +} + #[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 "". - pub function: String, - pub file: Option, + // fake name if you need to like ", + pub file: Option>, pub line: u32, // use 0 for no line info } @@ -59,6 +76,72 @@ unsafe fn extract_function_name(func: &zend_function) -> Option { Some(String::from_utf8_lossy(buffer.as_slice()).into_owned()) } +unsafe fn handle_file_cache_slot_helper( + execute_data: &zend_execute_data, + string_table: &mut RefMut, + cache_slots: &mut [usize; 2], +) -> Option> { + let offset = if cache_slots[1] > 0 { + cache_slots[1] + } else { + // Safety: if we have cache slots, we definitely have a func. + let func = &*execute_data.func; + let file = if func.type_ == ZEND_USER_FUNCTION as u8 { + let bytes = zend_string_to_bytes(func.op_array.filename.as_mut()); + String::from_utf8_lossy(bytes) + } else { + return None; + }; + let offset = string_table.insert(file.as_ref()); + cache_slots[1] = offset; + offset + }; + let str = string_table.get_offset(offset); + + // Safety: changing the lifetime to 'static is safe because + // the other threads using it are joined before this thread + // ever dies. + Some(Cow::Borrowed(transmute(str))) +} + +unsafe fn handle_file_cache_slot( + execute_data: &zend_execute_data, + string_table: &mut RefMut, + cache_slots: &mut [usize; 2], +) -> (Option>, u32) { + match handle_file_cache_slot_helper(execute_data, string_table, cache_slots) { + Some(filename) => { + let lineno = match execute_data.opline.as_ref() { + Some(opline) => opline.lineno, + None => 0, + }; + (Some(filename), lineno) + } + None => (None, 0), + } +} + +unsafe fn handle_function_cache_slot( + func: &zend_function, + string_table: &mut RefMut, + cache_slots: &mut [usize; 2], +) -> Option> { + let offset = if cache_slots[0] > 0 { + cache_slots[0] + } else { + let name = extract_function_name(func)?; + let offset = string_table.insert(name.as_ref()); + cache_slots[0] = offset; + offset + }; + let str = string_table.get_offset(offset); + + // Safety: changing the lifetime to 'static is safe because + // the other threads using it are joined before this thread + // ever dies. + Some(Cow::Borrowed(transmute(str))) +} + unsafe fn extract_file_and_line(execute_data: &zend_execute_data) -> (Option, u32) { // This should be Some, just being cautious. match execute_data.func.as_ref() { @@ -76,22 +159,57 @@ unsafe fn extract_file_and_line(execute_data: &zend_execute_data) -> (Option Option { - if let Some(func) = execute_data.func.as_ref() { - let function = extract_function_name(func); - let (file, line) = extract_file_and_line(execute_data); - - // 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.unwrap_or_else(|| " { + FUNCTION_CACHE_STATS.with(|cell| { + let mut stats = cell.borrow_mut(); + if cache_slots[0] == 0 { + if cache_slots[1] != 0 { + debug!( + "function cache slot 0 was zero, but slot 1 was {}", + cache_slots[1] + ); + } + stats.missed += 1; + } else { + if cache_slots[1] == 0 { + debug!("function cache slot 0 was non-zero, but slot 1 was zero"); + } + stats.hit += 1; + } + }); + let function = handle_function_cache_slot(func, &mut string_table, cache_slots); + let (file, line) = + handle_file_cache_slot(execute_data, &mut string_table, cache_slots); + + (function, file, line) + } + + None => { + FUNCTION_CACHE_STATS.with(|cell| { + let mut stats = cell.borrow_mut(); + stats.not_applicable += 1; + }); + let function = extract_function_name(func).map(Cow::Owned); + let (file, line) = extract_file_and_line(execute_data); + let file = file.map(Cow::Owned); + (function, file, line) + } + }; + + if function.is_some() || file.is_some() { + Some(ZendFrame { + function: function.unwrap_or(Cow::Borrowed(" Date: Tue, 28 Feb 2023 22:01:37 -0700 Subject: [PATCH 02/13] Note ZTS unsafety --- profiling/src/profiling/stalk_walking.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/profiling/src/profiling/stalk_walking.rs b/profiling/src/profiling/stalk_walking.rs index d205942d725..e5a888c0591 100644 --- a/profiling/src/profiling/stalk_walking.rs +++ b/profiling/src/profiling/stalk_walking.rs @@ -101,6 +101,7 @@ unsafe fn handle_file_cache_slot_helper( // Safety: changing the lifetime to 'static is safe because // the other threads using it are joined before this thread // ever dies. + // todo: this is _not_ ZTS safe. Some(Cow::Borrowed(transmute(str))) } @@ -139,6 +140,7 @@ unsafe fn handle_function_cache_slot( // Safety: changing the lifetime to 'static is safe because // the other threads using it are joined before this thread // ever dies. + // todo: this is _not_ ZTS safe. Some(Cow::Borrowed(transmute(str))) } From 615f9ae8e23f4a14d6474ca33a06f90e1f8f4f30 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Tue, 28 Feb 2023 22:12:58 -0700 Subject: [PATCH 03/13] sync rust toolchain --- profiling-store/rust-toolchain.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/profiling-store/rust-toolchain.toml b/profiling-store/rust-toolchain.toml index 292fe499e3b..6a56f2ea4f2 100644 --- a/profiling-store/rust-toolchain.toml +++ b/profiling-store/rust-toolchain.toml @@ -1,2 +1,2 @@ [toolchain] -channel = "stable" +channel = "1.64" From 3fe731b6217c86a578be8a3284b2480c467a39bb Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 1 Mar 2023 14:42:37 -0700 Subject: [PATCH 04/13] Add back php7 version, use cfg guards. Tweak stats output. --- profiling/src/lib.rs | 8 +++-- profiling/src/profiling/stalk_walking.rs | 42 +++++++++++++++++++----- 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/profiling/src/lib.rs b/profiling/src/lib.rs index 0e46c744774..a77064ffac8 100644 --- a/profiling/src/lib.rs +++ b/profiling/src/lib.rs @@ -712,7 +712,8 @@ extern "C" fn rshutdown(r#type: c_int, module_number: c_int) -> ZendResult { { profiling::FUNCTION_CACHE_STATS.with(|cell| { let stats = cell.borrow(); - debug!("Process cumulative {stats:?}"); + let hit_rate = stats.hit_rate(); + debug!("Process cumulative {stats:?} hit_rate: {hit_rate}"); }); } @@ -918,8 +919,11 @@ extern "C" fn startup(extension: *mut ZendExtension) -> ZendResult { // Safety: called during startup hook with correct params. unsafe { zend::datadog_php_profiling_startup(extension) }; + #[cfg(php8)] // Safety: calling this in startup/minit as required. - unsafe { bindings::ddog_php_prof_function_run_time_cache_init(PROFILER_NAME_CSTR.as_ptr()) }; + unsafe { + bindings::ddog_php_prof_function_run_time_cache_init(PROFILER_NAME_CSTR.as_ptr()) + }; // Ignore a failure as ZEND_VERSION.get() will return an Option if it's not set. let _ = ZEND_VERSION.get_or_try_init(|| { diff --git a/profiling/src/profiling/stalk_walking.rs b/profiling/src/profiling/stalk_walking.rs index e5a888c0591..a8d35110e3f 100644 --- a/profiling/src/profiling/stalk_walking.rs +++ b/profiling/src/profiling/stalk_walking.rs @@ -9,6 +9,10 @@ use std::cell::{RefCell, RefMut}; use std::mem::transmute; use std::str::Utf8Error; +/// Used to help track the function run_time_cache hit rate. It glosses over +/// the fact that there are two cache slots used, and they don't have to be in +/// sync. However, they usually are, so we simplify. +#[cfg(php8)] #[derive(Debug, Default)] pub struct FunctionRunTimeCacheStats { hit: usize, @@ -16,6 +20,14 @@ pub struct FunctionRunTimeCacheStats { not_applicable: usize, } +impl FunctionRunTimeCacheStats { + pub fn hit_rate(&self) -> f64 { + let denominator = (self.hit + self.missed + self.not_applicable) as f64; + self.hit as f64 / denominator + } +} + +#[cfg(php8)] thread_local! { static CACHED_STRINGS: RefCell = RefCell::new(OwnedStringTable::new()); pub static FUNCTION_CACHE_STATS: RefCell = RefCell::new(Default::default()) @@ -160,6 +172,7 @@ unsafe fn extract_file_and_line(execute_data: &zend_execute_data) -> (Option Option { let func = execute_data.func.as_ref()?; CACHED_STRINGS.with(|cell| { @@ -169,17 +182,8 @@ unsafe fn collect_call_frame(execute_data: &zend_execute_data) -> Option Option Option { + if let Some(func) = execute_data.func.as_ref() { + let function = extract_function_name(func); + let (file, line) = extract_file_and_line(execute_data); + + // 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_else(|| " Result, Utf8Error> { From b005c77b024d80d67245a64e508ff8aeb374e430 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 1 Mar 2023 15:09:52 -0700 Subject: [PATCH 05/13] Fix initial capacity of locations buffer --- profiling/src/profiling/mod.rs | 2 +- profiling/src/profiling/stalk_walking.rs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/profiling/src/profiling/mod.rs b/profiling/src/profiling/mod.rs index a94c192d618..2c8791b429b 100644 --- a/profiling/src/profiling/mod.rs +++ b/profiling/src/profiling/mod.rs @@ -266,7 +266,7 @@ impl TimeCollector { .expect("entry to exist; just inserted it") }; - let mut locations = vec![]; + let mut locations = Vec::with_capacity(message.value.frames.len()); let values = message.value.sample_values; let labels = message diff --git a/profiling/src/profiling/stalk_walking.rs b/profiling/src/profiling/stalk_walking.rs index a8d35110e3f..a706079fac7 100644 --- a/profiling/src/profiling/stalk_walking.rs +++ b/profiling/src/profiling/stalk_walking.rs @@ -3,7 +3,6 @@ use crate::bindings::{ zend_function, zend_string, ZEND_USER_FUNCTION, }; use datadog_profiling_store::{OwnedStringTable, StringTable}; -use log::debug; use std::borrow::Cow; use std::cell::{RefCell, RefMut}; use std::mem::transmute; From f57b17bfa32aa9a5e99349cd0d434f8052b5c241 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 1 Mar 2023 15:15:08 -0700 Subject: [PATCH 06/13] Add missing php8 guard --- profiling/src/profiling/stalk_walking.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/profiling/src/profiling/stalk_walking.rs b/profiling/src/profiling/stalk_walking.rs index a706079fac7..0bf1516a433 100644 --- a/profiling/src/profiling/stalk_walking.rs +++ b/profiling/src/profiling/stalk_walking.rs @@ -19,6 +19,7 @@ pub struct FunctionRunTimeCacheStats { not_applicable: usize, } +#[cfg(php8)] impl FunctionRunTimeCacheStats { pub fn hit_rate(&self) -> f64 { let denominator = (self.hit + self.missed + self.not_applicable) as f64; From e5464b82bfb5cd56fd948a9a4c691f6b08c3f756 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 1 Mar 2023 15:16:03 -0700 Subject: [PATCH 07/13] Clean up include for php7 --- profiling/src/profiling/stalk_walking.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/profiling/src/profiling/stalk_walking.rs b/profiling/src/profiling/stalk_walking.rs index 0bf1516a433..1074ef0a272 100644 --- a/profiling/src/profiling/stalk_walking.rs +++ b/profiling/src/profiling/stalk_walking.rs @@ -1,6 +1,6 @@ use crate::bindings::{ - ddog_php_prof_function_run_time_cache, ddog_php_prof_zend_string_view, zend_execute_data, - zend_function, zend_string, ZEND_USER_FUNCTION, + ddog_php_prof_zend_string_view, zend_execute_data, zend_function, zend_string, + ZEND_USER_FUNCTION, }; use datadog_profiling_store::{OwnedStringTable, StringTable}; use std::borrow::Cow; @@ -177,7 +177,7 @@ unsafe fn collect_call_frame(execute_data: &zend_execute_data) -> Option { FUNCTION_CACHE_STATS.with(|cell| { let mut stats = cell.borrow_mut(); From 61c3f1511c6d4b36b327a88fcdf63248af64ae53 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 1 Mar 2023 15:19:12 -0700 Subject: [PATCH 08/13] =?UTF-8?q?=F0=9F=A4=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- profiling/src/profiling/stalk_walking.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/profiling/src/profiling/stalk_walking.rs b/profiling/src/profiling/stalk_walking.rs index 1074ef0a272..2326f125de0 100644 --- a/profiling/src/profiling/stalk_walking.rs +++ b/profiling/src/profiling/stalk_walking.rs @@ -174,10 +174,11 @@ unsafe fn extract_file_and_line(execute_data: &zend_execute_data) -> (Option Option { + use crate::bindings::ddog_php_prof_function_run_time_cache; let func = execute_data.func.as_ref()?; CACHED_STRINGS.with(|cell| { let mut string_table = cell.borrow_mut(); - let (function, file, line) = match bindings::ddog_php_prof_function_run_time_cache(func) { + let (function, file, line) = match ddog_php_prof_function_run_time_cache(func) { Some(cache_slots) => { FUNCTION_CACHE_STATS.with(|cell| { let mut stats = cell.borrow_mut(); From 471ba11ec2d461e7c9074d31b8dba853dbc0caa6 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 1 Mar 2023 16:01:29 -0700 Subject: [PATCH 09/13] Move string_table into the same module --- profiling-store/Cargo.toml | 17 ----------------- profiling-store/rust-toolchain.toml | 2 -- profiling-store/src/lib.rs | 3 --- profiling/Cargo.toml | 6 ++++-- profiling/src/lib.rs | 1 + profiling/src/profiling/stalk_walking.rs | 2 +- .../src/string_table/borrowed.rs | 13 +++---------- .../src/string_table/bump_owned.rs | 6 +++--- .../src/string_table/mod.rs | 2 +- .../src/string_table/owned.rs | 0 10 files changed, 13 insertions(+), 39 deletions(-) delete mode 100644 profiling-store/Cargo.toml delete mode 100644 profiling-store/rust-toolchain.toml delete mode 100644 profiling-store/src/lib.rs rename {profiling-store => profiling}/src/string_table/borrowed.rs (89%) rename {profiling-store => profiling}/src/string_table/bump_owned.rs (91%) rename {profiling-store => profiling}/src/string_table/mod.rs (97%) rename {profiling-store => profiling}/src/string_table/owned.rs (100%) diff --git a/profiling-store/Cargo.toml b/profiling-store/Cargo.toml deleted file mode 100644 index 9784a1d1252..00000000000 --- a/profiling-store/Cargo.toml +++ /dev/null @@ -1,17 +0,0 @@ -# Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. -# This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present Datadog, Inc. - -[package] -name = "datadog-profiling-store" -version = "2.0.0" -edition = "2021" -license = "Apache-2.0" - -[dependencies] -ahash = "0.8.3" -anyhow = "1.0.68" -bumpalo = { version = "3.12.0", features = ["collections"] } -crossbeam-channel = "0.5.6" -derivative = "2.2.0" -prost = "0.11.6" -self_cell = "0.10.2" diff --git a/profiling-store/rust-toolchain.toml b/profiling-store/rust-toolchain.toml deleted file mode 100644 index 6a56f2ea4f2..00000000000 --- a/profiling-store/rust-toolchain.toml +++ /dev/null @@ -1,2 +0,0 @@ -[toolchain] -channel = "1.64" diff --git a/profiling-store/src/lib.rs b/profiling-store/src/lib.rs deleted file mode 100644 index 0e99c839b18..00000000000 --- a/profiling-store/src/lib.rs +++ /dev/null @@ -1,3 +0,0 @@ -mod string_table; - -pub use string_table::*; diff --git a/profiling/Cargo.toml b/profiling/Cargo.toml index f4e0ef775c8..6dcc24aa983 100644 --- a/profiling/Cargo.toml +++ b/profiling/Cargo.toml @@ -11,13 +11,14 @@ crate-type = ["cdylib"] # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] +ahash = { version = "0.8" } anyhow = { version = "1.0" } +bumpalo = { version = "3.12", features = ["collections"] } cfg-if = { version = "1.0" } cpu-time = { version = "1.0" } crossbeam-channel = { version = "0.5", default-features = false, features = ["std"] } datadog-profiling = { git = "https://github.com/DataDog/libdatadog", tag = "v2.0.0" } -datadog-profiling-store = { path = "../profiling-store" } -env_logger = { version = "0.9.3" } +env_logger = { version = "0.10" } indexmap = { version = "1.8" } lazy_static = { version = "1.4" } libc = "0.2" @@ -26,6 +27,7 @@ log = { version = "0.4", features = ["max_level_trace", "release_max_level_trace once_cell = { version = "1.12" } rand = { version = "0.8.5" } rand_distr = { version = "0.4.3" } +self_cell = { version = "0.10" } uuid = { version = "1.0", features = ["v4"] } [features] diff --git a/profiling/src/lib.rs b/profiling/src/lib.rs index a77064ffac8..ce6b146fadd 100644 --- a/profiling/src/lib.rs +++ b/profiling/src/lib.rs @@ -5,6 +5,7 @@ mod logging; mod pcntl; mod profiling; mod sapi; +mod string_table; use bindings as zend; use bindings::{sapi_globals, ZendExtension, ZendResult}; diff --git a/profiling/src/profiling/stalk_walking.rs b/profiling/src/profiling/stalk_walking.rs index 2326f125de0..0d954c5175f 100644 --- a/profiling/src/profiling/stalk_walking.rs +++ b/profiling/src/profiling/stalk_walking.rs @@ -2,7 +2,7 @@ use crate::bindings::{ ddog_php_prof_zend_string_view, zend_execute_data, zend_function, zend_string, ZEND_USER_FUNCTION, }; -use datadog_profiling_store::{OwnedStringTable, StringTable}; +use crate::string_table::{OwnedStringTable, StringTable}; use std::borrow::Cow; use std::cell::{RefCell, RefMut}; use std::mem::transmute; diff --git a/profiling-store/src/string_table/borrowed.rs b/profiling/src/string_table/borrowed.rs similarity index 89% rename from profiling-store/src/string_table/borrowed.rs rename to profiling/src/string_table/borrowed.rs index ab20ad727b1..1fc4f24b5eb 100644 --- a/profiling-store/src/string_table/borrowed.rs +++ b/profiling/src/string_table/borrowed.rs @@ -3,15 +3,8 @@ use std::collections::HashMap; use std::ops::Range; pub struct BorrowedStringTable<'a> { - pub(super) vec: Vec<&'a str>, - pub(super) map: HashMap<&'a str, usize, RandomState>, -} - -impl<'a> BorrowedStringTable<'a> { - #[inline] - pub fn new() -> Self { - Self::default() - } + pub vec: Vec<&'a str>, + pub map: HashMap<&'a str, usize, RandomState>, } impl<'a> Default for BorrowedStringTable<'a> { @@ -81,7 +74,7 @@ mod tests { #[test] pub fn borrowed_string_table() { - let set = BorrowedStringTable::<'static>::new(); + let set = BorrowedStringTable::<'static>::default(); super::super::tests::basic(set); } } diff --git a/profiling-store/src/string_table/bump_owned.rs b/profiling/src/string_table/bump_owned.rs similarity index 91% rename from profiling-store/src/string_table/bump_owned.rs rename to profiling/src/string_table/bump_owned.rs index c49ea06ef26..3ed2eb4a69d 100644 --- a/profiling-store/src/string_table/bump_owned.rs +++ b/profiling/src/string_table/bump_owned.rs @@ -1,14 +1,14 @@ use bumpalo::{collections, Bump}; use std::ops::Range; -pub(super) struct BorrowedStringTable<'b> { +pub struct BorrowedStringTable<'b> { arena: &'b Bump, - pub(super) set: super::borrowed::BorrowedStringTable<'b>, + pub set: super::borrowed::BorrowedStringTable<'b>, } impl<'b> BorrowedStringTable<'b> { #[inline] - pub(super) fn new(arena: &'b Bump) -> Self { + pub fn new(arena: &'b Bump) -> Self { Self { arena, set: Default::default(), diff --git a/profiling-store/src/string_table/mod.rs b/profiling/src/string_table/mod.rs similarity index 97% rename from profiling-store/src/string_table/mod.rs rename to profiling/src/string_table/mod.rs index 88641d4eea4..0d6fb938eb7 100644 --- a/profiling-store/src/string_table/mod.rs +++ b/profiling/src/string_table/mod.rs @@ -31,7 +31,7 @@ mod tests { use super::*; /// Pass in an empty set, which should only include the empty string at 0. - pub(crate) fn basic(mut set: S) { + pub fn basic(mut set: S) { // the empty string must always be included in the set at 0. let empty_str = set.get_offset(0); assert_eq!("", empty_str); diff --git a/profiling-store/src/string_table/owned.rs b/profiling/src/string_table/owned.rs similarity index 100% rename from profiling-store/src/string_table/owned.rs rename to profiling/src/string_table/owned.rs From 3cbc099abfd951d3b936d459e08fea7cc858a581 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 1 Mar 2023 16:02:22 -0700 Subject: [PATCH 10/13] Update Cargo.lock --- profiling/Cargo.lock | 135 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 115 insertions(+), 20 deletions(-) diff --git a/profiling/Cargo.lock b/profiling/Cargo.lock index 2ff46188fab..d0013c093ab 100644 --- a/profiling/Cargo.lock +++ b/profiling/Cargo.lock @@ -53,7 +53,7 @@ version = "0.2.14" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d9b39be18770d11421cdb1b9947a45dd3f37e93092cbf377614828a319d5fee8" dependencies = [ - "hermit-abi", + "hermit-abi 0.1.19", "libc", "winapi", ] @@ -80,7 +80,7 @@ dependencies = [ "cexpr", "clang-sys", "clap", - "env_logger", + "env_logger 0.9.3", "lazy_static", "lazycell", "log", @@ -289,15 +289,16 @@ dependencies = [ name = "datadog-php-profiling" version = "0.14.0" dependencies = [ + "ahash", "anyhow", "bindgen", + "bumpalo", "cc", "cfg-if", "cpu-time", "crossbeam-channel", "datadog-profiling", - "datadog-profiling-store", - "env_logger", + "env_logger 0.10.0", "indexmap", "lazy_static", "libc", @@ -305,6 +306,7 @@ dependencies = [ "once_cell", "rand", "rand_distr", + "self_cell", "uuid", ] @@ -339,19 +341,6 @@ dependencies = [ "tokio-util", ] -[[package]] -name = "datadog-profiling-store" -version = "2.0.0" -dependencies = [ - "ahash", - "anyhow", - "bumpalo", - "crossbeam-channel", - "derivative", - "prost", - "self_cell", -] - [[package]] name = "ddcommon" version = "2.0.0" @@ -405,6 +394,40 @@ dependencies = [ "termcolor", ] +[[package]] +name = "env_logger" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "85cdab6a89accf66733ad5a1693a4dcced6aeff64602b634530dd73c1f3ee9f0" +dependencies = [ + "humantime", + "is-terminal", + "log", + "regex", + "termcolor", +] + +[[package]] +name = "errno" +version = "0.2.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f639046355ee4f37944e44f60642c6f3a7efa3cf6b78c78a0d989a8ce6c396a1" +dependencies = [ + "errno-dragonfly", + "libc", + "winapi", +] + +[[package]] +name = "errno-dragonfly" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "aa68f1b12764fab894d2755d2518754e71b4fd80ecfb822714a1206c2aab39bf" +dependencies = [ + "cc", + "libc", +] + [[package]] name = "fnv" version = "1.0.7" @@ -532,6 +555,12 @@ dependencies = [ "libc", ] +[[package]] +name = "hermit-abi" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fed44880c466736ef9a5c5b5facefb5ed0785676d0c02d612db14e54f0d84286" + [[package]] name = "hex" version = "0.4.3" @@ -662,6 +691,28 @@ dependencies = [ "hashbrown", ] +[[package]] +name = "io-lifetimes" +version = "1.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1abeb7a0dd0f8181267ff8adc397075586500b81b28a73e8a0208b00fc170fb3" +dependencies = [ + "libc", + "windows-sys 0.45.0", +] + +[[package]] +name = "is-terminal" +version = "0.4.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "21b6b32576413a8e69b90e952e4a026476040d81017b80445deda5f2d3921857" +dependencies = [ + "hermit-abi 0.3.1", + "io-lifetimes", + "rustix", + "windows-sys 0.45.0", +] + [[package]] name = "itertools" version = "0.10.5" @@ -729,6 +780,12 @@ dependencies = [ "cc", ] +[[package]] +name = "linux-raw-sys" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f051f77a7c8e6957c0696eac88f26b0117e54f52d3fc682ab19397a8812846a4" + [[package]] name = "log" version = "0.4.17" @@ -784,7 +841,7 @@ dependencies = [ "libc", "log", "wasi", - "windows-sys", + "windows-sys 0.42.0", ] [[package]] @@ -998,6 +1055,20 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "08d43f7aa6b08d49f382cde6a7982047c3426db949b1424bc4b7ec9ae12c6ce2" +[[package]] +name = "rustix" +version = "0.36.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f43abb88211988493c1abb44a70efa56ff0ce98f233b7b276146f1f3f7ba9644" +dependencies = [ + "bitflags", + "errno", + "io-lifetimes", + "libc", + "linux-raw-sys", + "windows-sys 0.45.0", +] + [[package]] name = "rustls" version = "0.20.8" @@ -1043,7 +1114,7 @@ version = "0.1.21" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "713cfb06c7059f3588fb8044c0fad1d09e3c01d225e25b9220dbfdcf16dbb1b3" dependencies = [ - "windows-sys", + "windows-sys 0.42.0", ] [[package]] @@ -1226,7 +1297,7 @@ dependencies = [ "pin-project-lite", "socket2", "tokio-macros", - "windows-sys", + "windows-sys 0.42.0", ] [[package]] @@ -1501,6 +1572,30 @@ dependencies = [ "windows_x86_64_msvc", ] +[[package]] +name = "windows-sys" +version = "0.45.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "75283be5efb2831d37ea142365f009c02ec203cd29a3ebecbc093d52315b66d0" +dependencies = [ + "windows-targets", +] + +[[package]] +name = "windows-targets" +version = "0.42.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e2522491fbfcd58cc84d47aeb2958948c4b8982e9a2d8a2a35bbaed431390e7" +dependencies = [ + "windows_aarch64_gnullvm", + "windows_aarch64_msvc", + "windows_i686_gnu", + "windows_i686_msvc", + "windows_x86_64_gnu", + "windows_x86_64_gnullvm", + "windows_x86_64_msvc", +] + [[package]] name = "windows_aarch64_gnullvm" version = "0.42.1" From bbd29c840b42ca375d0aaa490bf27c7f0932189a Mon Sep 17 00:00:00 2001 From: Florian Engelhardt Date: Thu, 2 Mar 2023 12:36:38 +0100 Subject: [PATCH 11/13] allowe scheduled run with fore trigger --- .gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index fea64abd799..cf2706d33bb 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -40,7 +40,7 @@ build: deploy_to_reliability_env: stage: deploy rules: - - if: '$CI_PIPELINE_SOURCE == "schedule"' + - if: '$CI_PIPELINE_SOURCE == "schedule" && $FORCE_TRIGGER != "true"' when: never trigger: project: DataDog/apm-reliability/datadog-reliability-env From ab428daf298bb0d80cbd07ac00c5b35dfbcc0ce1 Mon Sep 17 00:00:00 2001 From: Florian Engelhardt Date: Thu, 2 Mar 2023 12:40:34 +0100 Subject: [PATCH 12/13] allowe scheduled run with fore trigger --- .gitlab-ci.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index cf2706d33bb..e18aff5403e 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -40,7 +40,8 @@ build: deploy_to_reliability_env: stage: deploy rules: - - if: '$CI_PIPELINE_SOURCE == "schedule" && $FORCE_TRIGGER != "true"' + - if: '$FORCE_TRIGGER == "true"' + - if: '$CI_PIPELINE_SOURCE == "schedule"' when: never trigger: project: DataDog/apm-reliability/datadog-reliability-env From c3ef2afce887166a24714b93db379913a8256262 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Thu, 2 Mar 2023 09:11:14 -0700 Subject: [PATCH 13/13] Fix handles for older PHP 8 versions --- profiling/src/php_ffi.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/profiling/src/php_ffi.c b/profiling/src/php_ffi.c index 5acca6e97de..d8ecf24fb00 100644 --- a/profiling/src/php_ffi.c +++ b/profiling/src/php_ffi.c @@ -115,8 +115,15 @@ static int ddog_php_prof_run_time_cache_handle = -1; void ddog_php_prof_function_run_time_cache_init(const char *module_name) { #if PHP_VERSION_ID >= 80000 // Grab 2, one for function name and one for filename. +#if PHP_VERSION_ID < 80200 + ddog_php_prof_run_time_cache_handle = + zend_get_op_array_extension_handle(module_name); + int second = zend_get_op_array_extension_handle(module_name); + ZEND_ASSERT(ddog_php_prof_run_time_cache_handle + 1 == second); +#else ddog_php_prof_run_time_cache_handle = zend_get_op_array_extension_handles(module_name, 2); +#endif #endif /* It's possible to work on PHP 7.4 as well, but there are opcache bugs