perf(profiling): speed up stack walking by using function run_time_cache

profiling/Cargo.toml

@@ -12,10 +12,12 @@ bench = false # disables cargo build in libtest bench
 # 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.2.0" }
 ddcommon = { git = "https://github.com/DataDog/libdatadog", tag = "v2.2.0" }
 env_logger = { version = "0.10" }
@@ -25,9 +27,10 @@ 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"] }
+ouroboros = { version = "0.17.0" }
 rand = { version = "0.8.5" }
 rand_distr = { version = "0.4.3" }
+uuid = { version = "1.0", features = ["v4"] }
 
 [dev-dependencies]
 criterion = { version = "0.4" }

profiling/benches/stack_walking.rs

@@ -16,7 +16,7 @@ fn benchmark(c: &mut Criterion) {
         let stack = unsafe { zend::ddog_php_test_create_fake_zend_execute_data(99) };
         group.throughput(criterion::Throughput::Elements(*depth as u64));
         group.bench_with_input(BenchmarkId::from_parameter(depth), depth, |b, &_depth| {
-            b.iter(|| unsafe { collect_stack_sample(black_box(stack)) })
+            b.iter(|| collect_stack_sample(black_box(stack)))
         });
     }
     group.finish();

profiling/build.rs

@@ -25,9 +25,10 @@ fn main() {
 
     let vernum = php_config_vernum();
     let preload = cfg_preload(vernum);
+    let run_time_cache = cfg_run_time_cache(vernum);
 
     generate_bindings(php_config_includes);
-    build_zend_php_ffis(php_config_includes, preload);
+    build_zend_php_ffis(php_config_includes, preload, run_time_cache);
 
     cfg_php_major_version(vernum);
     cfg_php_feature_flags(vernum);
@@ -68,7 +69,7 @@ const ZAI_H_FILES: &[&str] = &[
     "../zend_abstract_interface/json/json.h",
 ];
 
-fn build_zend_php_ffis(php_config_includes: &str, preload: bool) {
+fn build_zend_php_ffis(php_config_includes: &str, preload: bool, run_time_cache: bool) {
     println!("cargo:rerun-if-changed=src/php_ffi.h");
     println!("cargo:rerun-if-changed=src/php_ffi.c");
     println!("cargo:rerun-if-changed=../ext/handlers_api.c");
@@ -98,6 +99,7 @@ fn build_zend_php_ffis(php_config_includes: &str, preload: bool) {
 
     let files = ["src/php_ffi.c", "../ext/handlers_api.c"];
     let preload = if preload { "1" } else { "0" };
+    let run_time_cache = if run_time_cache { "1" } else { "0" };
 
     #[cfg(feature = "stack_walking_tests")]
     let stack_walking_tests = "1";
@@ -108,6 +110,7 @@ fn build_zend_php_ffis(php_config_includes: &str, preload: bool) {
     cc::Build::new()
         .files(files.into_iter().chain(zai_c_files.into_iter()))
         .define("CFG_PRELOAD", preload)
+        .define("CFG_RUN_TIME_CACHE", run_time_cache)
         .define("CFG_STACK_WALKING_TESTS", stack_walking_tests)
         .includes([Path::new("../ext")])
         .includes(
@@ -212,6 +215,15 @@ fn cfg_preload(vernum: u64) -> bool {
     }
 }
 
+fn cfg_run_time_cache(vernum: u64) -> bool {
+    if vernum >= 80000 {
+        println!("cargo:rustc-cfg=php_run_time_cache");
+        true
+    } else {
+        false
+    }
+}
+
 fn cfg_php_major_version(vernum: u64) {
     let major_version = match vernum {
         70000..=79999 => 7,

profiling/src/bindings/mod.rs

@@ -286,6 +286,23 @@ 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.
+    #[cfg(not(feature = "stack_walking_tests"))]
+    pub fn ddog_php_prof_function_run_time_cache(func: &zend_function) -> Option<&mut [usize; 2]>;
+
+    /// mock for testing
+    #[cfg(feature = "stack_walking_tests")]
+    pub fn ddog_test_php_prof_function_run_time_cache(
+        func: &zend_function,
+    ) -> Option<&mut [usize; 2]>;
 }
 
 #[cfg(php_preload)]
@@ -525,3 +542,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>()
+        );
+    }
+}

profiling/src/lib.rs

@@ -5,6 +5,7 @@ mod logging;
 mod pcntl;
 pub mod profiling;
 mod sapi;
+mod string_table;
 
 #[cfg(feature = "allocation_profiling")]
 mod allocation;
@@ -48,16 +49,18 @@ static PROFILER: Mutex<Option<Profiler>> = 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) }
@@ -110,7 +113,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),
@@ -214,7 +217,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.
@@ -250,6 +253,7 @@ extern "C" fn minit(r#type: c_int, module_number: c_int) -> ZendResult {
         copyright: b"Copyright Datadog\0".as_ptr(),
         startup: Some(startup),
         shutdown: Some(shutdown),
+        activate: Some(activate),
         ..Default::default()
     };
 
@@ -348,6 +352,11 @@ fn runtime_id() -> &'static Uuid {
         .get_or_init(|| unsafe { ddtrace_runtime_id.as_ref() }.map_or_else(Uuid::new_v4, |u| *u))
 }
 
+extern "C" fn activate() {
+    // Safety: calling in activate as required.
+    unsafe { profiling::activate_run_time_cache() };
+}
+
 /* If Failure is returned the VM will do a C exit; try hard to avoid that,
  * using it for catastrophic errors only.
  */
@@ -629,6 +638,15 @@ extern "C" fn rshutdown(r#type: c_int, module_number: c_int) -> ZendResult {
     #[cfg(debug_assertions)]
     trace!("RSHUTDOWN({}, {})", r#type, module_number);
 
+    #[cfg(php_run_time_cache)]
+    {
+        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();
 
@@ -810,6 +828,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(php_run_time_cache)]
+    // 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.

profiling/src/php_ffi.c

@@ -65,7 +65,22 @@ static post_startup_cb_result ddog_php_prof_post_startup_cb(void) {
 }
 #endif
 
+
+#if CFG_RUN_TIME_CACHE // defined by build.rs
+/**
+ * Currently used to ignore run_time_cache on CLI SAPI as a precaution against
+ * unbounded memory growth. Unbounded growth is more likely there since it's
+ * always one PHP request, and we only reset it on each new request.
+ */
+static bool _ignore_run_time_cache = false;
+#endif
+
 void datadog_php_profiling_startup(zend_extension *extension) {
+
+#if CFG_RUN_TIME_CACHE // defined by build.rs
+    _ignore_run_time_cache = strcmp(sapi_module.name, "cli") == 0;
+#endif
+
     datadog_php_profiling_get_profiling_context = noop_get_profiling_context;
 
     /* Due to the optional dependency on ddtrace, the profiling module will be
@@ -91,8 +106,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) =
@@ -144,8 +159,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) {
@@ -154,11 +168,114 @@ 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 CFG_RUN_TIME_CACHE // defined by build.rs
+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 CFG_RUN_TIME_CACHE // defined by build.rs
+    // 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
+#else
+    (void)module_name;
+#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.
+     */
+}
+
+#if CFG_RUN_TIME_CACHE // defined by build.rs
+static bool has_invalid_run_time_cache(zend_function const *func) {
+    if (UNEXPECTED(_ignore_run_time_cache) || UNEXPECTED(ddog_php_prof_run_time_cache_handle < 0))
+        return true;
+
+    // Trampolines use the extension slot for internal things.
+    bool is_trampoline = func->common.fn_flags & ZEND_ACC_CALL_VIA_TRAMPOLINE;
+
+#if PHP_VERSION_ID < 80200
+    // Internal functions don't have a runtime cache until PHP 8.2.
+    bool is_internal = func->type == ZEND_INTERNAL_FUNCTION;
+
+    // The bitwise-or is intentional. Branch prediction is not going to be great
+    // on either of these, so reducing the number of branches is preferred.
+    return is_trampoline | is_internal;
+#else
+    return is_trampoline;
+#endif
+}
+#endif
+
+uintptr_t *ddog_php_prof_function_run_time_cache(zend_function const *func) {
+#if CFG_STACK_WALKING_TESTS
+    return NULL;
+#endif
+#if CFG_RUN_TIME_CACHE
+    if (UNEXPECTED(has_invalid_run_time_cache(func))) return NULL;
+
+#if PHP_VERSION_ID < 80200
+    // Internal functions don't have a runtime cache until PHP 8.2.
+    uintptr_t *cache_addr = RUN_TIME_CACHE(&func->op_array);
+#else
+    uintptr_t *cache_addr = RUN_TIME_CACHE(&func->common);
+#endif
+
+    /* The above checks, including has_invalid_run_time_cache, protect this.
+     * It's better to fail now on the null, than to wait for the returned addr
+     * to get used, in the event future code changes screw this up.
+     */
+    ZEND_ASSERT(cache_addr);
+
+    return cache_addr + ddog_php_prof_run_time_cache_handle;
+
+#else
+    /* 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;
+#endif
+}
+
+#if CFG_STACK_WALKING_TESTS
+uintptr_t *ddog_test_php_prof_function_run_time_cache(zend_function const *func) {
+#if CFG_RUN_TIME_CACHE
+    if (_ignore_run_time_cache) return NULL;
+    zend_function *non_const_func = (zend_function *)func;
+#if PHP_VERSION_ID < 80200
+    if (non_const_func->op_array.run_time_cache__ptr == NULL) {
+        non_const_func->op_array.run_time_cache__ptr = calloc(1, sizeof(uintptr_t));
+        *non_const_func->op_array.run_time_cache__ptr = calloc(2, sizeof(uintptr_t));
+    }
+    return *non_const_func->op_array.run_time_cache__ptr;
+#else
+    if (non_const_func->common.run_time_cache__ptr == NULL) {
+        non_const_func->common.run_time_cache__ptr = calloc(1, sizeof(uintptr_t));
+        *non_const_func->common.run_time_cache__ptr = calloc(2, sizeof(uintptr_t));
+    }
+    return *non_const_func->common.run_time_cache__ptr;
+#endif
+#else
+    return NULL;
+#endif
+}
+#endif
+
 #if CFG_STACK_WALKING_TESTS
 static int (*og_snprintf)(char *, size_t, const char *, ...);