diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index fc9c1a2d7b5..0577acc7e8f 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -40,8 +40,9 @@ build: deploy_to_reliability_env: stage: deploy rules: - - if: '$CI_PIPELINE_SOURCE != "schedule"' - when: always + - if: '$FORCE_TRIGGER == "true"' + - if: '$CI_PIPELINE_SOURCE == "schedule"' + when: never trigger: project: DataDog/apm-reliability/datadog-reliability-env branch: $DOWNSTREAM_REL_BRANCH diff --git a/profiling/Cargo.lock b/profiling/Cargo.lock index e476dc409b4..37cc96b8efb 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" @@ -41,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", ] @@ -68,7 +80,7 @@ dependencies = [ "cexpr", "clang-sys", "clap", - "env_logger", + "env_logger 0.9.3", "lazy_static", "lazycell", "log", @@ -277,14 +289,16 @@ dependencies = [ name = "datadog-php-profiling" version = "0.15.0" dependencies = [ + "ahash", "anyhow", "bindgen", + "bumpalo", "cc", "cfg-if", "cpu-time", "crossbeam-channel", "datadog-profiling", - "env_logger", + "env_logger 0.10.0", "indexmap", "lazy_static", "libc", @@ -292,6 +306,7 @@ dependencies = [ "once_cell", "rand", "rand_distr", + "self_cell", "uuid", ] @@ -379,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" @@ -506,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" @@ -636,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" @@ -703,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" @@ -758,7 +841,7 @@ dependencies = [ "libc", "log", "wasi", - "windows-sys", + "windows-sys 0.42.0", ] [[package]] @@ -972,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" @@ -1017,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]] @@ -1059,6 +1156,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" @@ -1194,7 +1297,7 @@ dependencies = [ "pin-project-lite", "socket2", "tokio-macros", - "windows-sys", + "windows-sys 0.42.0", ] [[package]] @@ -1469,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" diff --git a/profiling/Cargo.toml b/profiling/Cargo.toml index 2c2ee94ec60..31714967e3c 100644 --- a/profiling/Cargo.toml +++ b/profiling/Cargo.toml @@ -11,21 +11,24 @@ 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" } -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" } -env_logger = { version = "0.9.3" } +env_logger = { version = "0.10" } indexmap = { version = "1.8" } lazy_static = { version = "1.4" } 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" } +self_cell = { version = "0.10" } +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 b819a5c2c64..0dbf3f97bb7 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]>; } #[cfg(php_preload)] @@ -498,3 +508,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 40b77ebc918..61cbec46482 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}; @@ -49,16 +50,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 +109,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 +228,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. @@ -730,6 +733,15 @@ 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(); + let hit_rate = stats.hit_rate(); + debug!("Process cumulative {stats:?} hit_rate: {hit_rate}"); + }); + } + REQUEST_LOCALS.with(|cell| { let mut locals = cell.borrow_mut(); @@ -932,6 +944,12 @@ 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()) + }; + // 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 e945b5515da..ad439a2ef52 100644 --- a/profiling/src/php_ffi.c +++ b/profiling/src/php_ffi.c @@ -79,8 +79,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) = @@ -132,8 +132,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) { @@ -142,7 +141,60 @@ 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. +#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 + * 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 c82955b7559..36668971680 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}; @@ -277,7 +277,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 @@ -291,7 +291,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, @@ -717,8 +717,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..0d954c5175f 100644 --- a/profiling/src/profiling/stalk_walking.rs +++ b/profiling/src/profiling/stalk_walking.rs @@ -2,14 +2,43 @@ use crate::bindings::{ ddog_php_prof_zend_string_view, zend_execute_data, zend_function, zend_string, ZEND_USER_FUNCTION, }; +use crate::string_table::{OwnedStringTable, StringTable}; +use std::borrow::Cow; +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, + missed: usize, + not_applicable: usize, +} + +#[cfg(php8)] +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()) +} + #[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 +88,74 @@ 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. + // todo: this is _not_ ZTS safe. + 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. + // todo: this is _not_ ZTS safe. + 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() { @@ -75,6 +172,54 @@ 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 ddog_php_prof_function_run_time_cache(func) { + Some(cache_slots) => { + FUNCTION_CACHE_STATS.with(|cell| { + let mut stats = cell.borrow_mut(); + if cache_slots[0] == 0 { + stats.missed += 1; + } else { + 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(" Option { if let Some(func) = execute_data.func.as_ref() { let function = extract_function_name(func); @@ -83,10 +228,10 @@ unsafe fn collect_call_frame(execute_data: &zend_execute_data) -> Option { + pub vec: Vec<&'a str>, + pub map: HashMap<&'a str, usize, RandomState>, +} + +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>::default(); + super::super::tests::basic(set); + } +} diff --git a/profiling/src/string_table/bump_owned.rs b/profiling/src/string_table/bump_owned.rs new file mode 100644 index 00000000000..3ed2eb4a69d --- /dev/null +++ b/profiling/src/string_table/bump_owned.rs @@ -0,0 +1,70 @@ +use bumpalo::{collections, Bump}; +use std::ops::Range; + +pub struct BorrowedStringTable<'b> { + arena: &'b Bump, + pub set: super::borrowed::BorrowedStringTable<'b>, +} + +impl<'b> BorrowedStringTable<'b> { + #[inline] + pub 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/src/string_table/mod.rs b/profiling/src/string_table/mod.rs new file mode 100644 index 00000000000..0d6fb938eb7 --- /dev/null +++ b/profiling/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 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); + } +}