From 241f72a438a75754171b82e9cb81aafc8886d327 Mon Sep 17 00:00:00 2001 From: Florian Engelhardt Date: Mon, 27 Jul 2026 16:39:30 +0200 Subject: [PATCH 01/25] perf(profiling): avoid FFI call on allocation hot path Read executor_globals.current_execute_data directly on NTS builds instead of calling through the C FFI wrapper for every allocation and reallocation. Keep the existing wrapper on ZTS builds. A 60-second macOS sample reduced get_current_execute_data self time from 0.412% (207/50,210 main-thread samples) to 0.002% (1/49,737). Known allocation-hook self time fell from 4.184% to 3.601%, a 13.9% relative reduction. Repeated end-to-end throughput remained within system scheduling noise. Validation: cargo test (22 passed); allocation sampling-distance, memory-peak, and GC PHPTs passed. --- profiling/src/allocation/allocation_ge84.rs | 7 ++++--- profiling/src/allocation/allocation_le83.rs | 7 ++++--- profiling/src/allocation/mod.rs | 9 +++++++++ 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/profiling/src/allocation/allocation_ge84.rs b/profiling/src/allocation/allocation_ge84.rs index 606f32df14..80670585aa 100644 --- a/profiling/src/allocation/allocation_ge84.rs +++ b/profiling/src/allocation/allocation_ge84.rs @@ -1,5 +1,6 @@ use crate::allocation::{ - allocation_profiling_stats_should_collect, collect_allocation, untrack_allocation, + allocation_profiling_stats_should_collect, collect_allocation, current_execute_data, + untrack_allocation, }; use crate::bindings as zend; use crate::PROFILER_NAME; @@ -306,7 +307,7 @@ unsafe fn alloc_prof_malloc_impl(len: size_t) -> *mut c_void { // during startup, minit, rinit, ... current_execute_data is null // we are only interested in allocations during userland operations - if zend::ddog_php_prof_get_current_execute_data().is_null() { + if current_execute_data().is_null() { return ptr; } @@ -505,7 +506,7 @@ unsafe fn alloc_prof_realloc_no_untrack_impl(prev_ptr: *mut c_void, len: size_t) unsafe fn alloc_prof_realloc_sample(ptr: *mut c_void, len: size_t) -> *mut c_void { // during startup, minit, rinit, ... current_execute_data is null // we are only interested in allocations during userland operations - if zend::ddog_php_prof_get_current_execute_data().is_null() { + if current_execute_data().is_null() { return ptr; } diff --git a/profiling/src/allocation/allocation_le83.rs b/profiling/src/allocation/allocation_le83.rs index c36de739e9..12a1b37c58 100644 --- a/profiling/src/allocation/allocation_le83.rs +++ b/profiling/src/allocation/allocation_le83.rs @@ -1,5 +1,6 @@ use crate::allocation::{ - allocation_profiling_stats_should_collect, collect_allocation, untrack_allocation, + allocation_profiling_stats_should_collect, collect_allocation, current_execute_data, + untrack_allocation, }; use crate::bindings::{ self as zend, datadog_php_install_handler, datadog_php_zif_handler, @@ -300,7 +301,7 @@ unsafe extern "C" fn alloc_prof_malloc(len: size_t) -> *mut c_void { // during startup, minit, rinit, ... current_execute_data is null // we are only interested in allocations during userland operations - if zend::ddog_php_prof_get_current_execute_data().is_null() { + if current_execute_data().is_null() { return ptr; } @@ -431,7 +432,7 @@ unsafe fn alloc_prof_realloc_no_untrack_impl(prev_ptr: *mut c_void, len: size_t) unsafe fn alloc_prof_realloc_sample(ptr: *mut c_void, len: size_t) -> *mut c_void { // during startup, minit, rinit, ... current_execute_data is null // we are only interested in allocations during userland operations - if zend::ddog_php_prof_get_current_execute_data().is_null() { + if current_execute_data().is_null() { return ptr; } diff --git a/profiling/src/allocation/mod.rs b/profiling/src/allocation/mod.rs index 9d4f46d53c..20f4d2280a 100644 --- a/profiling/src/allocation/mod.rs +++ b/profiling/src/allocation/mod.rs @@ -39,6 +39,15 @@ pub(crate) unsafe fn get_zend_mm_state() -> *mut Cell { ptr::addr_of_mut!((*globals).zend_mm_state) } +#[inline(always)] +pub(crate) unsafe fn current_execute_data() -> *mut zend::zend_execute_data { + #[cfg(not(php_zts))] + return ptr::addr_of!(zend::executor_globals.current_execute_data).read(); + + #[cfg(php_zts)] + zend::ddog_php_prof_get_current_execute_data() +} + /// Macros for accessing ZendMMState from PHP globals. /// These are shared between PHP 8.3- and 8.4+ implementations. /// They are exported at the crate root and can be used in submodules. From 6e8788349d19029cacd0c45aa3e49939d4265bb0 Mon Sep 17 00:00:00 2001 From: Florian Engelhardt Date: Mon, 27 Jul 2026 17:02:46 +0200 Subject: [PATCH 02/25] perf(profiling): remove indirect allocator forwarding Forward PHP 8.4+ allocations and frees from a single ZendMMState read instead of loading a function pointer, making an indirect Rust call, and reloading the state. Preserve neighboring custom allocator support with a predictable previous-handler check. Use unchecked heap extraction in the callbacks: rinit stores the heap before they can be invoked, and rshutdown removes them before clearing it. This avoids an Option discriminant check on every allocation and free. Across six 60-second runs per binary, mean allocation throughput increased from 9,366,444/s to 10,018,262/s (+6.96%) and median throughput increased by 7.56%. A 60-second native sample reduced known forwarding-path self time from 3.599% to 2.438% (-32.3% relative). Validation: cargo test (22 passed); allocation sampling-distance, memory-peak, and GC PHPTs passed. https://datadoghq.atlassian.net/browse/PROF-15506 --- profiling/src/allocation/allocation_ge84.rs | 84 +++++++-------------- profiling/src/allocation/mod.rs | 2 + 2 files changed, 28 insertions(+), 58 deletions(-) diff --git a/profiling/src/allocation/allocation_ge84.rs b/profiling/src/allocation/allocation_ge84.rs index 80670585aa..f19ed81d88 100644 --- a/profiling/src/allocation/allocation_ge84.rs +++ b/profiling/src/allocation/allocation_ge84.rs @@ -34,21 +34,11 @@ pub struct ZendMMState { /// The engine's previous custom shutdown function, if there is one. prev_custom_mm_shutdown: Option, /// Safety: this function pointer is only allowed to point to - /// `alloc_prof_prev_alloc()` when at the same time the - /// `ZEND_MM_STATE.prev_custom_mm_alloc` is initialised to a valid function - /// pointer, otherwise there will be dragons. - alloc: unsafe fn(size_t) -> *mut c_void, - /// Safety: this function pointer is only allowed to point to /// `alloc_prof_prev_realloc()` when at the same time the /// `ZEND_MM_STATE.prev_custom_mm_realloc` is initialised to a valid /// function pointer, otherwise there will be dragons. realloc: unsafe fn(*mut c_void, size_t) -> *mut c_void, /// Safety: this function pointer is only allowed to point to - /// `alloc_prof_prev_free()` when at the same time the - /// `ZEND_MM_STATE.prev_custom_mm_free` is initialised to a valid function - /// pointer, otherwise there will be dragons. - free: unsafe fn(*mut c_void), - /// Safety: this function pointer is only allowed to point to /// `alloc_prof_prev_gc()` when at the same time the /// `ZEND_MM_STATE.prev_custom_mm_gc` is initialised to a valid function /// pointer, otherwise there will be dragons. @@ -78,9 +68,7 @@ impl ZendMMState { prev_custom_mm_free: None, prev_custom_mm_gc: None, prev_custom_mm_shutdown: None, - alloc: super::alloc_prof_panic_alloc, realloc: super::alloc_prof_panic_realloc, - free: super::alloc_prof_panic_free, gc: alloc_prof_panic_gc, shutdown: alloc_prof_panic_shutdown, } @@ -127,14 +115,10 @@ pub fn alloc_prof_rinit(heap_live_enabled: bool) { ptr::addr_of_mut!(zend_mm_state.prev_custom_mm_shutdown), ); } - zend_mm_state.alloc = alloc_prof_prev_alloc; - zend_mm_state.free = alloc_prof_prev_free; zend_mm_state.realloc = alloc_prof_prev_realloc; zend_mm_state.gc = alloc_prof_prev_gc; zend_mm_state.shutdown = alloc_prof_prev_shutdown; } else { - zend_mm_state.alloc = alloc_prof_orig_alloc; - zend_mm_state.free = alloc_prof_orig_free; zend_mm_state.realloc = alloc_prof_orig_realloc; zend_mm_state.gc = alloc_prof_orig_gc; zend_mm_state.shutdown = alloc_prof_orig_shutdown; @@ -303,7 +287,7 @@ unsafe fn alloc_prof_malloc_impl(len: size_t) -> *mut c_void { #[cfg(feature = "debug_stats")] ALLOCATION_PROFILING_SIZE.fetch_add(len as u64, Relaxed); - let ptr = tls_zend_mm_state_get!(alloc)(len); + let ptr = alloc_prof_forward_alloc(len); // during startup, minit, rinit, ... current_execute_data is null // we are only interested in allocations during userland operations @@ -318,27 +302,19 @@ unsafe fn alloc_prof_malloc_impl(len: size_t) -> *mut c_void { ptr } -unsafe fn alloc_prof_prev_alloc(len: size_t) -> *mut c_void { - // Safety: `ZEND_MM_STATE.prev_custom_mm_alloc` will be initialised in - // `alloc_prof_rinit()` and only point to this function when - // `prev_custom_mm_alloc` is also initialised. - // Note: We use `.unwrap()` instead of `.unwrap_unchecked()` here because a - // neighboring extension could misbehave. If that happens, we want a proper - // panic with backtrace for debugging rather than undefined behavior. - let alloc = tls_zend_mm_state_get!(prev_custom_mm_alloc).unwrap(); - #[cfg(php_debug)] - { - alloc(len, ptr::null(), 0, ptr::null(), 0) +#[inline(always)] +unsafe fn alloc_prof_forward_alloc(len: size_t) -> *mut c_void { + let state = tls_zend_mm_state_copy!(); + if let Some(alloc) = state.prev_custom_mm_alloc { + #[cfg(php_debug)] + return alloc(len, ptr::null(), 0, ptr::null(), 0); + #[cfg(not(php_debug))] + return alloc(len); } - #[cfg(not(php_debug))] - alloc(len) -} -unsafe fn alloc_prof_orig_alloc(len: size_t) -> *mut c_void { - // Safety: `ZEND_MM_STATE.heap` will be initialised in `alloc_prof_rinit()` and custom ZendMM - // handlers only point to this function after successful init. Using `unwrap_unchecked()` is - // safe here as we have full control over ZendMM with no neighboring extensions. - let heap = tls_zend_mm_state_get!(heap).unwrap_unchecked(); + // SAFETY: this callback is only invoked after rinit stores the heap and + // before rshutdown clears it. + let heap = state.heap.unwrap_unchecked(); #[cfg(php_debug)] return zend::_zend_mm_alloc(heap, len, ptr::null(), 0, ptr::null(), 0); #[cfg(not(php_debug))] @@ -375,7 +351,7 @@ fn alloc_prof_free_handler(heap_live_enabled: bool) -> zend::VmMmCustomFreeFn { #[cfg(not(php_debug))] unsafe extern "C" fn alloc_prof_free_noop(ptr: *mut c_void) { - tls_zend_mm_state_get!(free)(ptr); + alloc_prof_forward_free(ptr); } #[cfg(php_debug)] @@ -386,7 +362,7 @@ unsafe extern "C" fn alloc_prof_free_noop( _orig_file: *const c_char, _orig_line: c_uint, ) { - tls_zend_mm_state_get!(free)(ptr); + alloc_prof_forward_free(ptr); } #[inline(always)] @@ -395,30 +371,22 @@ unsafe fn alloc_prof_free_impl(ptr: *mut c_void) { if !ptr.is_null() { untrack_allocation(ptr); } - tls_zend_mm_state_get!(free)(ptr); + alloc_prof_forward_free(ptr); } -unsafe fn alloc_prof_prev_free(ptr: *mut c_void) { - // Safety: `ZEND_MM_STATE.prev_custom_mm_free` will be initialised in - // `alloc_prof_rinit()` and only point to this function when - // `prev_custom_mm_free` is also initialised. - // Note: We use `.unwrap()` instead of `.unwrap_unchecked()` here because a - // neighboring extension could misbehave. If that happens, we want a proper - // panic with backtrace for debugging rather than undefined behavior. - let free = tls_zend_mm_state_get!(prev_custom_mm_free).unwrap(); - #[cfg(php_debug)] - { - free(ptr, core::ptr::null(), 0, core::ptr::null(), 0) +#[inline(always)] +unsafe fn alloc_prof_forward_free(ptr: *mut c_void) { + let state = tls_zend_mm_state_copy!(); + if let Some(free) = state.prev_custom_mm_free { + #[cfg(php_debug)] + return free(ptr, core::ptr::null(), 0, core::ptr::null(), 0); + #[cfg(not(php_debug))] + return free(ptr); } - #[cfg(not(php_debug))] - free(ptr) -} -unsafe fn alloc_prof_orig_free(ptr: *mut c_void) { - // Safety: `ZEND_MM_STATE.heap` will be initialised in `alloc_prof_rinit()` and custom ZendMM - // handlers only point to this function after successful init. Using `unwrap_unchecked()` is - // safe here as we have full control over ZendMM with no neighboring extensions. - let heap = tls_zend_mm_state_get!(heap).unwrap_unchecked(); + // SAFETY: this callback is only invoked after rinit stores the heap and + // before rshutdown clears it. + let heap = state.heap.unwrap_unchecked(); #[cfg(php_debug)] return zend::_zend_mm_free(heap, ptr, core::ptr::null(), 0, core::ptr::null(), 0); #[cfg(not(php_debug))] diff --git a/profiling/src/allocation/mod.rs b/profiling/src/allocation/mod.rs index 20f4d2280a..febaf12f79 100644 --- a/profiling/src/allocation/mod.rs +++ b/profiling/src/allocation/mod.rs @@ -275,6 +275,7 @@ fn initialization_panic() -> ! { panic!("Allocation profiler was not initialized properly. Please fill an issue stating the PHP version and the backtrace from this panic."); } +#[cfg(not(php_zend_mm_set_custom_handlers_ex))] unsafe fn alloc_prof_panic_alloc(_len: size_t) -> *mut c_void { initialization_panic(); } @@ -283,6 +284,7 @@ unsafe fn alloc_prof_panic_realloc(_prev_ptr: *mut c_void, _len: size_t) -> *mut initialization_panic(); } +#[cfg(not(php_zend_mm_set_custom_handlers_ex))] unsafe fn alloc_prof_panic_free(_ptr: *mut c_void) { initialization_panic(); } From 4147a6bd12ccf11452e296aecb1b623db34cf803 Mon Sep 17 00:00:00 2001 From: Florian Engelhardt Date: Mon, 27 Jul 2026 19:57:32 +0200 Subject: [PATCH 03/25] perf(profiling): optimize legacy allocator forwarding Apply the single-state allocation and free forwarding path to PHP 8.3 and older. Preserve the required ZendMM prepare/restore calls and neighboring custom allocator support while removing the intermediate function pointers and wrappers. On ZTS this also avoids repeated TSRM lookups. Across six balanced 60-second runs per binary on PHP 8.3 ZTS, mean allocation throughput increased from 28,079,883/s to 32,661,234/s (+16.32%) and median throughput increased by 16.65%. A full 60-second native sample reduced known forwarding-path self time from 16.31% to 12.30% (-24.6% relative). Validation: PHP 8.3 ZTS cargo test (22 passed); allocation sampling-distance, memory-peak, and GC PHPTs passed; PHP 8.5 release build and cargo test (22 passed). https://datadoghq.atlassian.net/browse/PROF-15506 --- profiling/src/allocation/allocation_le83.rs | 64 +++++++-------------- profiling/src/allocation/mod.rs | 10 ---- 2 files changed, 22 insertions(+), 52 deletions(-) diff --git a/profiling/src/allocation/allocation_le83.rs b/profiling/src/allocation/allocation_le83.rs index 12a1b37c58..88f61262e7 100644 --- a/profiling/src/allocation/allocation_le83.rs +++ b/profiling/src/allocation/allocation_le83.rs @@ -36,20 +36,10 @@ pub struct ZendMMState { prev_custom_mm_free: Option, prepare_restore_zend_heap: (ZendHeapPrepareFn, ZendHeapRestoreFn), /// Safety: this function pointer is only allowed to point to - /// `alloc_prof_prev_alloc()` when at the same time the - /// `ZEND_MM_STATE.prev_custom_mm_alloc` is initialised to a valid function - /// pointer, otherwise there will be dragons. - alloc: unsafe fn(size_t) -> *mut c_void, - /// Safety: this function pointer is only allowed to point to /// `alloc_prof_prev_realloc()` when at the same time the /// `ZEND_MM_STATE.prev_custom_mm_realloc` is initialised to a valid /// function pointer, otherwise there will be dragons. realloc: unsafe fn(*mut c_void, size_t) -> *mut c_void, - /// Safety: this function pointer is only allowed to point to - /// `alloc_prof_prev_free()` when at the same time the - /// `ZEND_MM_STATE.prev_custom_mm_free` is initialised to a valid function - /// pointer, otherwise there will be dragons. - free: unsafe fn(*mut c_void), } impl ZendMMState { @@ -61,9 +51,7 @@ impl ZendMMState { prev_custom_mm_realloc: None, prev_custom_mm_free: None, prepare_restore_zend_heap: (prepare_zend_heap, restore_zend_heap), - alloc: super::alloc_prof_panic_alloc, realloc: super::alloc_prof_panic_realloc, - free: super::alloc_prof_panic_free, } } } @@ -108,14 +96,10 @@ pub fn alloc_prof_rinit(heap_live_enabled: bool) { ptr::addr_of_mut!(zend_mm_state.prev_custom_mm_realloc), ); } - zend_mm_state.alloc = alloc_prof_prev_alloc; - zend_mm_state.free = alloc_prof_prev_free; zend_mm_state.realloc = alloc_prof_prev_realloc; zend_mm_state.prepare_restore_zend_heap = (prepare_zend_heap_none, restore_zend_heap_none); } else { - zend_mm_state.alloc = alloc_prof_orig_alloc; - zend_mm_state.free = alloc_prof_orig_free; zend_mm_state.realloc = alloc_prof_orig_realloc; zend_mm_state.prepare_restore_zend_heap = (prepare_zend_heap, restore_zend_heap); @@ -297,7 +281,7 @@ unsafe extern "C" fn alloc_prof_malloc(len: size_t) -> *mut c_void { #[cfg(feature = "debug_stats")] ALLOCATION_PROFILING_SIZE.fetch_add(len as u64, Relaxed); - let ptr = tls_zend_mm_state_get!(alloc)(len); + let ptr = alloc_prof_forward_alloc(len); // during startup, minit, rinit, ... current_execute_data is null // we are only interested in allocations during userland operations @@ -312,19 +296,17 @@ unsafe extern "C" fn alloc_prof_malloc(len: size_t) -> *mut c_void { ptr } -unsafe fn alloc_prof_prev_alloc(len: size_t) -> *mut c_void { - // Safety: `ZEND_MM_STATE.prev_custom_mm_alloc` will be initialised in - // `alloc_prof_rinit()` and only point to this function when - // `prev_custom_mm_alloc` is also initialised - let alloc = tls_zend_mm_state_get!(prev_custom_mm_alloc).unwrap(); - alloc(len) -} +#[inline(always)] +unsafe fn alloc_prof_forward_alloc(len: size_t) -> *mut c_void { + let state = tls_zend_mm_state_copy!(); + if let Some(alloc) = state.prev_custom_mm_alloc { + return alloc(len); + } -unsafe fn alloc_prof_orig_alloc(len: size_t) -> *mut c_void { - // Safety: `ZEND_MM_STATE.heap` will be initialised in `alloc_prof_rinit()` and custom ZendMM - // handlers are only installed and pointing to this function if initialization was succesful. - let heap = tls_zend_mm_state_get!(heap).unwrap_unchecked(); - let (prepare, restore) = tls_zend_mm_state_get!(prepare_restore_zend_heap); + // SAFETY: this callback is only invoked after rinit stores the heap and + // before rshutdown clears it. + let heap = state.heap.unwrap_unchecked(); + let (prepare, restore) = state.prepare_restore_zend_heap; let custom_heap = prepare(heap); #[cfg(php_debug)] let ptr: *mut c_void = zend::_zend_mm_alloc(heap, len, ptr::null(), 0, ptr::null(), 0); @@ -344,7 +326,7 @@ unsafe extern "C" fn alloc_prof_free(ptr: *mut c_void) { untrack_allocation(ptr); } - tls_zend_mm_state_get!(free)(ptr); + alloc_prof_forward_free(ptr); } fn alloc_prof_free_handler(heap_live_enabled: bool) -> zend::VmMmCustomFreeFn { @@ -356,21 +338,19 @@ fn alloc_prof_free_handler(heap_live_enabled: bool) -> zend::VmMmCustomFreeFn { } unsafe extern "C" fn alloc_prof_free_noop(ptr: *mut c_void) { - tls_zend_mm_state_get!(free)(ptr); + alloc_prof_forward_free(ptr); } -unsafe fn alloc_prof_prev_free(ptr: *mut c_void) { - // Safety: `ZEND_MM_STATE.prev_custom_mm_free` will be initialised in - // `alloc_prof_rinit()` and only point to this function when - // `prev_custom_mm_free` is also initialised - let free = tls_zend_mm_state_get!(prev_custom_mm_free).unwrap(); - free(ptr) -} +#[inline(always)] +unsafe fn alloc_prof_forward_free(ptr: *mut c_void) { + let state = tls_zend_mm_state_copy!(); + if let Some(free) = state.prev_custom_mm_free { + return free(ptr); + } -unsafe fn alloc_prof_orig_free(ptr: *mut c_void) { - // Safety: `ZEND_MM_STATE.heap` will be initialised in `alloc_prof_rinit()` and custom ZendMM - // handlers are only installed and pointing to this function if initialization was succesful. - let heap = tls_zend_mm_state_get!(heap).unwrap_unchecked(); + // SAFETY: this callback is only invoked after rinit stores the heap and + // before rshutdown clears it. + let heap = state.heap.unwrap_unchecked(); #[cfg(php_debug)] zend::_zend_mm_free(heap, ptr, core::ptr::null(), 0, core::ptr::null(), 0); #[cfg(not(php_debug))] diff --git a/profiling/src/allocation/mod.rs b/profiling/src/allocation/mod.rs index febaf12f79..32124f64c9 100644 --- a/profiling/src/allocation/mod.rs +++ b/profiling/src/allocation/mod.rs @@ -275,16 +275,6 @@ fn initialization_panic() -> ! { panic!("Allocation profiler was not initialized properly. Please fill an issue stating the PHP version and the backtrace from this panic."); } -#[cfg(not(php_zend_mm_set_custom_handlers_ex))] -unsafe fn alloc_prof_panic_alloc(_len: size_t) -> *mut c_void { - initialization_panic(); -} - unsafe fn alloc_prof_panic_realloc(_prev_ptr: *mut c_void, _len: size_t) -> *mut c_void { initialization_panic(); } - -#[cfg(not(php_zend_mm_set_custom_handlers_ex))] -unsafe fn alloc_prof_panic_free(_ptr: *mut c_void) { - initialization_panic(); -} From ddf62a8b4ffc1b8f6dbc97db5161888e706595cd Mon Sep 17 00:00:00 2001 From: Florian Engelhardt Date: Mon, 27 Jul 2026 20:35:12 +0200 Subject: [PATCH 04/25] perf(profiling): reuse TSRM cache in allocation hook Reuse one ZTS local-storage cache pointer to access both profiler globals and executor globals in the PHP 8.3-and-older allocation callback. This removes one TSRM lookup and the C current_execute_data wrapper per allocation while preserving the post-allocation executor-global read. Across six balanced 60-second runs per binary on PHP 8.3 ZTS, mean allocation throughput increased from 32,677,620/s to 34,432,538/s (+5.37%) and median throughput increased by 5.54%. Full native samples reduced selected TLS and wrapper self time from 17.74% to 12.95% (-27.0% relative). Validation: PHP 8.3 ZTS cargo test (22 passed); allocation sampling-distance, memory-peak, and GC PHPTs passed; PHP 8.5 release build and cargo test (22 passed). https://datadoghq.atlassian.net/browse/PROF-15506 --- profiling/src/allocation/allocation_le83.rs | 22 ++++++++++++--- profiling/src/allocation/mod.rs | 19 +++++++++++++ profiling/src/module_globals.rs | 30 ++++++++++++++++----- 3 files changed, 60 insertions(+), 11 deletions(-) diff --git a/profiling/src/allocation/allocation_le83.rs b/profiling/src/allocation/allocation_le83.rs index 88f61262e7..b3e046920b 100644 --- a/profiling/src/allocation/allocation_le83.rs +++ b/profiling/src/allocation/allocation_le83.rs @@ -2,10 +2,14 @@ use crate::allocation::{ allocation_profiling_stats_should_collect, collect_allocation, current_execute_data, untrack_allocation, }; +#[cfg(php_zts)] +use crate::allocation::{current_execute_data_from_cache, get_zend_mm_state_from_cache}; use crate::bindings::{ self as zend, datadog_php_install_handler, datadog_php_zif_handler, ddog_php_prof_copy_long_into_zval, }; +#[cfg(php_zts)] +use crate::module_globals; use crate::{RefCellExt, PROFILER_NAME, REQUEST_LOCALS}; use core::ptr; use libc::{c_char, c_int, c_void, size_t}; @@ -281,11 +285,22 @@ unsafe extern "C" fn alloc_prof_malloc(len: size_t) -> *mut c_void { #[cfg(feature = "debug_stats")] ALLOCATION_PROFILING_SIZE.fetch_add(len as u64, Relaxed); - let ptr = alloc_prof_forward_alloc(len); + #[cfg(php_zts)] + let ls_cache = module_globals::get_tsrm_ls_cache(); + #[cfg(php_zts)] + let state = (*get_zend_mm_state_from_cache(ls_cache)).get(); + #[cfg(not(php_zts))] + let state = tls_zend_mm_state_copy!(); + + let ptr = alloc_prof_forward_alloc(state, len); // during startup, minit, rinit, ... current_execute_data is null // we are only interested in allocations during userland operations - if current_execute_data().is_null() { + #[cfg(php_zts)] + let execute_data = current_execute_data_from_cache(ls_cache); + #[cfg(not(php_zts))] + let execute_data = current_execute_data(); + if execute_data.is_null() { return ptr; } @@ -297,8 +312,7 @@ unsafe extern "C" fn alloc_prof_malloc(len: size_t) -> *mut c_void { } #[inline(always)] -unsafe fn alloc_prof_forward_alloc(len: size_t) -> *mut c_void { - let state = tls_zend_mm_state_copy!(); +unsafe fn alloc_prof_forward_alloc(state: ZendMMState, len: size_t) -> *mut c_void { if let Some(alloc) = state.prev_custom_mm_alloc { return alloc(len); } diff --git a/profiling/src/allocation/mod.rs b/profiling/src/allocation/mod.rs index 32124f64c9..00ced142e6 100644 --- a/profiling/src/allocation/mod.rs +++ b/profiling/src/allocation/mod.rs @@ -39,6 +39,25 @@ pub(crate) unsafe fn get_zend_mm_state() -> *mut Cell { ptr::addr_of_mut!((*globals).zend_mm_state) } +#[cfg(php_zts)] +#[inline] +pub(crate) unsafe fn get_zend_mm_state_from_cache(ls_cache: *mut c_void) -> *mut Cell { + let globals = module_globals::get_profiler_globals_from_cache(ls_cache); + ptr::addr_of_mut!((*globals).zend_mm_state) +} + +#[cfg(php_zts)] +#[inline(always)] +pub(crate) unsafe fn current_execute_data_from_cache( + ls_cache: *mut c_void, +) -> *mut zend::zend_execute_data { + let offset = ptr::addr_of!(zend::executor_globals_offset).read(); + let globals = ls_cache + .byte_add(offset) + .cast::(); + ptr::addr_of!((*globals).current_execute_data).read() +} + #[inline(always)] pub(crate) unsafe fn current_execute_data() -> *mut zend::zend_execute_data { #[cfg(not(php_zts))] diff --git a/profiling/src/module_globals.rs b/profiling/src/module_globals.rs index 85c938e535..a57b041817 100644 --- a/profiling/src/module_globals.rs +++ b/profiling/src/module_globals.rs @@ -40,9 +40,13 @@ mod zts { } #[inline] - pub unsafe fn tsrmg_bulk(id: i32) -> *mut c_void { - let tls = tsrm_get_ls_cache() as *mut *mut *mut c_void; - let storage = *tls; // void** storage + pub unsafe fn get_ls_cache() -> *mut c_void { + tsrm_get_ls_cache() + } + + #[inline] + pub unsafe fn tsrmg_bulk(ls_cache: *mut c_void, id: i32) -> *mut c_void { + let storage = *(ls_cache as *mut *mut *mut c_void); // void** storage // TSRM_UNSHUFFLE_RSRC_ID(id) is just `id - 1`. let idx = (id - 1) as usize; @@ -51,6 +55,21 @@ mod zts { } } +#[cfg(php_zts)] +#[inline] +pub unsafe fn get_tsrm_ls_cache() -> *mut c_void { + zts::get_ls_cache() +} + +#[cfg(php_zts)] +#[inline] +pub unsafe fn get_profiler_globals_from_cache(ls_cache: *mut c_void) -> *mut ProfilerGlobals { + // SAFETY: As long as this is called during the times documented by + // get_profiler_globals(), GLOBALS_ID will be set by PHP. + let id = ptr::addr_of!(GLOBALS_ID).read(); + zts::tsrmg_bulk(ls_cache, id).cast() +} + /// Returns a pointer to the profiler globals for the current thread. /// /// # Safety @@ -64,10 +83,7 @@ mod zts { pub unsafe fn get_profiler_globals() -> *mut ProfilerGlobals { #[cfg(php_zts)] { - // SAFETY: As long as this is called during the times documented by - // our own safety requirements, GLOBALS_ID will be set by PHP. - let id = ptr::addr_of!(GLOBALS_ID).read(); - zts::tsrmg_bulk(id).cast() + get_profiler_globals_from_cache(get_tsrm_ls_cache()) } #[cfg(not(php_zts))] From a4aafc9ca7afabc779af64e8243edfae94010f6a Mon Sep 17 00:00:00 2001 From: Florian Engelhardt Date: Mon, 27 Jul 2026 20:42:32 +0200 Subject: [PATCH 05/25] test(profiling): stub ZendMM free for unit tests The free-handler selection test retains allocation callbacks that now call _zend_mm_free directly, but Rust unit-test binaries are not loaded by PHP. Provide a test-only stub with release and debug PHP signatures so both ZendMM API implementations link. Verified with cargo test on PHP 8.5 NTS and PHP 8.3 ZTS; both pass 22 tests. --- profiling/src/allocation/mod.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/profiling/src/allocation/mod.rs b/profiling/src/allocation/mod.rs index 32124f64c9..94e4b8eb0c 100644 --- a/profiling/src/allocation/mod.rs +++ b/profiling/src/allocation/mod.rs @@ -80,6 +80,23 @@ pub mod allocation_ge84; #[cfg(not(php_zend_mm_set_custom_handlers_ex))] pub mod allocation_le83; +// Handler-selection tests retain the free callbacks in a binary that is not loaded by PHP. +#[cfg(all(test, not(php_debug)))] +#[no_mangle] +unsafe extern "C" fn _zend_mm_free(_heap: *mut zend::_zend_mm_heap, _ptr: *mut c_void) {} + +#[cfg(all(test, php_debug))] +#[no_mangle] +unsafe extern "C" fn _zend_mm_free( + _heap: *mut zend::_zend_mm_heap, + _ptr: *mut c_void, + _file: *const libc::c_char, + _line: libc::c_uint, + _orig_file: *const libc::c_char, + _orig_line: libc::c_uint, +) { +} + /// Default sampling interval in bytes (4 MiB). pub const DEFAULT_ALLOCATION_SAMPLING_INTERVAL: NonZeroU32 = NonZero::new(1024 * 4096).unwrap(); From 7adc7a3c84b32f306397e774cebccd93d388057c Mon Sep 17 00:00:00 2001 From: Florian Engelhardt Date: Mon, 27 Jul 2026 20:42:32 +0200 Subject: [PATCH 06/25] test(profiling): stub ZendMM free for unit tests The free-handler selection test retains allocation callbacks that now call _zend_mm_free directly, but Rust unit-test binaries are not loaded by PHP. Provide a test-only stub with release and debug PHP signatures so both ZendMM API implementations link. Verified with cargo test on PHP 8.5 NTS and PHP 8.3 ZTS; both pass 22 tests. --- profiling/src/allocation/mod.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/profiling/src/allocation/mod.rs b/profiling/src/allocation/mod.rs index 00ced142e6..e22adef80d 100644 --- a/profiling/src/allocation/mod.rs +++ b/profiling/src/allocation/mod.rs @@ -99,6 +99,23 @@ pub mod allocation_ge84; #[cfg(not(php_zend_mm_set_custom_handlers_ex))] pub mod allocation_le83; +// Handler-selection tests retain the free callbacks in a binary that is not loaded by PHP. +#[cfg(all(test, not(php_debug)))] +#[no_mangle] +unsafe extern "C" fn _zend_mm_free(_heap: *mut zend::_zend_mm_heap, _ptr: *mut c_void) {} + +#[cfg(all(test, php_debug))] +#[no_mangle] +unsafe extern "C" fn _zend_mm_free( + _heap: *mut zend::_zend_mm_heap, + _ptr: *mut c_void, + _file: *const libc::c_char, + _line: libc::c_uint, + _orig_file: *const libc::c_char, + _orig_line: libc::c_uint, +) { +} + /// Default sampling interval in bytes (4 MiB). pub const DEFAULT_ALLOCATION_SAMPLING_INTERVAL: NonZeroU32 = NonZero::new(1024 * 4096).unwrap(); From a30b72ddd777b428b800f33a81dbe9dae6540007 Mon Sep 17 00:00:00 2001 From: Florian Engelhardt Date: Tue, 28 Jul 2026 07:37:41 +0200 Subject: [PATCH 07/25] perf(profiling): remove indirect realloc forwarding Forward reallocations from one ZendMMState read instead of loading a function pointer, making an indirect Rust call, and reloading state. Preserve neighboring custom allocator support and the PHP 8.3-and-older heap prepare/restore calls. Remove the realloc forwarding field, intermediate wrappers, and initialization panic callback from both ZendMM implementations. Validation: PHP 8.5 NTS and PHP 8.3 ZTS cargo test (22 passed each); five allocation PHPTs passed on both builds. https://datadoghq.atlassian.net/browse/PROF-15506 --- profiling/src/allocation/allocation_ge84.rs | 42 +++++++-------------- profiling/src/allocation/allocation_le83.rs | 34 ++++++----------- profiling/src/allocation/mod.rs | 5 +-- 3 files changed, 26 insertions(+), 55 deletions(-) diff --git a/profiling/src/allocation/allocation_ge84.rs b/profiling/src/allocation/allocation_ge84.rs index f19ed81d88..be7273559a 100644 --- a/profiling/src/allocation/allocation_ge84.rs +++ b/profiling/src/allocation/allocation_ge84.rs @@ -34,11 +34,6 @@ pub struct ZendMMState { /// The engine's previous custom shutdown function, if there is one. prev_custom_mm_shutdown: Option, /// Safety: this function pointer is only allowed to point to - /// `alloc_prof_prev_realloc()` when at the same time the - /// `ZEND_MM_STATE.prev_custom_mm_realloc` is initialised to a valid - /// function pointer, otherwise there will be dragons. - realloc: unsafe fn(*mut c_void, size_t) -> *mut c_void, - /// Safety: this function pointer is only allowed to point to /// `alloc_prof_prev_gc()` when at the same time the /// `ZEND_MM_STATE.prev_custom_mm_gc` is initialised to a valid function /// pointer, otherwise there will be dragons. @@ -68,7 +63,6 @@ impl ZendMMState { prev_custom_mm_free: None, prev_custom_mm_gc: None, prev_custom_mm_shutdown: None, - realloc: super::alloc_prof_panic_realloc, gc: alloc_prof_panic_gc, shutdown: alloc_prof_panic_shutdown, } @@ -115,11 +109,9 @@ pub fn alloc_prof_rinit(heap_live_enabled: bool) { ptr::addr_of_mut!(zend_mm_state.prev_custom_mm_shutdown), ); } - zend_mm_state.realloc = alloc_prof_prev_realloc; zend_mm_state.gc = alloc_prof_prev_gc; zend_mm_state.shutdown = alloc_prof_prev_shutdown; } else { - zend_mm_state.realloc = alloc_prof_orig_realloc; zend_mm_state.gc = alloc_prof_orig_gc; zend_mm_state.shutdown = alloc_prof_orig_shutdown; @@ -445,7 +437,7 @@ unsafe fn alloc_prof_realloc_impl(prev_ptr: *mut c_void, len: size_t) -> *mut c_ #[cfg(feature = "debug_stats")] ALLOCATION_PROFILING_SIZE.fetch_add(len as u64, Relaxed); - let ptr = tls_zend_mm_state_get!(realloc)(prev_ptr, len); + let ptr = alloc_prof_forward_realloc(prev_ptr, len); // ZendMM allocation failures raise a fatal error and bail out instead of // returning NULL. If realloc returns, prev_ptr has been consumed: untrack it @@ -465,7 +457,7 @@ unsafe fn alloc_prof_realloc_no_untrack_impl(prev_ptr: *mut c_void, len: size_t) #[cfg(feature = "debug_stats")] ALLOCATION_PROFILING_SIZE.fetch_add(len as u64, Relaxed); - let ptr = tls_zend_mm_state_get!(realloc)(prev_ptr, len); + let ptr = alloc_prof_forward_realloc(prev_ptr, len); alloc_prof_realloc_sample(ptr, len) } @@ -489,27 +481,19 @@ unsafe fn alloc_prof_realloc_sample(ptr: *mut c_void, len: size_t) -> *mut c_voi ptr } -unsafe fn alloc_prof_prev_realloc(prev_ptr: *mut c_void, len: size_t) -> *mut c_void { - // Safety: `ZEND_MM_STATE.prev_custom_mm_realloc` will be initialised in - // `alloc_prof_rinit()` and only point to this function when - // `prev_custom_mm_realloc` is also initialised. - // Note: We use `.unwrap()` instead of `.unwrap_unchecked()` here because a - // neighboring extension could misbehave. If that happens, we want a proper - // panic with backtrace for debugging rather than undefined behavior. - let realloc = tls_zend_mm_state_get!(prev_custom_mm_realloc).unwrap(); - #[cfg(php_debug)] - { - realloc(prev_ptr, len, ptr::null(), 0, ptr::null(), 0) +#[inline(always)] +unsafe fn alloc_prof_forward_realloc(prev_ptr: *mut c_void, len: size_t) -> *mut c_void { + let state = tls_zend_mm_state_copy!(); + if let Some(realloc) = state.prev_custom_mm_realloc { + #[cfg(php_debug)] + return realloc(prev_ptr, len, ptr::null(), 0, ptr::null(), 0); + #[cfg(not(php_debug))] + return realloc(prev_ptr, len); } - #[cfg(not(php_debug))] - realloc(prev_ptr, len) -} -unsafe fn alloc_prof_orig_realloc(prev_ptr: *mut c_void, len: size_t) -> *mut c_void { - // Safety: `ZEND_MM_STATE.heap` will be initialised in `alloc_prof_rinit()` and custom ZendMM - // handlers only point to this function after successful init. Using `unwrap_unchecked()` is - // safe here as we have full control over ZendMM with no neighboring extensions. - let heap = tls_zend_mm_state_get!(heap).unwrap_unchecked(); + // SAFETY: this callback is only invoked after rinit stores the heap and + // before rshutdown clears it. + let heap = state.heap.unwrap_unchecked(); #[cfg(php_debug)] return zend::_zend_mm_realloc(heap, prev_ptr, len, ptr::null(), 0, ptr::null(), 0); #[cfg(not(php_debug))] diff --git a/profiling/src/allocation/allocation_le83.rs b/profiling/src/allocation/allocation_le83.rs index 88f61262e7..7a8b2fa430 100644 --- a/profiling/src/allocation/allocation_le83.rs +++ b/profiling/src/allocation/allocation_le83.rs @@ -35,11 +35,6 @@ pub struct ZendMMState { /// The engine's previous custom free function, if there is one. prev_custom_mm_free: Option, prepare_restore_zend_heap: (ZendHeapPrepareFn, ZendHeapRestoreFn), - /// Safety: this function pointer is only allowed to point to - /// `alloc_prof_prev_realloc()` when at the same time the - /// `ZEND_MM_STATE.prev_custom_mm_realloc` is initialised to a valid - /// function pointer, otherwise there will be dragons. - realloc: unsafe fn(*mut c_void, size_t) -> *mut c_void, } impl ZendMMState { @@ -51,7 +46,6 @@ impl ZendMMState { prev_custom_mm_realloc: None, prev_custom_mm_free: None, prepare_restore_zend_heap: (prepare_zend_heap, restore_zend_heap), - realloc: super::alloc_prof_panic_realloc, } } } @@ -96,11 +90,9 @@ pub fn alloc_prof_rinit(heap_live_enabled: bool) { ptr::addr_of_mut!(zend_mm_state.prev_custom_mm_realloc), ); } - zend_mm_state.realloc = alloc_prof_prev_realloc; zend_mm_state.prepare_restore_zend_heap = (prepare_zend_heap_none, restore_zend_heap_none); } else { - zend_mm_state.realloc = alloc_prof_orig_realloc; zend_mm_state.prepare_restore_zend_heap = (prepare_zend_heap, restore_zend_heap); // Reset previous handlers to None. There might be a chaotic neighbor that @@ -383,7 +375,7 @@ unsafe fn alloc_prof_realloc_impl(prev_ptr: *mut c_void, len: size_t) -> *mut c_ #[cfg(feature = "debug_stats")] ALLOCATION_PROFILING_SIZE.fetch_add(len as u64, Relaxed); - let ptr = tls_zend_mm_state_get!(realloc)(prev_ptr, len); + let ptr = alloc_prof_forward_realloc(prev_ptr, len); // ZendMM allocation failures raise a fatal error and bail out instead of // returning NULL. If realloc returns, prev_ptr has been consumed: untrack it @@ -403,7 +395,7 @@ unsafe fn alloc_prof_realloc_no_untrack_impl(prev_ptr: *mut c_void, len: size_t) #[cfg(feature = "debug_stats")] ALLOCATION_PROFILING_SIZE.fetch_add(len as u64, Relaxed); - let ptr = tls_zend_mm_state_get!(realloc)(prev_ptr, len); + let ptr = alloc_prof_forward_realloc(prev_ptr, len); alloc_prof_realloc_sample(ptr, len) } @@ -427,19 +419,17 @@ unsafe fn alloc_prof_realloc_sample(ptr: *mut c_void, len: size_t) -> *mut c_voi ptr } -unsafe fn alloc_prof_prev_realloc(prev_ptr: *mut c_void, len: size_t) -> *mut c_void { - // Safety: `ZEND_MM_STATE.prev_custom_mm_realloc` will be initialised in - // `alloc_prof_rinit()` and only point to this function when - // `prev_custom_mm_realloc` is also initialised - let realloc = tls_zend_mm_state_get!(prev_custom_mm_realloc).unwrap(); - realloc(prev_ptr, len) -} +#[inline(always)] +unsafe fn alloc_prof_forward_realloc(prev_ptr: *mut c_void, len: size_t) -> *mut c_void { + let state = tls_zend_mm_state_copy!(); + if let Some(realloc) = state.prev_custom_mm_realloc { + return realloc(prev_ptr, len); + } -unsafe fn alloc_prof_orig_realloc(prev_ptr: *mut c_void, len: size_t) -> *mut c_void { - // Safety: `ZEND_MM_STATE.heap` will be initialised in `alloc_prof_rinit()` and custom ZendMM - // handlers are only installed and pointing to this function if initialization was succesful. - let heap = tls_zend_mm_state_get!(heap).unwrap_unchecked(); - let (prepare, restore) = tls_zend_mm_state_get!(prepare_restore_zend_heap); + // SAFETY: this callback is only invoked after rinit stores the heap and + // before rshutdown clears it. + let heap = state.heap.unwrap_unchecked(); + let (prepare, restore) = state.prepare_restore_zend_heap; let custom_heap = prepare(heap); #[cfg(php_debug)] let ptr: *mut c_void = diff --git a/profiling/src/allocation/mod.rs b/profiling/src/allocation/mod.rs index 94e4b8eb0c..30241604c2 100644 --- a/profiling/src/allocation/mod.rs +++ b/profiling/src/allocation/mod.rs @@ -287,11 +287,8 @@ pub fn alloc_prof_rshutdown() { allocation_ge84::alloc_prof_rshutdown(heap_live_enabled); } +#[cfg(php_zend_mm_set_custom_handlers_ex)] #[track_caller] fn initialization_panic() -> ! { panic!("Allocation profiler was not initialized properly. Please fill an issue stating the PHP version and the backtrace from this panic."); } - -unsafe fn alloc_prof_panic_realloc(_prev_ptr: *mut c_void, _len: size_t) -> *mut c_void { - initialization_panic(); -} From 43e6473757dc5d5cbe0387de46c5522a8c69c2aa Mon Sep 17 00:00:00 2001 From: Florian Engelhardt Date: Tue, 28 Jul 2026 07:37:41 +0200 Subject: [PATCH 08/25] perf(profiling): remove indirect realloc forwarding Forward reallocations from one ZendMMState read instead of loading a function pointer, making an indirect Rust call, and reloading state. Preserve neighboring custom allocator support and the PHP 8.3-and-older heap prepare/restore calls. Remove the realloc forwarding field, intermediate wrappers, and initialization panic callback from both ZendMM implementations. Validation: PHP 8.5 NTS and PHP 8.3 ZTS cargo test (22 passed each); five allocation PHPTs passed on both builds. https://datadoghq.atlassian.net/browse/PROF-15506 --- profiling/src/allocation/allocation_ge84.rs | 42 +++++++-------------- profiling/src/allocation/allocation_le83.rs | 34 ++++++----------- profiling/src/allocation/mod.rs | 5 +-- 3 files changed, 26 insertions(+), 55 deletions(-) diff --git a/profiling/src/allocation/allocation_ge84.rs b/profiling/src/allocation/allocation_ge84.rs index f19ed81d88..be7273559a 100644 --- a/profiling/src/allocation/allocation_ge84.rs +++ b/profiling/src/allocation/allocation_ge84.rs @@ -34,11 +34,6 @@ pub struct ZendMMState { /// The engine's previous custom shutdown function, if there is one. prev_custom_mm_shutdown: Option, /// Safety: this function pointer is only allowed to point to - /// `alloc_prof_prev_realloc()` when at the same time the - /// `ZEND_MM_STATE.prev_custom_mm_realloc` is initialised to a valid - /// function pointer, otherwise there will be dragons. - realloc: unsafe fn(*mut c_void, size_t) -> *mut c_void, - /// Safety: this function pointer is only allowed to point to /// `alloc_prof_prev_gc()` when at the same time the /// `ZEND_MM_STATE.prev_custom_mm_gc` is initialised to a valid function /// pointer, otherwise there will be dragons. @@ -68,7 +63,6 @@ impl ZendMMState { prev_custom_mm_free: None, prev_custom_mm_gc: None, prev_custom_mm_shutdown: None, - realloc: super::alloc_prof_panic_realloc, gc: alloc_prof_panic_gc, shutdown: alloc_prof_panic_shutdown, } @@ -115,11 +109,9 @@ pub fn alloc_prof_rinit(heap_live_enabled: bool) { ptr::addr_of_mut!(zend_mm_state.prev_custom_mm_shutdown), ); } - zend_mm_state.realloc = alloc_prof_prev_realloc; zend_mm_state.gc = alloc_prof_prev_gc; zend_mm_state.shutdown = alloc_prof_prev_shutdown; } else { - zend_mm_state.realloc = alloc_prof_orig_realloc; zend_mm_state.gc = alloc_prof_orig_gc; zend_mm_state.shutdown = alloc_prof_orig_shutdown; @@ -445,7 +437,7 @@ unsafe fn alloc_prof_realloc_impl(prev_ptr: *mut c_void, len: size_t) -> *mut c_ #[cfg(feature = "debug_stats")] ALLOCATION_PROFILING_SIZE.fetch_add(len as u64, Relaxed); - let ptr = tls_zend_mm_state_get!(realloc)(prev_ptr, len); + let ptr = alloc_prof_forward_realloc(prev_ptr, len); // ZendMM allocation failures raise a fatal error and bail out instead of // returning NULL. If realloc returns, prev_ptr has been consumed: untrack it @@ -465,7 +457,7 @@ unsafe fn alloc_prof_realloc_no_untrack_impl(prev_ptr: *mut c_void, len: size_t) #[cfg(feature = "debug_stats")] ALLOCATION_PROFILING_SIZE.fetch_add(len as u64, Relaxed); - let ptr = tls_zend_mm_state_get!(realloc)(prev_ptr, len); + let ptr = alloc_prof_forward_realloc(prev_ptr, len); alloc_prof_realloc_sample(ptr, len) } @@ -489,27 +481,19 @@ unsafe fn alloc_prof_realloc_sample(ptr: *mut c_void, len: size_t) -> *mut c_voi ptr } -unsafe fn alloc_prof_prev_realloc(prev_ptr: *mut c_void, len: size_t) -> *mut c_void { - // Safety: `ZEND_MM_STATE.prev_custom_mm_realloc` will be initialised in - // `alloc_prof_rinit()` and only point to this function when - // `prev_custom_mm_realloc` is also initialised. - // Note: We use `.unwrap()` instead of `.unwrap_unchecked()` here because a - // neighboring extension could misbehave. If that happens, we want a proper - // panic with backtrace for debugging rather than undefined behavior. - let realloc = tls_zend_mm_state_get!(prev_custom_mm_realloc).unwrap(); - #[cfg(php_debug)] - { - realloc(prev_ptr, len, ptr::null(), 0, ptr::null(), 0) +#[inline(always)] +unsafe fn alloc_prof_forward_realloc(prev_ptr: *mut c_void, len: size_t) -> *mut c_void { + let state = tls_zend_mm_state_copy!(); + if let Some(realloc) = state.prev_custom_mm_realloc { + #[cfg(php_debug)] + return realloc(prev_ptr, len, ptr::null(), 0, ptr::null(), 0); + #[cfg(not(php_debug))] + return realloc(prev_ptr, len); } - #[cfg(not(php_debug))] - realloc(prev_ptr, len) -} -unsafe fn alloc_prof_orig_realloc(prev_ptr: *mut c_void, len: size_t) -> *mut c_void { - // Safety: `ZEND_MM_STATE.heap` will be initialised in `alloc_prof_rinit()` and custom ZendMM - // handlers only point to this function after successful init. Using `unwrap_unchecked()` is - // safe here as we have full control over ZendMM with no neighboring extensions. - let heap = tls_zend_mm_state_get!(heap).unwrap_unchecked(); + // SAFETY: this callback is only invoked after rinit stores the heap and + // before rshutdown clears it. + let heap = state.heap.unwrap_unchecked(); #[cfg(php_debug)] return zend::_zend_mm_realloc(heap, prev_ptr, len, ptr::null(), 0, ptr::null(), 0); #[cfg(not(php_debug))] diff --git a/profiling/src/allocation/allocation_le83.rs b/profiling/src/allocation/allocation_le83.rs index b3e046920b..aa6005d653 100644 --- a/profiling/src/allocation/allocation_le83.rs +++ b/profiling/src/allocation/allocation_le83.rs @@ -39,11 +39,6 @@ pub struct ZendMMState { /// The engine's previous custom free function, if there is one. prev_custom_mm_free: Option, prepare_restore_zend_heap: (ZendHeapPrepareFn, ZendHeapRestoreFn), - /// Safety: this function pointer is only allowed to point to - /// `alloc_prof_prev_realloc()` when at the same time the - /// `ZEND_MM_STATE.prev_custom_mm_realloc` is initialised to a valid - /// function pointer, otherwise there will be dragons. - realloc: unsafe fn(*mut c_void, size_t) -> *mut c_void, } impl ZendMMState { @@ -55,7 +50,6 @@ impl ZendMMState { prev_custom_mm_realloc: None, prev_custom_mm_free: None, prepare_restore_zend_heap: (prepare_zend_heap, restore_zend_heap), - realloc: super::alloc_prof_panic_realloc, } } } @@ -100,11 +94,9 @@ pub fn alloc_prof_rinit(heap_live_enabled: bool) { ptr::addr_of_mut!(zend_mm_state.prev_custom_mm_realloc), ); } - zend_mm_state.realloc = alloc_prof_prev_realloc; zend_mm_state.prepare_restore_zend_heap = (prepare_zend_heap_none, restore_zend_heap_none); } else { - zend_mm_state.realloc = alloc_prof_orig_realloc; zend_mm_state.prepare_restore_zend_heap = (prepare_zend_heap, restore_zend_heap); // Reset previous handlers to None. There might be a chaotic neighbor that @@ -397,7 +389,7 @@ unsafe fn alloc_prof_realloc_impl(prev_ptr: *mut c_void, len: size_t) -> *mut c_ #[cfg(feature = "debug_stats")] ALLOCATION_PROFILING_SIZE.fetch_add(len as u64, Relaxed); - let ptr = tls_zend_mm_state_get!(realloc)(prev_ptr, len); + let ptr = alloc_prof_forward_realloc(prev_ptr, len); // ZendMM allocation failures raise a fatal error and bail out instead of // returning NULL. If realloc returns, prev_ptr has been consumed: untrack it @@ -417,7 +409,7 @@ unsafe fn alloc_prof_realloc_no_untrack_impl(prev_ptr: *mut c_void, len: size_t) #[cfg(feature = "debug_stats")] ALLOCATION_PROFILING_SIZE.fetch_add(len as u64, Relaxed); - let ptr = tls_zend_mm_state_get!(realloc)(prev_ptr, len); + let ptr = alloc_prof_forward_realloc(prev_ptr, len); alloc_prof_realloc_sample(ptr, len) } @@ -441,19 +433,17 @@ unsafe fn alloc_prof_realloc_sample(ptr: *mut c_void, len: size_t) -> *mut c_voi ptr } -unsafe fn alloc_prof_prev_realloc(prev_ptr: *mut c_void, len: size_t) -> *mut c_void { - // Safety: `ZEND_MM_STATE.prev_custom_mm_realloc` will be initialised in - // `alloc_prof_rinit()` and only point to this function when - // `prev_custom_mm_realloc` is also initialised - let realloc = tls_zend_mm_state_get!(prev_custom_mm_realloc).unwrap(); - realloc(prev_ptr, len) -} +#[inline(always)] +unsafe fn alloc_prof_forward_realloc(prev_ptr: *mut c_void, len: size_t) -> *mut c_void { + let state = tls_zend_mm_state_copy!(); + if let Some(realloc) = state.prev_custom_mm_realloc { + return realloc(prev_ptr, len); + } -unsafe fn alloc_prof_orig_realloc(prev_ptr: *mut c_void, len: size_t) -> *mut c_void { - // Safety: `ZEND_MM_STATE.heap` will be initialised in `alloc_prof_rinit()` and custom ZendMM - // handlers are only installed and pointing to this function if initialization was succesful. - let heap = tls_zend_mm_state_get!(heap).unwrap_unchecked(); - let (prepare, restore) = tls_zend_mm_state_get!(prepare_restore_zend_heap); + // SAFETY: this callback is only invoked after rinit stores the heap and + // before rshutdown clears it. + let heap = state.heap.unwrap_unchecked(); + let (prepare, restore) = state.prepare_restore_zend_heap; let custom_heap = prepare(heap); #[cfg(php_debug)] let ptr: *mut c_void = diff --git a/profiling/src/allocation/mod.rs b/profiling/src/allocation/mod.rs index e22adef80d..4b53d85326 100644 --- a/profiling/src/allocation/mod.rs +++ b/profiling/src/allocation/mod.rs @@ -306,11 +306,8 @@ pub fn alloc_prof_rshutdown() { allocation_ge84::alloc_prof_rshutdown(heap_live_enabled); } +#[cfg(php_zend_mm_set_custom_handlers_ex)] #[track_caller] fn initialization_panic() -> ! { panic!("Allocation profiler was not initialized properly. Please fill an issue stating the PHP version and the backtrace from this panic."); } - -unsafe fn alloc_prof_panic_realloc(_prev_ptr: *mut c_void, _len: size_t) -> *mut c_void { - initialization_panic(); -} From 496f6cafe1a0185436022f51c1dd22646b5fe1d2 Mon Sep 17 00:00:00 2001 From: Florian Engelhardt Date: Tue, 28 Jul 2026 08:15:21 +0200 Subject: [PATCH 09/25] fix(profiling): support legacy ZTS executor globals PHP 7.1 through 7.3 expose executor globals through executor_globals_id instead of the fast offset introduced in PHP 7.4. Reuse the cached TSRM storage through the resource-ID path on those versions and keep the offset path for PHP 7.4 and newer. Provide a test-only tsrm_get_ls_cache stub because Rust unit-test binaries are not loaded by PHP. Validation: PHP 7.3 ZTS cargo test (22 passed), release build, and allocation smoke test; PHP 8.3 ZTS and PHP 8.5 NTS cargo test (22 passed each). https://datadoghq.atlassian.net/browse/PROF-15506 --- profiling/build.rs | 5 ++++- profiling/src/allocation/mod.rs | 18 ++++++++++++++---- profiling/src/module_globals.rs | 14 +++++++++++++- 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/profiling/build.rs b/profiling/build.rs index 140662e699..cf88c39856 100644 --- a/profiling/build.rs +++ b/profiling/build.rs @@ -389,8 +389,11 @@ fn cfg_frameless(vernum: u64) -> bool { } fn cfg_php_feature_flags(vernum: u64) { - println!("cargo::rustc-check-cfg=cfg(php_gc_status, php_zend_compile_string_has_position, php_gc_status_extended, php_frameless, php_opcache_restart_hook, php_zend_mm_set_custom_handlers_ex)"); + println!("cargo::rustc-check-cfg=cfg(php_gc_status, php_zend_compile_string_has_position, php_gc_status_extended, php_frameless, php_opcache_restart_hook, php_zend_mm_set_custom_handlers_ex, php_zts_fast_globals)"); + if vernum >= 70400 { + println!("cargo:rustc-cfg=php_zts_fast_globals"); + } if vernum >= 70300 { println!("cargo:rustc-cfg=php_gc_status"); } diff --git a/profiling/src/allocation/mod.rs b/profiling/src/allocation/mod.rs index 4b53d85326..844c13857e 100644 --- a/profiling/src/allocation/mod.rs +++ b/profiling/src/allocation/mod.rs @@ -51,10 +51,20 @@ pub(crate) unsafe fn get_zend_mm_state_from_cache(ls_cache: *mut c_void) -> *mut pub(crate) unsafe fn current_execute_data_from_cache( ls_cache: *mut c_void, ) -> *mut zend::zend_execute_data { - let offset = ptr::addr_of!(zend::executor_globals_offset).read(); - let globals = ls_cache - .byte_add(offset) - .cast::(); + // PHP 7.4 introduced fast globals offsets. Older versions use the TSRM resource ID. + #[cfg(php_zts_fast_globals)] + let globals = { + let offset = ptr::addr_of!(zend::executor_globals_offset).read(); + ls_cache + .byte_add(offset) + .cast::() + }; + #[cfg(not(php_zts_fast_globals))] + let globals = { + let id = ptr::addr_of!(zend::executor_globals_id).read(); + module_globals::get_tsrm_resource_from_cache(ls_cache, id) + .cast::() + }; ptr::addr_of!((*globals).current_execute_data).read() } diff --git a/profiling/src/module_globals.rs b/profiling/src/module_globals.rs index a57b041817..ac1d01ce55 100644 --- a/profiling/src/module_globals.rs +++ b/profiling/src/module_globals.rs @@ -31,6 +31,12 @@ pub static mut GLOBALS: ProfilerGlobals = ProfilerGlobals { zend_mm_state: Cell::new(ZendMMState::new()), }; +#[cfg(all(test, php_zts))] +#[no_mangle] +unsafe extern "C" fn tsrm_get_ls_cache() -> *mut c_void { + ptr::null_mut() +} + #[cfg(php_zts)] mod zts { use core::ffi::c_void; @@ -61,13 +67,19 @@ pub unsafe fn get_tsrm_ls_cache() -> *mut c_void { zts::get_ls_cache() } +#[cfg(php_zts)] +#[inline] +pub unsafe fn get_tsrm_resource_from_cache(ls_cache: *mut c_void, id: i32) -> *mut c_void { + zts::tsrmg_bulk(ls_cache, id) +} + #[cfg(php_zts)] #[inline] pub unsafe fn get_profiler_globals_from_cache(ls_cache: *mut c_void) -> *mut ProfilerGlobals { // SAFETY: As long as this is called during the times documented by // get_profiler_globals(), GLOBALS_ID will be set by PHP. let id = ptr::addr_of!(GLOBALS_ID).read(); - zts::tsrmg_bulk(ls_cache, id).cast() + get_tsrm_resource_from_cache(ls_cache, id).cast() } /// Returns a pointer to the profiler globals for the current thread. From 02acd656e963354265362eeb6184cdd3ac6e7219 Mon Sep 17 00:00:00 2001 From: Florian Engelhardt Date: Tue, 28 Jul 2026 12:39:21 +0200 Subject: [PATCH 10/25] perf(profiling): skip idle internal interrupt handling On PHP 8.3 and older, the execute_internal hook called the full profiler interrupt handler after every internal function. Check EG(vm_interrupt) first so the TLS, request-state, and atomic work only runs when PHP has a pending VM interrupt. Across six balanced 60-second runs per binary on PHP 8.3 ZTS, mean allocation-loop throughput increased from 34,160,664/s to 36,061,991/s (+5.57%) and median throughput increased by 5.92%. Native samples reduced the manual interrupt path from 14.04% to 8.62% of main-thread samples. Wall-time correctness remained unchanged: a four-second CPU/sleep workload attributed 50.23% to sleep before and 50.31% after. Validation: PHP 7.3 ZTS cargo check; PHP 8.3 ZTS and PHP 8.5 NTS cargo test (22 passed each). https://datadoghq.atlassian.net/browse/PROF-15506 --- profiling/src/php_ffi.c | 8 ++++++++ profiling/src/php_ffi.h | 1 + profiling/src/wall_time.rs | 6 ++++-- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/profiling/src/php_ffi.c b/profiling/src/php_ffi.c index 1e906f3cbd..5799822ef0 100644 --- a/profiling/src/php_ffi.c +++ b/profiling/src/php_ffi.c @@ -248,6 +248,14 @@ zend_execute_data* ddog_php_prof_get_current_execute_data() { return EG(current_execute_data); } +bool ddog_php_prof_vm_interrupt_pending() { +#if PHP_VERSION_ID >= 80000 + return zend_atomic_bool_load_ex(&EG(vm_interrupt)); +#else + return EG(vm_interrupt); +#endif +} + #if CFG_FIBERS // defined by build.rs zend_fiber* ddog_php_prof_get_active_fiber() { diff --git a/profiling/src/php_ffi.h b/profiling/src/php_ffi.h index 558c3de441..1f402732ec 100644 --- a/profiling/src/php_ffi.h +++ b/profiling/src/php_ffi.h @@ -162,6 +162,7 @@ void ddog_php_prof_zend_mm_set_custom_handlers(zend_mm_heap *heap, ddog_php_prof_zend_mm_realloc _realloc); zend_execute_data* ddog_php_prof_get_current_execute_data(); +bool ddog_php_prof_vm_interrupt_pending(); #if CFG_FRAMELESS void ddog_php_prof_post_startup(); diff --git a/profiling/src/wall_time.rs b/profiling/src/wall_time.rs index 3f0a948127..7227fbcbdd 100644 --- a/profiling/src/wall_time.rs +++ b/profiling/src/wall_time.rs @@ -79,8 +79,10 @@ mod execute_internal { unsafe { prev_execute_internal(execute_data, return_value) }; // See safety section of `execute_data_func_is_trampoline` docs for why - // the leaf frame is used instead of the execute_data ptr. - ddog_php_prof_interrupt_function(leaf_frame); + // the leaf frame is used instead of the execute_data ptr. + if unsafe { zend::ddog_php_prof_vm_interrupt_pending() } { + ddog_php_prof_interrupt_function(leaf_frame); + } } /// # Safety From be256447ac02963392231381428c2b2500b7b2c0 Mon Sep 17 00:00:00 2001 From: Florian Engelhardt Date: Tue, 28 Jul 2026 13:22:13 +0200 Subject: [PATCH 11/25] fix(profiling): guard atomic interrupt load on PHP 8.2 --- profiling/src/php_ffi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/profiling/src/php_ffi.c b/profiling/src/php_ffi.c index 5799822ef0..be674ed216 100644 --- a/profiling/src/php_ffi.c +++ b/profiling/src/php_ffi.c @@ -249,7 +249,7 @@ zend_execute_data* ddog_php_prof_get_current_execute_data() { } bool ddog_php_prof_vm_interrupt_pending() { -#if PHP_VERSION_ID >= 80000 +#if PHP_VERSION_ID >= 80200 return zend_atomic_bool_load_ex(&EG(vm_interrupt)); #else return EG(vm_interrupt); From 4aaeba6e9dc5e2f52cec94e76fde05144116ef84 Mon Sep 17 00:00:00 2001 From: Florian Engelhardt Date: Tue, 28 Jul 2026 13:49:41 +0200 Subject: [PATCH 12/25] perf(profiling): select legacy allocator callback at rinit On PHP 8.3 and older, select the direct ZendMM or neighboring custom allocator callback once during rinit. The normal allocation callback no longer loads or branches on prev_custom_mm_alloc for every allocation; the custom allocator callback remains as a documented cold compatibility path. Across six balanced 60-second PHP 8.3 ZTS runs per binary, mean throughput increased from 31,167,496/s to 32,646,366/s (+4.74%) and median throughput increased by 4.56%. Native samples reduced alloc_prof_malloc self time from 6.13% to 3.54% (-42.3% relative). The equivalent PHP 8.5 NTS experiment measured within system noise (+0.79% mean / +0.39% median) and was not retained. Validation: PHP 8.5 NTS cargo test (22 passed), PHP 8.3 ZTS and PHP 7.3 ZTS cargo check, and five PHP 8.3 allocation PHPTs. https://datadoghq.atlassian.net/browse/PROF-15506 --- profiling/src/allocation/allocation_ge84.rs | 1 + profiling/src/allocation/allocation_le83.rs | 64 +++++++++++++-------- 2 files changed, 42 insertions(+), 23 deletions(-) diff --git a/profiling/src/allocation/allocation_ge84.rs b/profiling/src/allocation/allocation_ge84.rs index be7273559a..024b9c8b0a 100644 --- a/profiling/src/allocation/allocation_ge84.rs +++ b/profiling/src/allocation/allocation_ge84.rs @@ -297,6 +297,7 @@ unsafe fn alloc_prof_malloc_impl(len: size_t) -> *mut c_void { #[inline(always)] unsafe fn alloc_prof_forward_alloc(len: size_t) -> *mut c_void { let state = tls_zend_mm_state_copy!(); + // Compatibility path for another extension's previously installed custom allocator. if let Some(alloc) = state.prev_custom_mm_alloc { #[cfg(php_debug)] return alloc(len, ptr::null(), 0, ptr::null(), 0); diff --git a/profiling/src/allocation/allocation_le83.rs b/profiling/src/allocation/allocation_le83.rs index 7a8b2fa430..f784cd027d 100644 --- a/profiling/src/allocation/allocation_le83.rs +++ b/profiling/src/allocation/allocation_le83.rs @@ -104,6 +104,8 @@ pub fn alloc_prof_rinit(heap_live_enabled: bool) { zend_mm_state.prev_custom_mm_realloc = None; } + let malloc_handler = + alloc_prof_malloc_handler(zend_mm_state.prev_custom_mm_alloc.is_some()); let free_handler = alloc_prof_free_handler(heap_live_enabled); let realloc_handler = alloc_prof_realloc_handler(heap_live_enabled); @@ -111,7 +113,7 @@ pub fn alloc_prof_rinit(heap_live_enabled: bool) { unsafe { zend::ddog_php_prof_zend_mm_set_custom_handlers( heap, - Some(alloc_prof_malloc), + Some(malloc_handler), Some(free_handler), Some(realloc_handler), ); @@ -161,10 +163,12 @@ pub fn alloc_prof_rshutdown(heap_live_enabled: bool) { &mut custom_mm_realloc, ); } + let malloc_handler = + alloc_prof_malloc_handler(zend_mm_state.prev_custom_mm_alloc.is_some()); let free_handler = alloc_prof_free_handler(heap_live_enabled); let realloc_handler = alloc_prof_realloc_handler(heap_live_enabled); if custom_mm_free != Some(free_handler) - || custom_mm_malloc != Some(alloc_prof_malloc) + || custom_mm_malloc != Some(malloc_handler) || custom_mm_realloc != Some(realloc_handler) { // Custom handlers are installed, but it's not us. Someone, somewhere might have @@ -267,13 +271,47 @@ unsafe extern "C" fn alloc_prof_gc_mem_caches( } } +fn alloc_prof_malloc_handler(has_previous_allocator: bool) -> zend::VmMmCustomAllocFn { + if has_previous_allocator { + alloc_prof_malloc_custom + } else { + alloc_prof_malloc + } +} + unsafe extern "C" fn alloc_prof_malloc(len: size_t) -> *mut c_void { + alloc_prof_malloc_impl::(len) +} + +// Compatibility path for another extension's previously installed custom allocator. +#[cold] +unsafe extern "C" fn alloc_prof_malloc_custom(len: size_t) -> *mut c_void { + alloc_prof_malloc_impl::(len) +} + +#[inline(always)] +unsafe fn alloc_prof_malloc_impl(len: size_t) -> *mut c_void { #[cfg(feature = "debug_stats")] ALLOCATION_PROFILING_COUNT.fetch_add(1, Relaxed); #[cfg(feature = "debug_stats")] ALLOCATION_PROFILING_SIZE.fetch_add(len as u64, Relaxed); - let ptr = alloc_prof_forward_alloc(len); + let state = tls_zend_mm_state_copy!(); + let ptr = if CUSTOM { + state.prev_custom_mm_alloc.unwrap()(len) + } else { + // SAFETY: this callback is only invoked after rinit stores the heap and + // before rshutdown clears it. + let heap = state.heap.unwrap_unchecked(); + let (prepare, restore) = state.prepare_restore_zend_heap; + let custom_heap = prepare(heap); + #[cfg(php_debug)] + let ptr = zend::_zend_mm_alloc(heap, len, ptr::null(), 0, ptr::null(), 0); + #[cfg(not(php_debug))] + let ptr = zend::_zend_mm_alloc(heap, len); + restore(heap, custom_heap); + ptr + }; // during startup, minit, rinit, ... current_execute_data is null // we are only interested in allocations during userland operations @@ -288,26 +326,6 @@ unsafe extern "C" fn alloc_prof_malloc(len: size_t) -> *mut c_void { ptr } -#[inline(always)] -unsafe fn alloc_prof_forward_alloc(len: size_t) -> *mut c_void { - let state = tls_zend_mm_state_copy!(); - if let Some(alloc) = state.prev_custom_mm_alloc { - return alloc(len); - } - - // SAFETY: this callback is only invoked after rinit stores the heap and - // before rshutdown clears it. - let heap = state.heap.unwrap_unchecked(); - let (prepare, restore) = state.prepare_restore_zend_heap; - let custom_heap = prepare(heap); - #[cfg(php_debug)] - let ptr: *mut c_void = zend::_zend_mm_alloc(heap, len, ptr::null(), 0, ptr::null(), 0); - #[cfg(not(php_debug))] - let ptr: *mut c_void = zend::_zend_mm_alloc(heap, len); - restore(heap, custom_heap); - ptr -} - /// This function exists because when calling `zend_mm_set_custom_handlers()`, /// you need to pass a pointer to a `free()` function as well, otherwise your /// custom handlers won't be installed. We cannot just point to the original From e85b2a55fde27e3714dc8508e30caf50c41dbcc6 Mon Sep 17 00:00:00 2001 From: Florian Engelhardt Date: Tue, 28 Jul 2026 14:17:22 +0200 Subject: [PATCH 13/25] perf(profiling): select modern allocator callback at rinit Apply the same RINIT-selected allocation callback used for PHP 8.3 and older to PHP 8.4 and newer. The normal callback has no previous-allocator load or branch, while a separate cold callback preserves neighboring custom allocator support. The PHP 8.5 NTS benchmark was inconclusive but non-negative: a clean six-run retry measured +0.40% mean and +0.69% median with about 1.8% run variance. Keep the implementation for symmetry across ZendMM APIs rather than as a claimed performance win. Validation: PHP 8.5 NTS cargo test (22 passed) and profiler-release build. https://datadoghq.atlassian.net/browse/PROF-15506 --- profiling/src/allocation/allocation_ge84.rs | 80 ++++++++++++++------- 1 file changed, 54 insertions(+), 26 deletions(-) diff --git a/profiling/src/allocation/allocation_ge84.rs b/profiling/src/allocation/allocation_ge84.rs index 024b9c8b0a..d309711983 100644 --- a/profiling/src/allocation/allocation_ge84.rs +++ b/profiling/src/allocation/allocation_ge84.rs @@ -126,6 +126,8 @@ pub fn alloc_prof_rinit(heap_live_enabled: bool) { zend_mm_state.prev_custom_mm_shutdown = None; } + let malloc_handler = + alloc_prof_malloc_handler(zend_mm_state.prev_custom_mm_alloc.is_some()); let free_handler = alloc_prof_free_handler(heap_live_enabled); let realloc_handler = alloc_prof_realloc_handler(heap_live_enabled); @@ -133,7 +135,7 @@ pub fn alloc_prof_rinit(heap_live_enabled: bool) { unsafe { zend::zend_mm_set_custom_handlers_ex( heap, - Some(alloc_prof_malloc), + Some(malloc_handler), Some(free_handler), Some(realloc_handler), Some(alloc_prof_gc), @@ -189,10 +191,12 @@ pub fn alloc_prof_rshutdown(heap_live_enabled: bool) { &mut custom_mm_shutdown, ); } + let malloc_handler = + alloc_prof_malloc_handler(zend_mm_state.prev_custom_mm_alloc.is_some()); let free_handler = alloc_prof_free_handler(heap_live_enabled); let realloc_handler = alloc_prof_realloc_handler(heap_live_enabled); if custom_mm_free != Some(free_handler) - || custom_mm_malloc != Some(alloc_prof_malloc) + || custom_mm_malloc != Some(malloc_handler) || custom_mm_realloc != Some(realloc_handler) || custom_mm_gc != Some(alloc_prof_gc) || custom_mm_shutdown != Some(alloc_prof_shutdown) @@ -256,9 +260,17 @@ unsafe fn restore_zend_heap(heap: *mut zend::_zend_mm_heap, custom_heap: c_int) ptr::write(heap as *mut c_int, custom_heap); } +fn alloc_prof_malloc_handler(has_previous_allocator: bool) -> zend::VmMmCustomAllocFn { + if has_previous_allocator { + alloc_prof_malloc_custom + } else { + alloc_prof_malloc + } +} + #[cfg(not(php_debug))] unsafe extern "C" fn alloc_prof_malloc(len: size_t) -> *mut c_void { - alloc_prof_malloc_impl(len) + alloc_prof_malloc_impl::(len) } #[cfg(php_debug)] @@ -269,17 +281,53 @@ unsafe extern "C" fn alloc_prof_malloc( _orig_file: *const c_char, _orig_line: c_uint, ) -> *mut c_void { - alloc_prof_malloc_impl(len) + alloc_prof_malloc_impl::(len) +} + +// Compatibility path for another extension's previously installed custom allocator. +#[cold] +#[cfg(not(php_debug))] +unsafe extern "C" fn alloc_prof_malloc_custom(len: size_t) -> *mut c_void { + alloc_prof_malloc_impl::(len) +} + +#[cold] +#[cfg(php_debug)] +unsafe extern "C" fn alloc_prof_malloc_custom( + len: size_t, + _file: *const c_char, + _line: c_uint, + _orig_file: *const c_char, + _orig_line: c_uint, +) -> *mut c_void { + alloc_prof_malloc_impl::(len) } #[inline(always)] -unsafe fn alloc_prof_malloc_impl(len: size_t) -> *mut c_void { +unsafe fn alloc_prof_malloc_impl(len: size_t) -> *mut c_void { #[cfg(feature = "debug_stats")] ALLOCATION_PROFILING_COUNT.fetch_add(1, Relaxed); #[cfg(feature = "debug_stats")] ALLOCATION_PROFILING_SIZE.fetch_add(len as u64, Relaxed); - let ptr = alloc_prof_forward_alloc(len); + let state = tls_zend_mm_state_copy!(); + let ptr = if CUSTOM { + let alloc = state.prev_custom_mm_alloc.unwrap(); + #[cfg(php_debug)] + let ptr = alloc(len, ptr::null(), 0, ptr::null(), 0); + #[cfg(not(php_debug))] + let ptr = alloc(len); + ptr + } else { + // SAFETY: this callback is only invoked after rinit stores the heap and + // before rshutdown clears it. + let heap = state.heap.unwrap_unchecked(); + #[cfg(php_debug)] + let ptr = zend::_zend_mm_alloc(heap, len, ptr::null(), 0, ptr::null(), 0); + #[cfg(not(php_debug))] + let ptr = zend::_zend_mm_alloc(heap, len); + ptr + }; // during startup, minit, rinit, ... current_execute_data is null // we are only interested in allocations during userland operations @@ -294,26 +342,6 @@ unsafe fn alloc_prof_malloc_impl(len: size_t) -> *mut c_void { ptr } -#[inline(always)] -unsafe fn alloc_prof_forward_alloc(len: size_t) -> *mut c_void { - let state = tls_zend_mm_state_copy!(); - // Compatibility path for another extension's previously installed custom allocator. - if let Some(alloc) = state.prev_custom_mm_alloc { - #[cfg(php_debug)] - return alloc(len, ptr::null(), 0, ptr::null(), 0); - #[cfg(not(php_debug))] - return alloc(len); - } - - // SAFETY: this callback is only invoked after rinit stores the heap and - // before rshutdown clears it. - let heap = state.heap.unwrap_unchecked(); - #[cfg(php_debug)] - return zend::_zend_mm_alloc(heap, len, ptr::null(), 0, ptr::null(), 0); - #[cfg(not(php_debug))] - zend::_zend_mm_alloc(heap, len) -} - /// This function exists because when calling `zend_mm_set_custom_handlers()`, /// you need to pass a pointer to a `free()` function as well, otherwise your /// custom handlers won't be installed. We cannot just point to the original From ae4de114677b21fadd402370fdbedb3aca37ef32 Mon Sep 17 00:00:00 2001 From: Florian Engelhardt Date: Tue, 28 Jul 2026 17:24:30 +0200 Subject: [PATCH 14/25] perf(profiling): select free and realloc callbacks at rinit --- profiling/src/allocation/allocation_ge84.rs | 219 ++++++++++++-------- profiling/src/allocation/allocation_le83.rs | 196 +++++++++++------- 2 files changed, 245 insertions(+), 170 deletions(-) diff --git a/profiling/src/allocation/allocation_ge84.rs b/profiling/src/allocation/allocation_ge84.rs index d309711983..9ffd83ecd5 100644 --- a/profiling/src/allocation/allocation_ge84.rs +++ b/profiling/src/allocation/allocation_ge84.rs @@ -128,8 +128,14 @@ pub fn alloc_prof_rinit(heap_live_enabled: bool) { let malloc_handler = alloc_prof_malloc_handler(zend_mm_state.prev_custom_mm_alloc.is_some()); - let free_handler = alloc_prof_free_handler(heap_live_enabled); - let realloc_handler = alloc_prof_realloc_handler(heap_live_enabled); + let free_handler = alloc_prof_free_handler( + heap_live_enabled, + zend_mm_state.prev_custom_mm_free.is_some(), + ); + let realloc_handler = alloc_prof_realloc_handler( + heap_live_enabled, + zend_mm_state.prev_custom_mm_realloc.is_some(), + ); // install our custom handler to ZendMM unsafe { @@ -193,8 +199,14 @@ pub fn alloc_prof_rshutdown(heap_live_enabled: bool) { } let malloc_handler = alloc_prof_malloc_handler(zend_mm_state.prev_custom_mm_alloc.is_some()); - let free_handler = alloc_prof_free_handler(heap_live_enabled); - let realloc_handler = alloc_prof_realloc_handler(heap_live_enabled); + let free_handler = alloc_prof_free_handler( + heap_live_enabled, + zend_mm_state.prev_custom_mm_free.is_some(), + ); + let realloc_handler = alloc_prof_realloc_handler( + heap_live_enabled, + zend_mm_state.prev_custom_mm_realloc.is_some(), + ); if custom_mm_free != Some(free_handler) || custom_mm_malloc != Some(malloc_handler) || custom_mm_realloc != Some(realloc_handler) @@ -347,88 +359,98 @@ unsafe fn alloc_prof_malloc_impl(len: size_t) -> *mut c_void /// custom handlers won't be installed. We cannot just point to the original /// `zend::_zend_mm_free()` as the function definitions differ. #[cfg(not(php_debug))] -unsafe extern "C" fn alloc_prof_free(ptr: *mut c_void) { - alloc_prof_free_impl(ptr); +unsafe extern "C" fn alloc_prof_free(ptr: *mut c_void) { + alloc_prof_free_impl::(ptr); } #[cfg(php_debug)] -unsafe extern "C" fn alloc_prof_free( +unsafe extern "C" fn alloc_prof_free( ptr: *mut c_void, _file: *const c_char, _line: c_uint, _orig_file: *const c_char, _orig_line: c_uint, ) { - alloc_prof_free_impl(ptr); -} - -fn alloc_prof_free_handler(heap_live_enabled: bool) -> zend::VmMmCustomFreeFn { - if heap_live_enabled { - alloc_prof_free - } else { - alloc_prof_free_noop - } + alloc_prof_free_impl::(ptr); } +// Compatibility path for another extension's previously installed custom allocator. +#[cold] #[cfg(not(php_debug))] -unsafe extern "C" fn alloc_prof_free_noop(ptr: *mut c_void) { - alloc_prof_forward_free(ptr); +unsafe extern "C" fn alloc_prof_free_custom(ptr: *mut c_void) { + alloc_prof_free_impl::(ptr); } +#[cold] #[cfg(php_debug)] -unsafe extern "C" fn alloc_prof_free_noop( +unsafe extern "C" fn alloc_prof_free_custom( ptr: *mut c_void, _file: *const c_char, _line: c_uint, _orig_file: *const c_char, _orig_line: c_uint, ) { - alloc_prof_forward_free(ptr); + alloc_prof_free_impl::(ptr); +} + +fn alloc_prof_free_handler( + heap_live_enabled: bool, + has_previous_allocator: bool, +) -> zend::VmMmCustomFreeFn { + match (heap_live_enabled, has_previous_allocator) { + (true, false) => alloc_prof_free::, + (false, false) => alloc_prof_free::, + (true, true) => alloc_prof_free_custom::, + (false, true) => alloc_prof_free_custom::, + } } #[inline(always)] -unsafe fn alloc_prof_free_impl(ptr: *mut c_void) { - // Heap-live is enabled when this handler is registered. - if !ptr.is_null() { +unsafe fn alloc_prof_free_impl(ptr: *mut c_void) { + if TRACK && !ptr.is_null() { untrack_allocation(ptr); } - alloc_prof_forward_free(ptr); -} -#[inline(always)] -unsafe fn alloc_prof_forward_free(ptr: *mut c_void) { let state = tls_zend_mm_state_copy!(); - if let Some(free) = state.prev_custom_mm_free { + if CUSTOM { + let free = state.prev_custom_mm_free.unwrap(); + #[cfg(php_debug)] + free(ptr, core::ptr::null(), 0, core::ptr::null(), 0); + #[cfg(not(php_debug))] + free(ptr); + } else { + // SAFETY: this callback is only invoked after rinit stores the heap and + // before rshutdown clears it. + let heap = state.heap.unwrap_unchecked(); #[cfg(php_debug)] - return free(ptr, core::ptr::null(), 0, core::ptr::null(), 0); + zend::_zend_mm_free(heap, ptr, core::ptr::null(), 0, core::ptr::null(), 0); #[cfg(not(php_debug))] - return free(ptr); + zend::_zend_mm_free(heap, ptr); } - - // SAFETY: this callback is only invoked after rinit stores the heap and - // before rshutdown clears it. - let heap = state.heap.unwrap_unchecked(); - #[cfg(php_debug)] - return zend::_zend_mm_free(heap, ptr, core::ptr::null(), 0, core::ptr::null(), 0); - #[cfg(not(php_debug))] - zend::_zend_mm_free(heap, ptr); } -fn alloc_prof_realloc_handler(heap_live_enabled: bool) -> zend::VmMmCustomReallocFn { - if heap_live_enabled { - alloc_prof_realloc - } else { - alloc_prof_realloc_no_untrack +fn alloc_prof_realloc_handler( + heap_live_enabled: bool, + has_previous_allocator: bool, +) -> zend::VmMmCustomReallocFn { + match (heap_live_enabled, has_previous_allocator) { + (true, false) => alloc_prof_realloc::, + (false, false) => alloc_prof_realloc::, + (true, true) => alloc_prof_realloc_custom::, + (false, true) => alloc_prof_realloc_custom::, } } #[cfg(not(php_debug))] -unsafe extern "C" fn alloc_prof_realloc(prev_ptr: *mut c_void, len: size_t) -> *mut c_void { - alloc_prof_realloc_impl(prev_ptr, len) +unsafe extern "C" fn alloc_prof_realloc( + prev_ptr: *mut c_void, + len: size_t, +) -> *mut c_void { + alloc_prof_realloc_impl::(prev_ptr, len) } #[cfg(php_debug)] -unsafe extern "C" fn alloc_prof_realloc( +unsafe extern "C" fn alloc_prof_realloc( prev_ptr: *mut c_void, len: size_t, _file: *const c_char, @@ -436,19 +458,22 @@ unsafe extern "C" fn alloc_prof_realloc( _orig_file: *const c_char, _orig_line: c_uint, ) -> *mut c_void { - alloc_prof_realloc_impl(prev_ptr, len) + alloc_prof_realloc_impl::(prev_ptr, len) } +// Compatibility path for another extension's previously installed custom allocator. +#[cold] #[cfg(not(php_debug))] -unsafe extern "C" fn alloc_prof_realloc_no_untrack( +unsafe extern "C" fn alloc_prof_realloc_custom( prev_ptr: *mut c_void, len: size_t, ) -> *mut c_void { - alloc_prof_realloc_no_untrack_impl(prev_ptr, len) + alloc_prof_realloc_impl::(prev_ptr, len) } +#[cold] #[cfg(php_debug)] -unsafe extern "C" fn alloc_prof_realloc_no_untrack( +unsafe extern "C" fn alloc_prof_realloc_custom( prev_ptr: *mut c_void, len: size_t, _file: *const c_char, @@ -456,41 +481,49 @@ unsafe extern "C" fn alloc_prof_realloc_no_untrack( _orig_file: *const c_char, _orig_line: c_uint, ) -> *mut c_void { - alloc_prof_realloc_no_untrack_impl(prev_ptr, len) + alloc_prof_realloc_impl::(prev_ptr, len) } #[inline(always)] -unsafe fn alloc_prof_realloc_impl(prev_ptr: *mut c_void, len: size_t) -> *mut c_void { +unsafe fn alloc_prof_realloc_impl( + prev_ptr: *mut c_void, + len: size_t, +) -> *mut c_void { #[cfg(feature = "debug_stats")] ALLOCATION_PROFILING_COUNT.fetch_add(1, Relaxed); #[cfg(feature = "debug_stats")] ALLOCATION_PROFILING_SIZE.fetch_add(len as u64, Relaxed); - let ptr = alloc_prof_forward_realloc(prev_ptr, len); + let state = tls_zend_mm_state_copy!(); + let ptr = if CUSTOM { + let realloc = state.prev_custom_mm_realloc.unwrap(); + #[cfg(php_debug)] + let ptr = realloc(prev_ptr, len, ptr::null(), 0, ptr::null(), 0); + #[cfg(not(php_debug))] + let ptr = realloc(prev_ptr, len); + ptr + } else { + // SAFETY: this callback is only invoked after rinit stores the heap and + // before rshutdown clears it. + let heap = state.heap.unwrap_unchecked(); + #[cfg(php_debug)] + let ptr = zend::_zend_mm_realloc(heap, prev_ptr, len, ptr::null(), 0, ptr::null(), 0); + #[cfg(not(php_debug))] + let ptr = zend::_zend_mm_realloc(heap, prev_ptr, len); + ptr + }; // ZendMM allocation failures raise a fatal error and bail out instead of // returning NULL. If realloc returns, prev_ptr has been consumed: untrack it // before any userland-only early return, then let the new allocation be // re-sampled at the reported size. - if !prev_ptr.is_null() { + if UNTRACK && !prev_ptr.is_null() { untrack_allocation(prev_ptr); } alloc_prof_realloc_sample(ptr, len) } -#[inline(always)] -unsafe fn alloc_prof_realloc_no_untrack_impl(prev_ptr: *mut c_void, len: size_t) -> *mut c_void { - #[cfg(feature = "debug_stats")] - ALLOCATION_PROFILING_COUNT.fetch_add(1, Relaxed); - #[cfg(feature = "debug_stats")] - ALLOCATION_PROFILING_SIZE.fetch_add(len as u64, Relaxed); - - let ptr = alloc_prof_forward_realloc(prev_ptr, len); - - alloc_prof_realloc_sample(ptr, len) -} - #[inline(always)] unsafe fn alloc_prof_realloc_sample(ptr: *mut c_void, len: size_t) -> *mut c_void { // during startup, minit, rinit, ... current_execute_data is null @@ -510,25 +543,6 @@ unsafe fn alloc_prof_realloc_sample(ptr: *mut c_void, len: size_t) -> *mut c_voi ptr } -#[inline(always)] -unsafe fn alloc_prof_forward_realloc(prev_ptr: *mut c_void, len: size_t) -> *mut c_void { - let state = tls_zend_mm_state_copy!(); - if let Some(realloc) = state.prev_custom_mm_realloc { - #[cfg(php_debug)] - return realloc(prev_ptr, len, ptr::null(), 0, ptr::null(), 0); - #[cfg(not(php_debug))] - return realloc(prev_ptr, len); - } - - // SAFETY: this callback is only invoked after rinit stores the heap and - // before rshutdown clears it. - let heap = state.heap.unwrap_unchecked(); - #[cfg(php_debug)] - return zend::_zend_mm_realloc(heap, prev_ptr, len, ptr::null(), 0, ptr::null(), 0); - #[cfg(not(php_debug))] - zend::_zend_mm_realloc(heap, prev_ptr, len) -} - unsafe extern "C" fn alloc_prof_gc() -> size_t { tls_zend_mm_state_get!(gc)() } @@ -576,14 +590,39 @@ mod tests { use super::*; #[test] - fn free_handler_tracks_only_when_heap_live_is_enabled() { + fn handlers_are_selected_at_rinit() { + assert_eq!( + alloc_prof_free_handler(true, false) as usize, + alloc_prof_free:: as zend::VmMmCustomFreeFn as usize + ); + assert_eq!( + alloc_prof_free_handler(false, false) as usize, + alloc_prof_free:: as zend::VmMmCustomFreeFn as usize + ); + assert_eq!( + alloc_prof_free_handler(true, true) as usize, + alloc_prof_free_custom:: as zend::VmMmCustomFreeFn as usize + ); + assert_eq!( + alloc_prof_free_handler(false, true) as usize, + alloc_prof_free_custom:: as zend::VmMmCustomFreeFn as usize + ); + + assert_eq!( + alloc_prof_realloc_handler(true, false) as usize, + alloc_prof_realloc:: as zend::VmMmCustomReallocFn as usize + ); + assert_eq!( + alloc_prof_realloc_handler(false, false) as usize, + alloc_prof_realloc:: as zend::VmMmCustomReallocFn as usize + ); assert_eq!( - alloc_prof_free_handler(true) as usize, - alloc_prof_free as zend::VmMmCustomFreeFn as usize + alloc_prof_realloc_handler(true, true) as usize, + alloc_prof_realloc_custom:: as zend::VmMmCustomReallocFn as usize ); assert_eq!( - alloc_prof_free_handler(false) as usize, - alloc_prof_free_noop as zend::VmMmCustomFreeFn as usize + alloc_prof_realloc_handler(false, true) as usize, + alloc_prof_realloc_custom:: as zend::VmMmCustomReallocFn as usize ); } diff --git a/profiling/src/allocation/allocation_le83.rs b/profiling/src/allocation/allocation_le83.rs index f784cd027d..e10b83b1c8 100644 --- a/profiling/src/allocation/allocation_le83.rs +++ b/profiling/src/allocation/allocation_le83.rs @@ -106,8 +106,14 @@ pub fn alloc_prof_rinit(heap_live_enabled: bool) { let malloc_handler = alloc_prof_malloc_handler(zend_mm_state.prev_custom_mm_alloc.is_some()); - let free_handler = alloc_prof_free_handler(heap_live_enabled); - let realloc_handler = alloc_prof_realloc_handler(heap_live_enabled); + let free_handler = alloc_prof_free_handler( + heap_live_enabled, + zend_mm_state.prev_custom_mm_free.is_some(), + ); + let realloc_handler = alloc_prof_realloc_handler( + heap_live_enabled, + zend_mm_state.prev_custom_mm_realloc.is_some(), + ); // install our custom handler to ZendMM unsafe { @@ -165,8 +171,14 @@ pub fn alloc_prof_rshutdown(heap_live_enabled: bool) { } let malloc_handler = alloc_prof_malloc_handler(zend_mm_state.prev_custom_mm_alloc.is_some()); - let free_handler = alloc_prof_free_handler(heap_live_enabled); - let realloc_handler = alloc_prof_realloc_handler(heap_live_enabled); + let free_handler = alloc_prof_free_handler( + heap_live_enabled, + zend_mm_state.prev_custom_mm_free.is_some(), + ); + let realloc_handler = alloc_prof_realloc_handler( + heap_live_enabled, + zend_mm_state.prev_custom_mm_realloc.is_some(), + ); if custom_mm_free != Some(free_handler) || custom_mm_malloc != Some(malloc_handler) || custom_mm_realloc != Some(realloc_handler) @@ -330,94 +342,114 @@ unsafe fn alloc_prof_malloc_impl(len: size_t) -> *mut c_void /// you need to pass a pointer to a `free()` function as well, otherwise your /// custom handlers won't be installed. We cannot just point to the original /// `zend::_zend_mm_free()` as the function definitions differ. -unsafe extern "C" fn alloc_prof_free(ptr: *mut c_void) { - // Heap-live is enabled when this handler is registered. - if !ptr.is_null() { - untrack_allocation(ptr); - } - - alloc_prof_forward_free(ptr); +unsafe extern "C" fn alloc_prof_free(ptr: *mut c_void) { + alloc_prof_free_impl::(ptr) } -fn alloc_prof_free_handler(heap_live_enabled: bool) -> zend::VmMmCustomFreeFn { - if heap_live_enabled { - alloc_prof_free - } else { - alloc_prof_free_noop - } +// Compatibility path for another extension's previously installed custom allocator. +#[cold] +unsafe extern "C" fn alloc_prof_free_custom(ptr: *mut c_void) { + alloc_prof_free_impl::(ptr) } -unsafe extern "C" fn alloc_prof_free_noop(ptr: *mut c_void) { - alloc_prof_forward_free(ptr); +fn alloc_prof_free_handler( + heap_live_enabled: bool, + has_previous_allocator: bool, +) -> zend::VmMmCustomFreeFn { + match (heap_live_enabled, has_previous_allocator) { + (true, false) => alloc_prof_free::, + (false, false) => alloc_prof_free::, + (true, true) => alloc_prof_free_custom::, + (false, true) => alloc_prof_free_custom::, + } } #[inline(always)] -unsafe fn alloc_prof_forward_free(ptr: *mut c_void) { - let state = tls_zend_mm_state_copy!(); - if let Some(free) = state.prev_custom_mm_free { - return free(ptr); +unsafe fn alloc_prof_free_impl(ptr: *mut c_void) { + if TRACK && !ptr.is_null() { + untrack_allocation(ptr); } - // SAFETY: this callback is only invoked after rinit stores the heap and - // before rshutdown clears it. - let heap = state.heap.unwrap_unchecked(); - #[cfg(php_debug)] - zend::_zend_mm_free(heap, ptr, core::ptr::null(), 0, core::ptr::null(), 0); - #[cfg(not(php_debug))] - zend::_zend_mm_free(heap, ptr); + let state = tls_zend_mm_state_copy!(); + if CUSTOM { + state.prev_custom_mm_free.unwrap()(ptr); + } else { + // SAFETY: this callback is only invoked after rinit stores the heap and + // before rshutdown clears it. + let heap = state.heap.unwrap_unchecked(); + #[cfg(php_debug)] + zend::_zend_mm_free(heap, ptr, core::ptr::null(), 0, core::ptr::null(), 0); + #[cfg(not(php_debug))] + zend::_zend_mm_free(heap, ptr); + } } -fn alloc_prof_realloc_handler(heap_live_enabled: bool) -> zend::VmMmCustomReallocFn { - if heap_live_enabled { - alloc_prof_realloc - } else { - alloc_prof_realloc_no_untrack +fn alloc_prof_realloc_handler( + heap_live_enabled: bool, + has_previous_allocator: bool, +) -> zend::VmMmCustomReallocFn { + match (heap_live_enabled, has_previous_allocator) { + (true, false) => alloc_prof_realloc::, + (false, false) => alloc_prof_realloc::, + (true, true) => alloc_prof_realloc_custom::, + (false, true) => alloc_prof_realloc_custom::, } } -unsafe extern "C" fn alloc_prof_realloc(prev_ptr: *mut c_void, len: size_t) -> *mut c_void { - alloc_prof_realloc_impl(prev_ptr, len) +unsafe extern "C" fn alloc_prof_realloc( + prev_ptr: *mut c_void, + len: size_t, +) -> *mut c_void { + alloc_prof_realloc_impl::(prev_ptr, len) } -unsafe extern "C" fn alloc_prof_realloc_no_untrack( +// Compatibility path for another extension's previously installed custom allocator. +#[cold] +unsafe extern "C" fn alloc_prof_realloc_custom( prev_ptr: *mut c_void, len: size_t, ) -> *mut c_void { - alloc_prof_realloc_no_untrack_impl(prev_ptr, len) + alloc_prof_realloc_impl::(prev_ptr, len) } #[inline(always)] -unsafe fn alloc_prof_realloc_impl(prev_ptr: *mut c_void, len: size_t) -> *mut c_void { +unsafe fn alloc_prof_realloc_impl( + prev_ptr: *mut c_void, + len: size_t, +) -> *mut c_void { #[cfg(feature = "debug_stats")] ALLOCATION_PROFILING_COUNT.fetch_add(1, Relaxed); #[cfg(feature = "debug_stats")] ALLOCATION_PROFILING_SIZE.fetch_add(len as u64, Relaxed); - let ptr = alloc_prof_forward_realloc(prev_ptr, len); + let state = tls_zend_mm_state_copy!(); + let ptr = if CUSTOM { + state.prev_custom_mm_realloc.unwrap()(prev_ptr, len) + } else { + // SAFETY: this callback is only invoked after rinit stores the heap and + // before rshutdown clears it. + let heap = state.heap.unwrap_unchecked(); + let (prepare, restore) = state.prepare_restore_zend_heap; + let custom_heap = prepare(heap); + #[cfg(php_debug)] + let ptr = zend::_zend_mm_realloc(heap, prev_ptr, len, ptr::null(), 0, ptr::null(), 0); + #[cfg(not(php_debug))] + let ptr = zend::_zend_mm_realloc(heap, prev_ptr, len); + restore(heap, custom_heap); + ptr + }; // ZendMM allocation failures raise a fatal error and bail out instead of // returning NULL. If realloc returns, prev_ptr has been consumed: untrack it // before any userland-only early return, then let the new allocation be // re-sampled at the reported size. - if !prev_ptr.is_null() { + if UNTRACK && !prev_ptr.is_null() { untrack_allocation(prev_ptr); } alloc_prof_realloc_sample(ptr, len) } -#[inline(always)] -unsafe fn alloc_prof_realloc_no_untrack_impl(prev_ptr: *mut c_void, len: size_t) -> *mut c_void { - #[cfg(feature = "debug_stats")] - ALLOCATION_PROFILING_COUNT.fetch_add(1, Relaxed); - #[cfg(feature = "debug_stats")] - ALLOCATION_PROFILING_SIZE.fetch_add(len as u64, Relaxed); - - let ptr = alloc_prof_forward_realloc(prev_ptr, len); - - alloc_prof_realloc_sample(ptr, len) -} - #[inline(always)] unsafe fn alloc_prof_realloc_sample(ptr: *mut c_void, len: size_t) -> *mut c_void { // during startup, minit, rinit, ... current_execute_data is null @@ -437,27 +469,6 @@ unsafe fn alloc_prof_realloc_sample(ptr: *mut c_void, len: size_t) -> *mut c_voi ptr } -#[inline(always)] -unsafe fn alloc_prof_forward_realloc(prev_ptr: *mut c_void, len: size_t) -> *mut c_void { - let state = tls_zend_mm_state_copy!(); - if let Some(realloc) = state.prev_custom_mm_realloc { - return realloc(prev_ptr, len); - } - - // SAFETY: this callback is only invoked after rinit stores the heap and - // before rshutdown clears it. - let heap = state.heap.unwrap_unchecked(); - let (prepare, restore) = state.prepare_restore_zend_heap; - let custom_heap = prepare(heap); - #[cfg(php_debug)] - let ptr: *mut c_void = - zend::_zend_mm_realloc(heap, prev_ptr, len, ptr::null(), 0, ptr::null(), 0); - #[cfg(not(php_debug))] - let ptr: *mut c_void = zend::_zend_mm_realloc(heap, prev_ptr, len); - restore(heap, custom_heap); - ptr -} - /// safe wrapper for `zend::is_zend_mm()`. /// `true` means the internal ZendMM is being used, `false` means that a custom memory manager is /// installed. Upstream returns a `c_bool` as of PHP 8.0. PHP 7 returns a `c_int` @@ -477,14 +488,39 @@ mod tests { use super::*; #[test] - fn free_handler_tracks_only_when_heap_live_is_enabled() { + fn handlers_are_selected_at_rinit() { + assert_eq!( + alloc_prof_free_handler(true, false) as usize, + alloc_prof_free:: as usize + ); + assert_eq!( + alloc_prof_free_handler(false, false) as usize, + alloc_prof_free:: as usize + ); + assert_eq!( + alloc_prof_free_handler(true, true) as usize, + alloc_prof_free_custom:: as usize + ); + assert_eq!( + alloc_prof_free_handler(false, true) as usize, + alloc_prof_free_custom:: as usize + ); + + assert_eq!( + alloc_prof_realloc_handler(true, false) as usize, + alloc_prof_realloc:: as usize + ); + assert_eq!( + alloc_prof_realloc_handler(false, false) as usize, + alloc_prof_realloc:: as usize + ); assert_eq!( - alloc_prof_free_handler(true) as usize, - alloc_prof_free as usize + alloc_prof_realloc_handler(true, true) as usize, + alloc_prof_realloc_custom:: as usize ); assert_eq!( - alloc_prof_free_handler(false) as usize, - alloc_prof_free_noop as usize + alloc_prof_realloc_handler(false, true) as usize, + alloc_prof_realloc_custom:: as usize ); } From 010c2f717b67c705fd3aa69210bf7977adf0113f Mon Sep 17 00:00:00 2001 From: Florian Engelhardt Date: Tue, 28 Jul 2026 20:04:13 +0200 Subject: [PATCH 15/25] test(profiling): stub realloc callback dependencies --- profiling/src/allocation/mod.rs | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/profiling/src/allocation/mod.rs b/profiling/src/allocation/mod.rs index 30241604c2..f366d6a1cb 100644 --- a/profiling/src/allocation/mod.rs +++ b/profiling/src/allocation/mod.rs @@ -80,7 +80,12 @@ pub mod allocation_ge84; #[cfg(not(php_zend_mm_set_custom_handlers_ex))] pub mod allocation_le83; -// Handler-selection tests retain the free callbacks in a binary that is not loaded by PHP. +// Handler-selection tests retain callbacks in a binary that is not loaded by PHP. +#[cfg(all(test, not(php_zts)))] +#[export_name = "executor_globals"] +static mut TEST_EXECUTOR_GLOBALS: core::mem::MaybeUninit = + core::mem::MaybeUninit::zeroed(); + #[cfg(all(test, not(php_debug)))] #[no_mangle] unsafe extern "C" fn _zend_mm_free(_heap: *mut zend::_zend_mm_heap, _ptr: *mut c_void) {} @@ -97,6 +102,30 @@ unsafe extern "C" fn _zend_mm_free( ) { } +#[cfg(all(test, not(php_debug)))] +#[no_mangle] +unsafe extern "C" fn _zend_mm_realloc( + _heap: *mut zend::_zend_mm_heap, + _ptr: *mut c_void, + _len: size_t, +) -> *mut c_void { + ptr::null_mut() +} + +#[cfg(all(test, php_debug))] +#[no_mangle] +unsafe extern "C" fn _zend_mm_realloc( + _heap: *mut zend::_zend_mm_heap, + _ptr: *mut c_void, + _len: size_t, + _file: *const libc::c_char, + _line: libc::c_uint, + _orig_file: *const libc::c_char, + _orig_line: libc::c_uint, +) -> *mut c_void { + ptr::null_mut() +} + /// Default sampling interval in bytes (4 MiB). pub const DEFAULT_ALLOCATION_SAMPLING_INTERVAL: NonZeroU32 = NonZero::new(1024 * 4096).unwrap(); From 9cefb02219a3b37836480dffe5fe2167784d6758 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Tue, 28 Jul 2026 18:05:59 -0600 Subject: [PATCH 16/25] Revert "fix(profiling): guard atomic interrupt load on PHP 8.2" This reverts commit be256447ac02963392231381428c2b2500b7b2c0. --- profiling/src/php_ffi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/profiling/src/php_ffi.c b/profiling/src/php_ffi.c index be674ed216..5799822ef0 100644 --- a/profiling/src/php_ffi.c +++ b/profiling/src/php_ffi.c @@ -249,7 +249,7 @@ zend_execute_data* ddog_php_prof_get_current_execute_data() { } bool ddog_php_prof_vm_interrupt_pending() { -#if PHP_VERSION_ID >= 80200 +#if PHP_VERSION_ID >= 80000 return zend_atomic_bool_load_ex(&EG(vm_interrupt)); #else return EG(vm_interrupt); From 9ac8a332b4dff04ed1586522b1dda36b253aed55 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Tue, 28 Jul 2026 18:06:02 -0600 Subject: [PATCH 17/25] Revert "perf(profiling): skip idle internal interrupt handling" This reverts commit 02acd656e963354265362eeb6184cdd3ac6e7219. --- profiling/src/php_ffi.c | 8 -------- profiling/src/php_ffi.h | 1 - profiling/src/wall_time.rs | 6 ++---- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/profiling/src/php_ffi.c b/profiling/src/php_ffi.c index 5799822ef0..1e906f3cbd 100644 --- a/profiling/src/php_ffi.c +++ b/profiling/src/php_ffi.c @@ -248,14 +248,6 @@ zend_execute_data* ddog_php_prof_get_current_execute_data() { return EG(current_execute_data); } -bool ddog_php_prof_vm_interrupt_pending() { -#if PHP_VERSION_ID >= 80000 - return zend_atomic_bool_load_ex(&EG(vm_interrupt)); -#else - return EG(vm_interrupt); -#endif -} - #if CFG_FIBERS // defined by build.rs zend_fiber* ddog_php_prof_get_active_fiber() { diff --git a/profiling/src/php_ffi.h b/profiling/src/php_ffi.h index 1f402732ec..558c3de441 100644 --- a/profiling/src/php_ffi.h +++ b/profiling/src/php_ffi.h @@ -162,7 +162,6 @@ void ddog_php_prof_zend_mm_set_custom_handlers(zend_mm_heap *heap, ddog_php_prof_zend_mm_realloc _realloc); zend_execute_data* ddog_php_prof_get_current_execute_data(); -bool ddog_php_prof_vm_interrupt_pending(); #if CFG_FRAMELESS void ddog_php_prof_post_startup(); diff --git a/profiling/src/wall_time.rs b/profiling/src/wall_time.rs index 7227fbcbdd..3f0a948127 100644 --- a/profiling/src/wall_time.rs +++ b/profiling/src/wall_time.rs @@ -79,10 +79,8 @@ mod execute_internal { unsafe { prev_execute_internal(execute_data, return_value) }; // See safety section of `execute_data_func_is_trampoline` docs for why - // the leaf frame is used instead of the execute_data ptr. - if unsafe { zend::ddog_php_prof_vm_interrupt_pending() } { - ddog_php_prof_interrupt_function(leaf_frame); - } + // the leaf frame is used instead of the execute_data ptr. + ddog_php_prof_interrupt_function(leaf_frame); } /// # Safety From 8cb3cde012b27d0ea07cfd3d7b3cac23d19b9faa Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Tue, 28 Jul 2026 18:07:04 -0600 Subject: [PATCH 18/25] perf(profiling): use relaxed read to skip idle interrupts This is very similar to the approach taken by Florian, which did a check for EG(vm_interrupt). Although I was able to reproduce a small speedup there, I realized that it's more nuanced than that: 1. This doesn't apply to actual VM interrupts, only to things which need to check for a pending interrupt. 2. EG(vm_interrupt) technically isn't related to the thing we care about, which is the interrupt count. So this adds ddog_php_prof_interrupt_function_unlikely which is the same as ddog_php_prof_interrupt_function at a high level, but it is optimized to assume that there isn't a pending interrupt (opposite of ddog_php_prof_interrupt_function). --- profiling/src/capi.rs | 4 +- profiling/src/wall_time.rs | 95 +++++++++++++++++++++----------- tests/tea/profiling/profiling.cc | 6 +- tests/tea/profiling/profiling.h | 1 + tracer/engine_hooks.c | 10 +++- 5 files changed, 78 insertions(+), 38 deletions(-) diff --git a/profiling/src/capi.rs b/profiling/src/capi.rs index 0ad6588fb3..a96ddf24fc 100644 --- a/profiling/src/capi.rs +++ b/profiling/src/capi.rs @@ -57,7 +57,9 @@ extern "C" fn ddog_php_prof_trigger_time_sample() { } } -pub use crate::wall_time::ddog_php_prof_interrupt_function; +pub use crate::wall_time::{ + ddog_php_prof_interrupt_function, ddog_php_prof_interrupt_function_unlikely, +}; #[cfg(test)] mod tests { diff --git a/profiling/src/wall_time.rs b/profiling/src/wall_time.rs index 3f0a948127..f1e3160fb6 100644 --- a/profiling/src/wall_time.rs +++ b/profiling/src/wall_time.rs @@ -2,7 +2,7 @@ //! implementation reasons, it has cpu-time code as well. use crate::bindings::{zend_execute_data, zend_interrupt_function, VmInterruptFn}; -use crate::{profiling::Profiler, RefCellExt, REQUEST_LOCALS}; +use crate::{profiling::Profiler, RefCellExt, RequestLocals, REQUEST_LOCALS}; use core::ptr; use log::debug; use std::sync::atomic::Ordering; @@ -79,8 +79,8 @@ mod execute_internal { unsafe { prev_execute_internal(execute_data, return_value) }; // See safety section of `execute_data_func_is_trampoline` docs for why - // the leaf frame is used instead of the execute_data ptr. - ddog_php_prof_interrupt_function(leaf_frame); + // the leaf frame is used instead of the execute_data ptr. + ddog_php_prof_interrupt_function_unlikely(leaf_frame); } /// # Safety @@ -109,31 +109,65 @@ static mut PREV_INTERRUPT_FUNCTION: Option = None; #[no_mangle] #[inline(never)] pub extern "C" fn ddog_php_prof_interrupt_function(execute_data: *mut zend_execute_data) { - let result = REQUEST_LOCALS.try_with_borrow(|locals| { - if !locals.system_settings().profiling_enabled { - return; - } + if let Err(err) = + REQUEST_LOCALS.try_with_borrow(|locals| interrupt_function(locals, execute_data)) + { + debug!("ddog_php_prof_interrupt_function failed to borrow request locals: {err}"); + } +} + +fn interrupt_function(locals: &RequestLocals, execute_data: *mut zend_execute_data) { + let profiling_disabled = !locals.system_settings().profiling_enabled; + + /* Other extensions/modules or the engine itself may trigger an + * interrupt, but given how expensive it is to gather a stack trace, + * it should only be done if we triggered it ourselves. So + * interrupt_count serves dual purposes: + * 1. Track how many interrupts there were. + * 2. Ensure we don't collect on someone else's interrupt. + */ + let interrupt_count = locals.interrupt_count.swap(0, Ordering::SeqCst); + if profiling_disabled | (interrupt_count == 0) { + return; + } + + if let Some(profiler) = Profiler::get() { + // Safety: execute_data was provided by the engine, and the profiler doesn't mutate it. + profiler.collect_time(execute_data, interrupt_count); + } +} - /* Other extensions/modules or the engine itself may trigger an - * interrupt, but given how expensive it is to gather a stack trace, - * it should only be done if we triggered it ourselves. So - * interrupt_count serves dual purposes: - * 1. Track how many interrupts there were. - * 2. Ensure we don't collect on someone else's interrupt. - */ - let interrupt_count = locals.interrupt_count.swap(0, Ordering::SeqCst); - if interrupt_count == 0 { +/// This function is like [`ddog_php_prof_interrupt_function`] except it's +/// optimized for the expectation that it's unlikely that there's actually an +/// interrupt actively needing to be handled. This is because we have to insert +/// this check in a variety of places, and in many of them (execute_internal, +/// frameless functions) there won't be an interrupt at all. +/// +/// # Safety +/// The zend_execute_data pointer should come from the engine to ensure it and +/// its sub-objects are valid. +#[no_mangle] +#[inline(never)] +pub extern "C" fn ddog_php_prof_interrupt_function_unlikely(execute_data: *mut zend_execute_data) { + let result = REQUEST_LOCALS.try_with_borrow(|locals| { + // Optimize here with the expectation there isn't an interrupt, because + // overwhelmingly, there will not be one. The relaxed load avoids the + // more expensive atomic read-modify-write in `interrupt_function` on + // the idle path. It is only a fast-path hint: `interrupt_function` does + // the authoritative swap. If another consumer clears the count between + // the load and swap, the swap simply observes zero; if a producer adds + // an interrupt after this load observes zero, the count remains pending + // and the corresponding VM interrupt will provide another opportunity + // to handle it. + if locals.interrupt_count.load(Ordering::Relaxed) == 0 { return; } - if let Some(profiler) = Profiler::get() { - // Safety: execute_data was provided by the engine, and the profiler doesn't mutate it. - profiler.collect_time(execute_data, interrupt_count); - } + interrupt_function(locals, execute_data); }); if let Err(err) = result { - debug!("ddog_php_prof_interrupt_function failed to borrow request locals: {err}"); + debug!("ddog_php_prof_interrupt_function_unlikely failed to borrow request locals: {err}"); } } @@ -144,7 +178,8 @@ mod frameless { use crate::bindings::{ zend_flf_functions, zend_flf_handlers, zend_frameless_function_info, }; - use crate::{profiling::Profiler, zend, RefCellExt, REQUEST_LOCALS}; + use crate::wall_time::ddog_php_prof_interrupt_function; + use crate::{zend, RefCellExt, REQUEST_LOCALS}; use dynasmrt::{dynasm, DynasmApi, ExecutableBuffer}; use log::error; use std::ffi::c_void; @@ -271,23 +306,17 @@ mod frameless { #[inline(never)] pub extern "C" fn ddog_php_prof_icall_trampoline_target() { let interrupt_count = REQUEST_LOCALS - .try_with_borrow(|locals| { - if !locals.system_settings().profiling_enabled { - return 0; - } - locals.interrupt_count.swap(0, Ordering::SeqCst) - }) + .try_with_borrow(|locals| locals.interrupt_count.load(Ordering::Relaxed)) .unwrap_or(0); if interrupt_count == 0 { return; } - if let Some(profiler) = Profiler::get() { - // SAFETY: profiler doesn't mutate execute_data - let execute_data = unsafe { zend::ddog_php_prof_get_current_execute_data() }; - profiler.collect_time(execute_data, interrupt_count); - } + // Fetching the execute data is intentionally delayed until we know + // that the interrupt count is greater than 0 for perf. + let execute_data = unsafe { zend::ddog_php_prof_get_current_execute_data() }; + ddog_php_prof_interrupt_function(execute_data); } } diff --git a/tests/tea/profiling/profiling.cc b/tests/tea/profiling/profiling.cc index 8c6e361427..7de7779530 100644 --- a/tests/tea/profiling/profiling.cc +++ b/tests/tea/profiling/profiling.cc @@ -9,7 +9,7 @@ ZEND_TLS datadog_php_stack_sample last_stack_sample; ZEND_API datadog_php_stack_sample tea_get_last_stack_sample(void) { return last_stack_sample; } -ZEND_API void ddog_php_prof_interrupt_function(zend_execute_data *execute_data) { +ZEND_API void ddog_php_prof_interrupt_function_unlikely(zend_execute_data *execute_data) { datadog_php_stack_sample_ctor(&last_stack_sample); /* Don't try to re-implement everything. Remember, the tracer is being @@ -34,6 +34,10 @@ ZEND_API void ddog_php_prof_interrupt_function(zend_execute_data *execute_data) } } +ZEND_API void ddog_php_prof_interrupt_function(zend_execute_data *execute_data) { + ddog_php_prof_interrupt_function_unlikely(execute_data); +} + ZEND_API zend_extension_version_info extension_version_info = { ZEND_EXTENSION_API_NO, ZEND_EXTENSION_BUILD_ID, diff --git a/tests/tea/profiling/profiling.h b/tests/tea/profiling/profiling.h index 75f8aa6aa8..452476071f 100644 --- a/tests/tea/profiling/profiling.h +++ b/tests/tea/profiling/profiling.h @@ -10,6 +10,7 @@ BEGIN_EXTERN_C() ZEND_API datadog_php_stack_sample tea_get_last_stack_sample(void); ZEND_API void ddog_php_prof_interrupt_function(zend_execute_data *execute_data); +ZEND_API void ddog_php_prof_interrupt_function_unlikely(zend_execute_data *execute_data); END_EXTERN_C() #endif diff --git a/tracer/engine_hooks.c b/tracer/engine_hooks.c index e2b2eaea97..bbf9e16cb9 100644 --- a/tracer/engine_hooks.c +++ b/tracer/engine_hooks.c @@ -41,10 +41,14 @@ void dd_search_for_profiling_symbols(void *arg) { if (extension->name && strcmp(extension->name, "datadog-profiling") == 0) { DL_HANDLE handle = extension->handle; - profiling_interrupt_function = (void(*)(zend_execute_data *))DL_FETCH_SYMBOL(handle, "ddog_php_prof_interrupt_function"); + profiling_interrupt_function = (void(*)(zend_execute_data *))DL_FETCH_SYMBOL(handle, "ddog_php_prof_interrupt_function_unlikely"); + if (!profiling_interrupt_function) { + // Fall back for compatibility with profiler versions from before the + // unlikely-pending fast path was exported. + profiling_interrupt_function = (void(*)(zend_execute_data *))DL_FETCH_SYMBOL(handle, "ddog_php_prof_interrupt_function"); + } if (UNEXPECTED(!profiling_interrupt_function)) { - LOG(WARN, "[Datadog Trace] Profiling was detected, but locating symbol %s failed: %s\n", "ddog_php_prof_interrupt_function", - GET_DL_ERROR()); + LOG(WARN, "[Datadog Trace] Profiling was detected, but locating an interrupt function failed: %s\n", GET_DL_ERROR()); } profiling_notify_trace_finished = (profiling_notify_trace_finished_t)DL_FETCH_SYMBOL(handle, "datadog_profiling_notify_trace_finished"); From 32abe928af2f1a804426a975fc77c589ecc92955 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Tue, 28 Jul 2026 19:41:36 -0600 Subject: [PATCH 19/25] perf(profiling): move interrupt_count to module globals And use system settings from the global, rather than through the REQUEST_LOCALS. --- profiling/src/allocation/mod.rs | 7 +-- profiling/src/capi.rs | 5 +- profiling/src/lib.rs | 12 +++-- profiling/src/module_globals.rs | 56 ++++++++++++++++++---- profiling/src/wall_time.rs | 82 ++++++++++++++++----------------- 5 files changed, 105 insertions(+), 57 deletions(-) diff --git a/profiling/src/allocation/mod.rs b/profiling/src/allocation/mod.rs index f366d6a1cb..51ecac48db 100644 --- a/profiling/src/allocation/mod.rs +++ b/profiling/src/allocation/mod.rs @@ -199,9 +199,10 @@ pub fn collect_allocation(ptr: *mut c_void, len: size_t) { // Check if there's a pending time interrupt that we can handle now // instead of waiting for an interrupt handler. This is slightly more // accurate and efficient, win-win. - let interrupt_count = REQUEST_LOCALS - .try_with_borrow(|locals| locals.interrupt_count.swap(0, Ordering::SeqCst)) - .unwrap_or(0); + // SAFETY: allocation samples are collected on an initialized PHP request thread. + let globals = unsafe { module_globals::get_profiler_globals() }; + // SAFETY: the current thread's module globals are valid through GSHUTDOWN. + let interrupt_count = unsafe { (*globals).interrupt_count.swap(0, Ordering::SeqCst) }; // SAFETY: execute_data was provided by the engine, and the profiler // doesn't mutate it. diff --git a/profiling/src/capi.rs b/profiling/src/capi.rs index a96ddf24fc..932b6d530d 100644 --- a/profiling/src/capi.rs +++ b/profiling/src/capi.rs @@ -46,7 +46,10 @@ extern "C" fn ddog_php_prof_trigger_time_sample() { if locals.system_settings().profiling_enabled { // Safety: only vm interrupts are stored there, or possibly null (edges only). if let Some(vm_interrupt) = unsafe { locals.vm_interrupt_addr.as_ref() } { - locals.interrupt_count.fetch_add(1, Ordering::SeqCst); + // SAFETY: this callback runs on an initialized PHP request thread. + let globals = unsafe { crate::module_globals::get_profiler_globals() }; + // SAFETY: the current thread's module globals are valid through GSHUTDOWN. + unsafe { (*globals).interrupt_count.fetch_add(1, Ordering::SeqCst) }; vm_interrupt.store(true, Ordering::SeqCst); } } diff --git a/profiling/src/lib.rs b/profiling/src/lib.rs index e327b25ff5..f815f1514c 100644 --- a/profiling/src/lib.rs +++ b/profiling/src/lib.rs @@ -425,7 +425,6 @@ pub struct RequestLocals { pub system_settings: ptr::NonNull, pub profiling_experimental_heap_live_enabled: bool, - pub interrupt_count: AtomicU32, pub vm_interrupt_addr: *const AtomicBool, } @@ -450,7 +449,6 @@ impl Default for RequestLocals { tags: vec![], system_settings: SystemSettings::get(), profiling_experimental_heap_live_enabled: false, - interrupt_count: AtomicU32::new(0), vm_interrupt_addr: ptr::null_mut(), } } @@ -738,8 +736,11 @@ extern "C" fn rinit(_type: c_int, _module_number: c_int) -> ZendResult { } if let Some(profiler) = Profiler::get() { + // SAFETY: PHP module globals are initialized for this request thread. + let globals = unsafe { module_globals::get_profiler_globals() }; let interrupt = VmInterrupt { - interrupt_count_ptr: &locals.interrupt_count as *const AtomicU32, + // SAFETY: `globals` is valid until this thread's GSHUTDOWN. + interrupt_count_ptr: unsafe { ptr::addr_of!((*globals).interrupt_count) }, engine_ptr: locals.vm_interrupt_addr, }; profiler.add_interrupt(interrupt); @@ -795,8 +796,11 @@ extern "C" fn rshutdown(_type: c_int, _module_number: c_int) -> ZendResult { // and we don't need to optimize for that. if system_settings.profiling_enabled { if let Some(profiler) = Profiler::get() { + // SAFETY: PHP module globals remain initialized through RSHUTDOWN. + let globals = unsafe { module_globals::get_profiler_globals() }; let interrupt = VmInterrupt { - interrupt_count_ptr: &locals.interrupt_count, + // SAFETY: `globals` remains valid until this thread's GSHUTDOWN. + interrupt_count_ptr: unsafe { ptr::addr_of!((*globals).interrupt_count) }, engine_ptr: locals.vm_interrupt_addr, }; profiler.remove_interrupt(interrupt); diff --git a/profiling/src/module_globals.rs b/profiling/src/module_globals.rs index 85c938e535..470b6a08f4 100644 --- a/profiling/src/module_globals.rs +++ b/profiling/src/module_globals.rs @@ -2,6 +2,7 @@ use crate::allocation; use core::cell::Cell; use core::ffi::c_void; use core::ptr; +use core::sync::atomic::AtomicU32; #[cfg(php_zend_mm_set_custom_handlers_ex)] use crate::allocation::allocation_ge84::ZendMMState; @@ -13,6 +14,12 @@ pub struct ProfilerGlobals { /// Wrapped in `Cell` to prevent torn reads/writes when allocation hooks /// are called re-entrantly during `rinit()`/`rshutdown()`. pub zend_mm_state: Cell, + /// Number of profiler time interrupts pending for this PHP thread. + /// + /// The profiler timer thread updates this through a pointer registered by + /// the PHP thread, so the value must remain atomic despite living in + /// thread-local PHP module globals. + pub interrupt_count: AtomicU32, } /// We need TSRM to call into GINIT and GSHUTDOWN to observe spawning and @@ -29,8 +36,17 @@ pub static mut GLOBALS_ID: i32 = 0; #[cfg(not(php_zts))] pub static mut GLOBALS: ProfilerGlobals = ProfilerGlobals { zend_mm_state: Cell::new(ZendMMState::new()), + interrupt_count: AtomicU32::new(0), }; +// Unit tests are not loaded by PHP, so provide the TSRM symbol needed to link +// tests that retain the ZTS module-global accessors. +#[cfg(all(test, php_zts))] +#[no_mangle] +unsafe extern "C" fn tsrm_get_ls_cache() -> *mut c_void { + ptr::null_mut() +} + #[cfg(php_zts)] mod zts { use core::ffi::c_void; @@ -40,9 +56,13 @@ mod zts { } #[inline] - pub unsafe fn tsrmg_bulk(id: i32) -> *mut c_void { - let tls = tsrm_get_ls_cache() as *mut *mut *mut c_void; - let storage = *tls; // void** storage + pub unsafe fn get_ls_cache() -> *mut c_void { + tsrm_get_ls_cache() + } + + #[inline] + pub unsafe fn tsrmg_bulk(ls_cache: *mut c_void, id: i32) -> *mut c_void { + let storage = *(ls_cache as *mut *mut *mut c_void); // void** storage // TSRM_UNSHUFFLE_RSRC_ID(id) is just `id - 1`. let idx = (id - 1) as usize; @@ -51,6 +71,27 @@ mod zts { } } +#[cfg(php_zts)] +#[inline] +pub unsafe fn get_tsrm_ls_cache() -> *mut c_void { + zts::get_ls_cache() +} + +#[cfg(php_zts)] +#[inline] +pub unsafe fn get_tsrm_resource_from_cache(ls_cache: *mut c_void, id: i32) -> *mut c_void { + zts::tsrmg_bulk(ls_cache, id) +} + +#[cfg(php_zts)] +#[inline] +pub unsafe fn get_profiler_globals_from_cache(ls_cache: *mut c_void) -> *mut ProfilerGlobals { + // SAFETY: As long as this is called during the times documented by + // get_profiler_globals(), GLOBALS_ID will be set by PHP. + let id = ptr::addr_of!(GLOBALS_ID).read(); + get_tsrm_resource_from_cache(ls_cache, id).cast() +} + /// Returns a pointer to the profiler globals for the current thread. /// /// # Safety @@ -64,10 +105,7 @@ mod zts { pub unsafe fn get_profiler_globals() -> *mut ProfilerGlobals { #[cfg(php_zts)] { - // SAFETY: As long as this is called during the times documented by - // our own safety requirements, GLOBALS_ID will be set by PHP. - let id = ptr::addr_of!(GLOBALS_ID).read(); - zts::tsrmg_bulk(id).cast() + get_profiler_globals_from_cache(get_tsrm_ls_cache()) } #[cfg(not(php_zts))] @@ -85,11 +123,13 @@ pub unsafe extern "C" fn ginit(_globals_ptr: *mut c_void) { #[cfg(php_zts)] crate::timeline::timeline_ginit(); + let globals = _globals_ptr.cast::(); + (*globals).interrupt_count = AtomicU32::new(0); + // Initialize ZendMMState in PHP globals for ZTS builds. For NTS builds, // this was already done in its const initializer. #[cfg(php_zts)] { - let globals = _globals_ptr.cast::(); (*globals).zend_mm_state = Cell::new(ZendMMState::new()); } diff --git a/profiling/src/wall_time.rs b/profiling/src/wall_time.rs index f1e3160fb6..a698b7ab9a 100644 --- a/profiling/src/wall_time.rs +++ b/profiling/src/wall_time.rs @@ -2,9 +2,10 @@ //! implementation reasons, it has cpu-time code as well. use crate::bindings::{zend_execute_data, zend_interrupt_function, VmInterruptFn}; -use crate::{profiling::Profiler, RefCellExt, RequestLocals, REQUEST_LOCALS}; +use crate::config::SystemSettings; +use crate::module_globals::{self, ProfilerGlobals}; +use crate::profiling::Profiler; use core::ptr; -use log::debug; use std::sync::atomic::Ordering; #[cfg(not(php_frameless))] @@ -109,16 +110,12 @@ static mut PREV_INTERRUPT_FUNCTION: Option = None; #[no_mangle] #[inline(never)] pub extern "C" fn ddog_php_prof_interrupt_function(execute_data: *mut zend_execute_data) { - if let Err(err) = - REQUEST_LOCALS.try_with_borrow(|locals| interrupt_function(locals, execute_data)) - { - debug!("ddog_php_prof_interrupt_function failed to borrow request locals: {err}"); - } + // SAFETY: interrupt callbacks run while the current PHP thread's module globals are valid. + let globals = unsafe { &*module_globals::get_profiler_globals() }; + interrupt_function(globals, execute_data); } -fn interrupt_function(locals: &RequestLocals, execute_data: *mut zend_execute_data) { - let profiling_disabled = !locals.system_settings().profiling_enabled; - +fn interrupt_function(globals: &ProfilerGlobals, execute_data: *mut zend_execute_data) { /* Other extensions/modules or the engine itself may trigger an * interrupt, but given how expensive it is to gather a stack trace, * it should only be done if we triggered it ourselves. So @@ -126,8 +123,15 @@ fn interrupt_function(locals: &RequestLocals, execute_data: *mut zend_execute_da * 1. Track how many interrupts there were. * 2. Ensure we don't collect on someone else's interrupt. */ - let interrupt_count = locals.interrupt_count.swap(0, Ordering::SeqCst); - if profiling_disabled | (interrupt_count == 0) { + let interrupt_count = globals.interrupt_count.swap(0, Ordering::SeqCst); + if interrupt_count == 0 { + return; + } + + // SAFETY: `SystemSettings::get()` points to an initialized process + // lifetime static. Lifecycle updates happen in synchronized startup, fork, + // or shutdown phases, and this reference is short-lived. + if !unsafe { SystemSettings::get().as_ref() }.profiling_enabled { return; } @@ -149,26 +153,23 @@ fn interrupt_function(locals: &RequestLocals, execute_data: *mut zend_execute_da #[no_mangle] #[inline(never)] pub extern "C" fn ddog_php_prof_interrupt_function_unlikely(execute_data: *mut zend_execute_data) { - let result = REQUEST_LOCALS.try_with_borrow(|locals| { - // Optimize here with the expectation there isn't an interrupt, because - // overwhelmingly, there will not be one. The relaxed load avoids the - // more expensive atomic read-modify-write in `interrupt_function` on - // the idle path. It is only a fast-path hint: `interrupt_function` does - // the authoritative swap. If another consumer clears the count between - // the load and swap, the swap simply observes zero; if a producer adds - // an interrupt after this load observes zero, the count remains pending - // and the corresponding VM interrupt will provide another opportunity - // to handle it. - if locals.interrupt_count.load(Ordering::Relaxed) == 0 { - return; - } - - interrupt_function(locals, execute_data); - }); - - if let Err(err) = result { - debug!("ddog_php_prof_interrupt_function_unlikely failed to borrow request locals: {err}"); + // SAFETY: interrupt checks run while the current PHP thread's module globals are valid. + let globals = unsafe { &*module_globals::get_profiler_globals() }; + + // Optimize here with the expectation there isn't an interrupt, because + // overwhelmingly, there will not be one. The relaxed load avoids the + // more expensive atomic read-modify-write in `interrupt_function` on + // the idle path. It is only a fast-path hint: `interrupt_function` does + // the authoritative swap. If another consumer clears the count between + // the load and swap, the swap simply observes zero; if a producer adds + // an interrupt after this load observes zero, the count remains pending + // and the corresponding VM interrupt will provide another opportunity + // to handle it. + if globals.interrupt_count.load(Ordering::Relaxed) == 0 { + return; } + + interrupt_function(globals, execute_data); } #[cfg(php_frameless)] @@ -178,8 +179,9 @@ mod frameless { use crate::bindings::{ zend_flf_functions, zend_flf_handlers, zend_frameless_function_info, }; - use crate::wall_time::ddog_php_prof_interrupt_function; - use crate::{zend, RefCellExt, REQUEST_LOCALS}; + use crate::module_globals; + use crate::wall_time::interrupt_function; + use crate::zend; use dynasmrt::{dynasm, DynasmApi, ExecutableBuffer}; use log::error; use std::ffi::c_void; @@ -305,18 +307,16 @@ mod frameless { #[no_mangle] #[inline(never)] pub extern "C" fn ddog_php_prof_icall_trampoline_target() { - let interrupt_count = REQUEST_LOCALS - .try_with_borrow(|locals| locals.interrupt_count.load(Ordering::Relaxed)) - .unwrap_or(0); - - if interrupt_count == 0 { + // SAFETY: frameless handlers run while the current PHP thread's module globals are + // valid. Retain the pointer so the authoritative swap reuses the same TSRM lookup. + let globals = unsafe { &*module_globals::get_profiler_globals() }; + if globals.interrupt_count.load(Ordering::Relaxed) == 0 { return; } - // Fetching the execute data is intentionally delayed until we know - // that the interrupt count is greater than 0 for perf. + // Fetching execute data is intentionally delayed until a profiler interrupt is pending. let execute_data = unsafe { zend::ddog_php_prof_get_current_execute_data() }; - ddog_php_prof_interrupt_function(execute_data); + interrupt_function(globals, execute_data); } } From a8016a970125ee12c04d51d171d98318d80e6beb Mon Sep 17 00:00:00 2001 From: Florian Engelhardt Date: Wed, 29 Jul 2026 08:02:48 +0200 Subject: [PATCH 20/25] fix(profiling): allow legacy executor globals on macOS --- profiling/build.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/profiling/build.rs b/profiling/build.rs index cf88c39856..0047272333 100644 --- a/profiling/build.rs +++ b/profiling/build.rs @@ -633,6 +633,7 @@ fn apple_linker_flags() { "_sapi_module", // TSRM (ZTS builds only; harmless to list on NTS — they simply // won't appear as undefined) + "_executor_globals_id", "_tsrm_get_ls_cache", "_tsrm_set_new_thread_end_handler", // ZTS globals offsets (replace direct globals on ZTS) From d9ea1f85f49b0f47056701e17cde409b6aa2e3cd Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 29 Jul 2026 09:26:50 -0600 Subject: [PATCH 21/25] Revert "perf(profiling): move interrupt_count to module globals" This reverts commit 32abe928af2f1a804426a975fc77c589ecc92955. --- profiling/src/allocation/mod.rs | 7 ++- profiling/src/capi.rs | 5 +- profiling/src/lib.rs | 12 ++--- profiling/src/module_globals.rs | 56 ++++------------------ profiling/src/wall_time.rs | 82 ++++++++++++++++----------------- 5 files changed, 57 insertions(+), 105 deletions(-) diff --git a/profiling/src/allocation/mod.rs b/profiling/src/allocation/mod.rs index 51ecac48db..f366d6a1cb 100644 --- a/profiling/src/allocation/mod.rs +++ b/profiling/src/allocation/mod.rs @@ -199,10 +199,9 @@ pub fn collect_allocation(ptr: *mut c_void, len: size_t) { // Check if there's a pending time interrupt that we can handle now // instead of waiting for an interrupt handler. This is slightly more // accurate and efficient, win-win. - // SAFETY: allocation samples are collected on an initialized PHP request thread. - let globals = unsafe { module_globals::get_profiler_globals() }; - // SAFETY: the current thread's module globals are valid through GSHUTDOWN. - let interrupt_count = unsafe { (*globals).interrupt_count.swap(0, Ordering::SeqCst) }; + let interrupt_count = REQUEST_LOCALS + .try_with_borrow(|locals| locals.interrupt_count.swap(0, Ordering::SeqCst)) + .unwrap_or(0); // SAFETY: execute_data was provided by the engine, and the profiler // doesn't mutate it. diff --git a/profiling/src/capi.rs b/profiling/src/capi.rs index 932b6d530d..a96ddf24fc 100644 --- a/profiling/src/capi.rs +++ b/profiling/src/capi.rs @@ -46,10 +46,7 @@ extern "C" fn ddog_php_prof_trigger_time_sample() { if locals.system_settings().profiling_enabled { // Safety: only vm interrupts are stored there, or possibly null (edges only). if let Some(vm_interrupt) = unsafe { locals.vm_interrupt_addr.as_ref() } { - // SAFETY: this callback runs on an initialized PHP request thread. - let globals = unsafe { crate::module_globals::get_profiler_globals() }; - // SAFETY: the current thread's module globals are valid through GSHUTDOWN. - unsafe { (*globals).interrupt_count.fetch_add(1, Ordering::SeqCst) }; + locals.interrupt_count.fetch_add(1, Ordering::SeqCst); vm_interrupt.store(true, Ordering::SeqCst); } } diff --git a/profiling/src/lib.rs b/profiling/src/lib.rs index f815f1514c..e327b25ff5 100644 --- a/profiling/src/lib.rs +++ b/profiling/src/lib.rs @@ -425,6 +425,7 @@ pub struct RequestLocals { pub system_settings: ptr::NonNull, pub profiling_experimental_heap_live_enabled: bool, + pub interrupt_count: AtomicU32, pub vm_interrupt_addr: *const AtomicBool, } @@ -449,6 +450,7 @@ impl Default for RequestLocals { tags: vec![], system_settings: SystemSettings::get(), profiling_experimental_heap_live_enabled: false, + interrupt_count: AtomicU32::new(0), vm_interrupt_addr: ptr::null_mut(), } } @@ -736,11 +738,8 @@ extern "C" fn rinit(_type: c_int, _module_number: c_int) -> ZendResult { } if let Some(profiler) = Profiler::get() { - // SAFETY: PHP module globals are initialized for this request thread. - let globals = unsafe { module_globals::get_profiler_globals() }; let interrupt = VmInterrupt { - // SAFETY: `globals` is valid until this thread's GSHUTDOWN. - interrupt_count_ptr: unsafe { ptr::addr_of!((*globals).interrupt_count) }, + interrupt_count_ptr: &locals.interrupt_count as *const AtomicU32, engine_ptr: locals.vm_interrupt_addr, }; profiler.add_interrupt(interrupt); @@ -796,11 +795,8 @@ extern "C" fn rshutdown(_type: c_int, _module_number: c_int) -> ZendResult { // and we don't need to optimize for that. if system_settings.profiling_enabled { if let Some(profiler) = Profiler::get() { - // SAFETY: PHP module globals remain initialized through RSHUTDOWN. - let globals = unsafe { module_globals::get_profiler_globals() }; let interrupt = VmInterrupt { - // SAFETY: `globals` remains valid until this thread's GSHUTDOWN. - interrupt_count_ptr: unsafe { ptr::addr_of!((*globals).interrupt_count) }, + interrupt_count_ptr: &locals.interrupt_count, engine_ptr: locals.vm_interrupt_addr, }; profiler.remove_interrupt(interrupt); diff --git a/profiling/src/module_globals.rs b/profiling/src/module_globals.rs index 470b6a08f4..85c938e535 100644 --- a/profiling/src/module_globals.rs +++ b/profiling/src/module_globals.rs @@ -2,7 +2,6 @@ use crate::allocation; use core::cell::Cell; use core::ffi::c_void; use core::ptr; -use core::sync::atomic::AtomicU32; #[cfg(php_zend_mm_set_custom_handlers_ex)] use crate::allocation::allocation_ge84::ZendMMState; @@ -14,12 +13,6 @@ pub struct ProfilerGlobals { /// Wrapped in `Cell` to prevent torn reads/writes when allocation hooks /// are called re-entrantly during `rinit()`/`rshutdown()`. pub zend_mm_state: Cell, - /// Number of profiler time interrupts pending for this PHP thread. - /// - /// The profiler timer thread updates this through a pointer registered by - /// the PHP thread, so the value must remain atomic despite living in - /// thread-local PHP module globals. - pub interrupt_count: AtomicU32, } /// We need TSRM to call into GINIT and GSHUTDOWN to observe spawning and @@ -36,17 +29,8 @@ pub static mut GLOBALS_ID: i32 = 0; #[cfg(not(php_zts))] pub static mut GLOBALS: ProfilerGlobals = ProfilerGlobals { zend_mm_state: Cell::new(ZendMMState::new()), - interrupt_count: AtomicU32::new(0), }; -// Unit tests are not loaded by PHP, so provide the TSRM symbol needed to link -// tests that retain the ZTS module-global accessors. -#[cfg(all(test, php_zts))] -#[no_mangle] -unsafe extern "C" fn tsrm_get_ls_cache() -> *mut c_void { - ptr::null_mut() -} - #[cfg(php_zts)] mod zts { use core::ffi::c_void; @@ -56,13 +40,9 @@ mod zts { } #[inline] - pub unsafe fn get_ls_cache() -> *mut c_void { - tsrm_get_ls_cache() - } - - #[inline] - pub unsafe fn tsrmg_bulk(ls_cache: *mut c_void, id: i32) -> *mut c_void { - let storage = *(ls_cache as *mut *mut *mut c_void); // void** storage + pub unsafe fn tsrmg_bulk(id: i32) -> *mut c_void { + let tls = tsrm_get_ls_cache() as *mut *mut *mut c_void; + let storage = *tls; // void** storage // TSRM_UNSHUFFLE_RSRC_ID(id) is just `id - 1`. let idx = (id - 1) as usize; @@ -71,27 +51,6 @@ mod zts { } } -#[cfg(php_zts)] -#[inline] -pub unsafe fn get_tsrm_ls_cache() -> *mut c_void { - zts::get_ls_cache() -} - -#[cfg(php_zts)] -#[inline] -pub unsafe fn get_tsrm_resource_from_cache(ls_cache: *mut c_void, id: i32) -> *mut c_void { - zts::tsrmg_bulk(ls_cache, id) -} - -#[cfg(php_zts)] -#[inline] -pub unsafe fn get_profiler_globals_from_cache(ls_cache: *mut c_void) -> *mut ProfilerGlobals { - // SAFETY: As long as this is called during the times documented by - // get_profiler_globals(), GLOBALS_ID will be set by PHP. - let id = ptr::addr_of!(GLOBALS_ID).read(); - get_tsrm_resource_from_cache(ls_cache, id).cast() -} - /// Returns a pointer to the profiler globals for the current thread. /// /// # Safety @@ -105,7 +64,10 @@ pub unsafe fn get_profiler_globals_from_cache(ls_cache: *mut c_void) -> *mut Pro pub unsafe fn get_profiler_globals() -> *mut ProfilerGlobals { #[cfg(php_zts)] { - get_profiler_globals_from_cache(get_tsrm_ls_cache()) + // SAFETY: As long as this is called during the times documented by + // our own safety requirements, GLOBALS_ID will be set by PHP. + let id = ptr::addr_of!(GLOBALS_ID).read(); + zts::tsrmg_bulk(id).cast() } #[cfg(not(php_zts))] @@ -123,13 +85,11 @@ pub unsafe extern "C" fn ginit(_globals_ptr: *mut c_void) { #[cfg(php_zts)] crate::timeline::timeline_ginit(); - let globals = _globals_ptr.cast::(); - (*globals).interrupt_count = AtomicU32::new(0); - // Initialize ZendMMState in PHP globals for ZTS builds. For NTS builds, // this was already done in its const initializer. #[cfg(php_zts)] { + let globals = _globals_ptr.cast::(); (*globals).zend_mm_state = Cell::new(ZendMMState::new()); } diff --git a/profiling/src/wall_time.rs b/profiling/src/wall_time.rs index a698b7ab9a..f1e3160fb6 100644 --- a/profiling/src/wall_time.rs +++ b/profiling/src/wall_time.rs @@ -2,10 +2,9 @@ //! implementation reasons, it has cpu-time code as well. use crate::bindings::{zend_execute_data, zend_interrupt_function, VmInterruptFn}; -use crate::config::SystemSettings; -use crate::module_globals::{self, ProfilerGlobals}; -use crate::profiling::Profiler; +use crate::{profiling::Profiler, RefCellExt, RequestLocals, REQUEST_LOCALS}; use core::ptr; +use log::debug; use std::sync::atomic::Ordering; #[cfg(not(php_frameless))] @@ -110,12 +109,16 @@ static mut PREV_INTERRUPT_FUNCTION: Option = None; #[no_mangle] #[inline(never)] pub extern "C" fn ddog_php_prof_interrupt_function(execute_data: *mut zend_execute_data) { - // SAFETY: interrupt callbacks run while the current PHP thread's module globals are valid. - let globals = unsafe { &*module_globals::get_profiler_globals() }; - interrupt_function(globals, execute_data); + if let Err(err) = + REQUEST_LOCALS.try_with_borrow(|locals| interrupt_function(locals, execute_data)) + { + debug!("ddog_php_prof_interrupt_function failed to borrow request locals: {err}"); + } } -fn interrupt_function(globals: &ProfilerGlobals, execute_data: *mut zend_execute_data) { +fn interrupt_function(locals: &RequestLocals, execute_data: *mut zend_execute_data) { + let profiling_disabled = !locals.system_settings().profiling_enabled; + /* Other extensions/modules or the engine itself may trigger an * interrupt, but given how expensive it is to gather a stack trace, * it should only be done if we triggered it ourselves. So @@ -123,15 +126,8 @@ fn interrupt_function(globals: &ProfilerGlobals, execute_data: *mut zend_execute * 1. Track how many interrupts there were. * 2. Ensure we don't collect on someone else's interrupt. */ - let interrupt_count = globals.interrupt_count.swap(0, Ordering::SeqCst); - if interrupt_count == 0 { - return; - } - - // SAFETY: `SystemSettings::get()` points to an initialized process - // lifetime static. Lifecycle updates happen in synchronized startup, fork, - // or shutdown phases, and this reference is short-lived. - if !unsafe { SystemSettings::get().as_ref() }.profiling_enabled { + let interrupt_count = locals.interrupt_count.swap(0, Ordering::SeqCst); + if profiling_disabled | (interrupt_count == 0) { return; } @@ -153,23 +149,26 @@ fn interrupt_function(globals: &ProfilerGlobals, execute_data: *mut zend_execute #[no_mangle] #[inline(never)] pub extern "C" fn ddog_php_prof_interrupt_function_unlikely(execute_data: *mut zend_execute_data) { - // SAFETY: interrupt checks run while the current PHP thread's module globals are valid. - let globals = unsafe { &*module_globals::get_profiler_globals() }; - - // Optimize here with the expectation there isn't an interrupt, because - // overwhelmingly, there will not be one. The relaxed load avoids the - // more expensive atomic read-modify-write in `interrupt_function` on - // the idle path. It is only a fast-path hint: `interrupt_function` does - // the authoritative swap. If another consumer clears the count between - // the load and swap, the swap simply observes zero; if a producer adds - // an interrupt after this load observes zero, the count remains pending - // and the corresponding VM interrupt will provide another opportunity - // to handle it. - if globals.interrupt_count.load(Ordering::Relaxed) == 0 { - return; - } + let result = REQUEST_LOCALS.try_with_borrow(|locals| { + // Optimize here with the expectation there isn't an interrupt, because + // overwhelmingly, there will not be one. The relaxed load avoids the + // more expensive atomic read-modify-write in `interrupt_function` on + // the idle path. It is only a fast-path hint: `interrupt_function` does + // the authoritative swap. If another consumer clears the count between + // the load and swap, the swap simply observes zero; if a producer adds + // an interrupt after this load observes zero, the count remains pending + // and the corresponding VM interrupt will provide another opportunity + // to handle it. + if locals.interrupt_count.load(Ordering::Relaxed) == 0 { + return; + } - interrupt_function(globals, execute_data); + interrupt_function(locals, execute_data); + }); + + if let Err(err) = result { + debug!("ddog_php_prof_interrupt_function_unlikely failed to borrow request locals: {err}"); + } } #[cfg(php_frameless)] @@ -179,9 +178,8 @@ mod frameless { use crate::bindings::{ zend_flf_functions, zend_flf_handlers, zend_frameless_function_info, }; - use crate::module_globals; - use crate::wall_time::interrupt_function; - use crate::zend; + use crate::wall_time::ddog_php_prof_interrupt_function; + use crate::{zend, RefCellExt, REQUEST_LOCALS}; use dynasmrt::{dynasm, DynasmApi, ExecutableBuffer}; use log::error; use std::ffi::c_void; @@ -307,16 +305,18 @@ mod frameless { #[no_mangle] #[inline(never)] pub extern "C" fn ddog_php_prof_icall_trampoline_target() { - // SAFETY: frameless handlers run while the current PHP thread's module globals are - // valid. Retain the pointer so the authoritative swap reuses the same TSRM lookup. - let globals = unsafe { &*module_globals::get_profiler_globals() }; - if globals.interrupt_count.load(Ordering::Relaxed) == 0 { + let interrupt_count = REQUEST_LOCALS + .try_with_borrow(|locals| locals.interrupt_count.load(Ordering::Relaxed)) + .unwrap_or(0); + + if interrupt_count == 0 { return; } - // Fetching execute data is intentionally delayed until a profiler interrupt is pending. + // Fetching the execute data is intentionally delayed until we know + // that the interrupt count is greater than 0 for perf. let execute_data = unsafe { zend::ddog_php_prof_get_current_execute_data() }; - interrupt_function(globals, execute_data); + ddog_php_prof_interrupt_function(execute_data); } } From f0dde0818c356fde0a77ac4e1bda187c36f0b3d8 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 29 Jul 2026 09:27:32 -0600 Subject: [PATCH 22/25] Revert "perf(profiling): use relaxed read to skip idle interrupts" This reverts commit 8cb3cde012b27d0ea07cfd3d7b3cac23d19b9faa. --- profiling/src/capi.rs | 4 +- profiling/src/wall_time.rs | 95 +++++++++++--------------------- tests/tea/profiling/profiling.cc | 6 +- tests/tea/profiling/profiling.h | 1 - tracer/engine_hooks.c | 10 +--- 5 files changed, 38 insertions(+), 78 deletions(-) diff --git a/profiling/src/capi.rs b/profiling/src/capi.rs index a96ddf24fc..0ad6588fb3 100644 --- a/profiling/src/capi.rs +++ b/profiling/src/capi.rs @@ -57,9 +57,7 @@ extern "C" fn ddog_php_prof_trigger_time_sample() { } } -pub use crate::wall_time::{ - ddog_php_prof_interrupt_function, ddog_php_prof_interrupt_function_unlikely, -}; +pub use crate::wall_time::ddog_php_prof_interrupt_function; #[cfg(test)] mod tests { diff --git a/profiling/src/wall_time.rs b/profiling/src/wall_time.rs index f1e3160fb6..3f0a948127 100644 --- a/profiling/src/wall_time.rs +++ b/profiling/src/wall_time.rs @@ -2,7 +2,7 @@ //! implementation reasons, it has cpu-time code as well. use crate::bindings::{zend_execute_data, zend_interrupt_function, VmInterruptFn}; -use crate::{profiling::Profiler, RefCellExt, RequestLocals, REQUEST_LOCALS}; +use crate::{profiling::Profiler, RefCellExt, REQUEST_LOCALS}; use core::ptr; use log::debug; use std::sync::atomic::Ordering; @@ -79,8 +79,8 @@ mod execute_internal { unsafe { prev_execute_internal(execute_data, return_value) }; // See safety section of `execute_data_func_is_trampoline` docs for why - // the leaf frame is used instead of the execute_data ptr. - ddog_php_prof_interrupt_function_unlikely(leaf_frame); + // the leaf frame is used instead of the execute_data ptr. + ddog_php_prof_interrupt_function(leaf_frame); } /// # Safety @@ -109,65 +109,31 @@ static mut PREV_INTERRUPT_FUNCTION: Option = None; #[no_mangle] #[inline(never)] pub extern "C" fn ddog_php_prof_interrupt_function(execute_data: *mut zend_execute_data) { - if let Err(err) = - REQUEST_LOCALS.try_with_borrow(|locals| interrupt_function(locals, execute_data)) - { - debug!("ddog_php_prof_interrupt_function failed to borrow request locals: {err}"); - } -} - -fn interrupt_function(locals: &RequestLocals, execute_data: *mut zend_execute_data) { - let profiling_disabled = !locals.system_settings().profiling_enabled; - - /* Other extensions/modules or the engine itself may trigger an - * interrupt, but given how expensive it is to gather a stack trace, - * it should only be done if we triggered it ourselves. So - * interrupt_count serves dual purposes: - * 1. Track how many interrupts there were. - * 2. Ensure we don't collect on someone else's interrupt. - */ - let interrupt_count = locals.interrupt_count.swap(0, Ordering::SeqCst); - if profiling_disabled | (interrupt_count == 0) { - return; - } - - if let Some(profiler) = Profiler::get() { - // Safety: execute_data was provided by the engine, and the profiler doesn't mutate it. - profiler.collect_time(execute_data, interrupt_count); - } -} - -/// This function is like [`ddog_php_prof_interrupt_function`] except it's -/// optimized for the expectation that it's unlikely that there's actually an -/// interrupt actively needing to be handled. This is because we have to insert -/// this check in a variety of places, and in many of them (execute_internal, -/// frameless functions) there won't be an interrupt at all. -/// -/// # Safety -/// The zend_execute_data pointer should come from the engine to ensure it and -/// its sub-objects are valid. -#[no_mangle] -#[inline(never)] -pub extern "C" fn ddog_php_prof_interrupt_function_unlikely(execute_data: *mut zend_execute_data) { let result = REQUEST_LOCALS.try_with_borrow(|locals| { - // Optimize here with the expectation there isn't an interrupt, because - // overwhelmingly, there will not be one. The relaxed load avoids the - // more expensive atomic read-modify-write in `interrupt_function` on - // the idle path. It is only a fast-path hint: `interrupt_function` does - // the authoritative swap. If another consumer clears the count between - // the load and swap, the swap simply observes zero; if a producer adds - // an interrupt after this load observes zero, the count remains pending - // and the corresponding VM interrupt will provide another opportunity - // to handle it. - if locals.interrupt_count.load(Ordering::Relaxed) == 0 { + if !locals.system_settings().profiling_enabled { return; } - interrupt_function(locals, execute_data); + /* Other extensions/modules or the engine itself may trigger an + * interrupt, but given how expensive it is to gather a stack trace, + * it should only be done if we triggered it ourselves. So + * interrupt_count serves dual purposes: + * 1. Track how many interrupts there were. + * 2. Ensure we don't collect on someone else's interrupt. + */ + let interrupt_count = locals.interrupt_count.swap(0, Ordering::SeqCst); + if interrupt_count == 0 { + return; + } + + if let Some(profiler) = Profiler::get() { + // Safety: execute_data was provided by the engine, and the profiler doesn't mutate it. + profiler.collect_time(execute_data, interrupt_count); + } }); if let Err(err) = result { - debug!("ddog_php_prof_interrupt_function_unlikely failed to borrow request locals: {err}"); + debug!("ddog_php_prof_interrupt_function failed to borrow request locals: {err}"); } } @@ -178,8 +144,7 @@ mod frameless { use crate::bindings::{ zend_flf_functions, zend_flf_handlers, zend_frameless_function_info, }; - use crate::wall_time::ddog_php_prof_interrupt_function; - use crate::{zend, RefCellExt, REQUEST_LOCALS}; + use crate::{profiling::Profiler, zend, RefCellExt, REQUEST_LOCALS}; use dynasmrt::{dynasm, DynasmApi, ExecutableBuffer}; use log::error; use std::ffi::c_void; @@ -306,17 +271,23 @@ mod frameless { #[inline(never)] pub extern "C" fn ddog_php_prof_icall_trampoline_target() { let interrupt_count = REQUEST_LOCALS - .try_with_borrow(|locals| locals.interrupt_count.load(Ordering::Relaxed)) + .try_with_borrow(|locals| { + if !locals.system_settings().profiling_enabled { + return 0; + } + locals.interrupt_count.swap(0, Ordering::SeqCst) + }) .unwrap_or(0); if interrupt_count == 0 { return; } - // Fetching the execute data is intentionally delayed until we know - // that the interrupt count is greater than 0 for perf. - let execute_data = unsafe { zend::ddog_php_prof_get_current_execute_data() }; - ddog_php_prof_interrupt_function(execute_data); + if let Some(profiler) = Profiler::get() { + // SAFETY: profiler doesn't mutate execute_data + let execute_data = unsafe { zend::ddog_php_prof_get_current_execute_data() }; + profiler.collect_time(execute_data, interrupt_count); + } } } diff --git a/tests/tea/profiling/profiling.cc b/tests/tea/profiling/profiling.cc index 7de7779530..8c6e361427 100644 --- a/tests/tea/profiling/profiling.cc +++ b/tests/tea/profiling/profiling.cc @@ -9,7 +9,7 @@ ZEND_TLS datadog_php_stack_sample last_stack_sample; ZEND_API datadog_php_stack_sample tea_get_last_stack_sample(void) { return last_stack_sample; } -ZEND_API void ddog_php_prof_interrupt_function_unlikely(zend_execute_data *execute_data) { +ZEND_API void ddog_php_prof_interrupt_function(zend_execute_data *execute_data) { datadog_php_stack_sample_ctor(&last_stack_sample); /* Don't try to re-implement everything. Remember, the tracer is being @@ -34,10 +34,6 @@ ZEND_API void ddog_php_prof_interrupt_function_unlikely(zend_execute_data *execu } } -ZEND_API void ddog_php_prof_interrupt_function(zend_execute_data *execute_data) { - ddog_php_prof_interrupt_function_unlikely(execute_data); -} - ZEND_API zend_extension_version_info extension_version_info = { ZEND_EXTENSION_API_NO, ZEND_EXTENSION_BUILD_ID, diff --git a/tests/tea/profiling/profiling.h b/tests/tea/profiling/profiling.h index 452476071f..75f8aa6aa8 100644 --- a/tests/tea/profiling/profiling.h +++ b/tests/tea/profiling/profiling.h @@ -10,7 +10,6 @@ BEGIN_EXTERN_C() ZEND_API datadog_php_stack_sample tea_get_last_stack_sample(void); ZEND_API void ddog_php_prof_interrupt_function(zend_execute_data *execute_data); -ZEND_API void ddog_php_prof_interrupt_function_unlikely(zend_execute_data *execute_data); END_EXTERN_C() #endif diff --git a/tracer/engine_hooks.c b/tracer/engine_hooks.c index bbf9e16cb9..e2b2eaea97 100644 --- a/tracer/engine_hooks.c +++ b/tracer/engine_hooks.c @@ -41,14 +41,10 @@ void dd_search_for_profiling_symbols(void *arg) { if (extension->name && strcmp(extension->name, "datadog-profiling") == 0) { DL_HANDLE handle = extension->handle; - profiling_interrupt_function = (void(*)(zend_execute_data *))DL_FETCH_SYMBOL(handle, "ddog_php_prof_interrupt_function_unlikely"); - if (!profiling_interrupt_function) { - // Fall back for compatibility with profiler versions from before the - // unlikely-pending fast path was exported. - profiling_interrupt_function = (void(*)(zend_execute_data *))DL_FETCH_SYMBOL(handle, "ddog_php_prof_interrupt_function"); - } + profiling_interrupt_function = (void(*)(zend_execute_data *))DL_FETCH_SYMBOL(handle, "ddog_php_prof_interrupt_function"); if (UNEXPECTED(!profiling_interrupt_function)) { - LOG(WARN, "[Datadog Trace] Profiling was detected, but locating an interrupt function failed: %s\n", GET_DL_ERROR()); + LOG(WARN, "[Datadog Trace] Profiling was detected, but locating symbol %s failed: %s\n", "ddog_php_prof_interrupt_function", + GET_DL_ERROR()); } profiling_notify_trace_finished = (profiling_notify_trace_finished_t)DL_FETCH_SYMBOL(handle, "datadog_profiling_notify_trace_finished"); From d329cf623da6c2e29d17150ecd3646677b4b6e1e Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Wed, 29 Jul 2026 11:26:09 -0600 Subject: [PATCH 23/25] perf(prof): use TSRM globals for interrupt_count, Ordering::Relaxed These changes specifically speed up the hot paths of the interrupt function. Notably, this can be a bit hot on 8.3 and below because of execute_internal, and it's used on frameless functions on 8.4+. The biggest change comes from avoiding REQUEST_LOCALS. In this case, this is totally safe because interrupt_count is atomic. This also removes the profiling_enabled check of the interrupt function. This is unnecessary because: 1. We do not set interrupts when disabled to begin with, generally. trigger_time_sample can still trigger but this is test only. 2. For edge cases having a pending interrupt going into a fork, the child will call `Profiler::kill()` which will cause `Profiler::get()` to return None. --- profiling/src/allocation/mod.rs | 7 ++- profiling/src/capi.rs | 5 +- profiling/src/lib.rs | 12 ++-- profiling/src/module_globals.rs | 65 +++++++++++++++++++--- profiling/src/profiling/interrupts.rs | 4 +- profiling/src/profiling/mod.rs | 1 + profiling/src/wall_time.rs | 79 +++++++++++++-------------- 7 files changed, 112 insertions(+), 61 deletions(-) diff --git a/profiling/src/allocation/mod.rs b/profiling/src/allocation/mod.rs index f366d6a1cb..bb6a762f26 100644 --- a/profiling/src/allocation/mod.rs +++ b/profiling/src/allocation/mod.rs @@ -199,9 +199,10 @@ pub fn collect_allocation(ptr: *mut c_void, len: size_t) { // Check if there's a pending time interrupt that we can handle now // instead of waiting for an interrupt handler. This is slightly more // accurate and efficient, win-win. - let interrupt_count = REQUEST_LOCALS - .try_with_borrow(|locals| locals.interrupt_count.swap(0, Ordering::SeqCst)) - .unwrap_or(0); + // SAFETY: allocation samples are collected on an initialized PHP request thread. + let globals = unsafe { module_globals::get_profiler_globals() }; + // SAFETY: the current thread's module globals are valid through GSHUTDOWN. + let interrupt_count = unsafe { (*globals).interrupt_count.swap(0, Ordering::Relaxed) }; // SAFETY: execute_data was provided by the engine, and the profiler // doesn't mutate it. diff --git a/profiling/src/capi.rs b/profiling/src/capi.rs index 0ad6588fb3..0fe39a7b38 100644 --- a/profiling/src/capi.rs +++ b/profiling/src/capi.rs @@ -46,7 +46,10 @@ extern "C" fn ddog_php_prof_trigger_time_sample() { if locals.system_settings().profiling_enabled { // Safety: only vm interrupts are stored there, or possibly null (edges only). if let Some(vm_interrupt) = unsafe { locals.vm_interrupt_addr.as_ref() } { - locals.interrupt_count.fetch_add(1, Ordering::SeqCst); + // SAFETY: this callback runs on an initialized PHP request thread. + let globals = unsafe { crate::module_globals::get_profiler_globals() }; + // SAFETY: the current thread's module globals are valid through GSHUTDOWN. + unsafe { (*globals).interrupt_count.fetch_add(1, Ordering::Relaxed) }; vm_interrupt.store(true, Ordering::SeqCst); } } diff --git a/profiling/src/lib.rs b/profiling/src/lib.rs index e327b25ff5..f815f1514c 100644 --- a/profiling/src/lib.rs +++ b/profiling/src/lib.rs @@ -425,7 +425,6 @@ pub struct RequestLocals { pub system_settings: ptr::NonNull, pub profiling_experimental_heap_live_enabled: bool, - pub interrupt_count: AtomicU32, pub vm_interrupt_addr: *const AtomicBool, } @@ -450,7 +449,6 @@ impl Default for RequestLocals { tags: vec![], system_settings: SystemSettings::get(), profiling_experimental_heap_live_enabled: false, - interrupt_count: AtomicU32::new(0), vm_interrupt_addr: ptr::null_mut(), } } @@ -738,8 +736,11 @@ extern "C" fn rinit(_type: c_int, _module_number: c_int) -> ZendResult { } if let Some(profiler) = Profiler::get() { + // SAFETY: PHP module globals are initialized for this request thread. + let globals = unsafe { module_globals::get_profiler_globals() }; let interrupt = VmInterrupt { - interrupt_count_ptr: &locals.interrupt_count as *const AtomicU32, + // SAFETY: `globals` is valid until this thread's GSHUTDOWN. + interrupt_count_ptr: unsafe { ptr::addr_of!((*globals).interrupt_count) }, engine_ptr: locals.vm_interrupt_addr, }; profiler.add_interrupt(interrupt); @@ -795,8 +796,11 @@ extern "C" fn rshutdown(_type: c_int, _module_number: c_int) -> ZendResult { // and we don't need to optimize for that. if system_settings.profiling_enabled { if let Some(profiler) = Profiler::get() { + // SAFETY: PHP module globals remain initialized through RSHUTDOWN. + let globals = unsafe { module_globals::get_profiler_globals() }; let interrupt = VmInterrupt { - interrupt_count_ptr: &locals.interrupt_count, + // SAFETY: `globals` remains valid until this thread's GSHUTDOWN. + interrupt_count_ptr: unsafe { ptr::addr_of!((*globals).interrupt_count) }, engine_ptr: locals.vm_interrupt_addr, }; profiler.remove_interrupt(interrupt); diff --git a/profiling/src/module_globals.rs b/profiling/src/module_globals.rs index 85c938e535..c91559656e 100644 --- a/profiling/src/module_globals.rs +++ b/profiling/src/module_globals.rs @@ -2,6 +2,7 @@ use crate::allocation; use core::cell::Cell; use core::ffi::c_void; use core::ptr; +use core::sync::atomic::AtomicU32; #[cfg(php_zend_mm_set_custom_handlers_ex)] use crate::allocation::allocation_ge84::ZendMMState; @@ -13,6 +14,12 @@ pub struct ProfilerGlobals { /// Wrapped in `Cell` to prevent torn reads/writes when allocation hooks /// are called re-entrantly during `rinit()`/`rshutdown()`. pub zend_mm_state: Cell, + /// Number of profiler time interrupts pending for this PHP thread. + /// + /// The profiler timer thread updates this through a pointer registered by + /// the PHP thread, so the value must remain atomic despite living in + /// thread-local PHP module globals. + pub interrupt_count: AtomicU32, } /// We need TSRM to call into GINIT and GSHUTDOWN to observe spawning and @@ -29,6 +36,7 @@ pub static mut GLOBALS_ID: i32 = 0; #[cfg(not(php_zts))] pub static mut GLOBALS: ProfilerGlobals = ProfilerGlobals { zend_mm_state: Cell::new(ZendMMState::new()), + interrupt_count: AtomicU32::new(0), }; #[cfg(php_zts)] @@ -40,9 +48,13 @@ mod zts { } #[inline] - pub unsafe fn tsrmg_bulk(id: i32) -> *mut c_void { - let tls = tsrm_get_ls_cache() as *mut *mut *mut c_void; - let storage = *tls; // void** storage + pub unsafe fn get_ls_cache() -> *mut c_void { + tsrm_get_ls_cache() + } + + #[inline] + pub unsafe fn tsrmg_bulk(ls_cache: *mut c_void, id: i32) -> *mut c_void { + let storage = *(ls_cache as *mut *mut *mut c_void); // void** storage // TSRM_UNSHUFFLE_RSRC_ID(id) is just `id - 1`. let idx = (id - 1) as usize; @@ -51,6 +63,27 @@ mod zts { } } +#[cfg(php_zts)] +#[inline] +pub unsafe fn get_tsrm_ls_cache() -> *mut c_void { + zts::get_ls_cache() +} + +#[cfg(php_zts)] +#[inline] +pub unsafe fn get_tsrm_resource_from_cache(ls_cache: *mut c_void, id: i32) -> *mut c_void { + zts::tsrmg_bulk(ls_cache, id) +} + +#[cfg(php_zts)] +#[inline] +pub unsafe fn get_profiler_globals_from_cache(ls_cache: *mut c_void) -> *mut ProfilerGlobals { + // SAFETY: As long as this is called during the times documented by + // get_profiler_globals(), GLOBALS_ID will be set by PHP. + let id = ptr::addr_of!(GLOBALS_ID).read(); + get_tsrm_resource_from_cache(ls_cache, id).cast() +} + /// Returns a pointer to the profiler globals for the current thread. /// /// # Safety @@ -64,10 +97,7 @@ mod zts { pub unsafe fn get_profiler_globals() -> *mut ProfilerGlobals { #[cfg(php_zts)] { - // SAFETY: As long as this is called during the times documented by - // our own safety requirements, GLOBALS_ID will be set by PHP. - let id = ptr::addr_of!(GLOBALS_ID).read(); - zts::tsrmg_bulk(id).cast() + get_profiler_globals_from_cache(get_tsrm_ls_cache()) } #[cfg(not(php_zts))] @@ -85,12 +115,13 @@ pub unsafe extern "C" fn ginit(_globals_ptr: *mut c_void) { #[cfg(php_zts)] crate::timeline::timeline_ginit(); - // Initialize ZendMMState in PHP globals for ZTS builds. For NTS builds, - // this was already done in its const initializer. + // Initialize PHP globals for ZTS builds. For NTS builds, this was already + // done in its const initializer. #[cfg(php_zts)] { let globals = _globals_ptr.cast::(); (*globals).zend_mm_state = Cell::new(ZendMMState::new()); + (*globals).interrupt_count = AtomicU32::new(0); } // SAFETY: this is called in thread ginit as expected, and no other places. @@ -113,3 +144,19 @@ pub unsafe extern "C" fn gshutdown(_globals_ptr: *mut c_void) { // SAFETY: this is called in thread gshutdown as expected, no other places. allocation::gshutdown(); } + +// Unit tests are not loaded by PHP, so provide the PHP globals and TSRM symbol +// needed to link code retained in the test executable. +#[cfg(test)] +mod test_symbols { + #[cfg(not(php_zts))] + #[export_name = "compiler_globals"] + static mut TEST_COMPILER_GLOBALS: core::mem::MaybeUninit = + core::mem::MaybeUninit::zeroed(); + + #[cfg(php_zts)] + #[no_mangle] + unsafe extern "C" fn tsrm_get_ls_cache() -> *mut core::ffi::c_void { + core::ptr::null_mut() + } +} diff --git a/profiling/src/profiling/interrupts.rs b/profiling/src/profiling/interrupts.rs index bd1e32107e..7cd9ab2830 100644 --- a/profiling/src/profiling/interrupts.rs +++ b/profiling/src/profiling/interrupts.rs @@ -50,7 +50,7 @@ impl InterruptManager { // Reset interrupt counter to prevent sampling during `mshutdown` (PHP 8.0 bug with // userland destructors), but leave the interrupt flag unchanged as other extensions // may have raised it. - (*interrupt.interrupt_count_ptr).store(0, Ordering::SeqCst); + (*interrupt.interrupt_count_ptr).store(0, Ordering::Relaxed); } } @@ -62,7 +62,7 @@ impl InterruptManager { pub(super) fn trigger_interrupts(&self) { let vm_interrupts = self.vm_interrupts.lock().unwrap(); vm_interrupts.iter().for_each(|obj| unsafe { - (*obj.interrupt_count_ptr).fetch_add(1, Ordering::SeqCst); + (*obj.interrupt_count_ptr).fetch_add(1, Ordering::Relaxed); (*obj.engine_ptr).store(true, Ordering::SeqCst); }); } diff --git a/profiling/src/profiling/mod.rs b/profiling/src/profiling/mod.rs index 89c686bd18..759066c937 100644 --- a/profiling/src/profiling/mod.rs +++ b/profiling/src/profiling/mod.rs @@ -1130,6 +1130,7 @@ impl Profiler { /// Collect a stack sample with elapsed wall time. Collects CPU time if /// it's enabled and available. + #[export_name = "ddog_php_prof_collect_time"] #[cfg_attr(feature = "tracing", tracing::instrument(skip_all, level = "debug"))] pub fn collect_time(&self, execute_data: *mut zend_execute_data, interrupt_count: u32) { // todo: should probably exclude the wall and CPU time used by collecting the sample. diff --git a/profiling/src/wall_time.rs b/profiling/src/wall_time.rs index 3f0a948127..4acca712c8 100644 --- a/profiling/src/wall_time.rs +++ b/profiling/src/wall_time.rs @@ -2,10 +2,10 @@ //! implementation reasons, it has cpu-time code as well. use crate::bindings::{zend_execute_data, zend_interrupt_function, VmInterruptFn}; -use crate::{profiling::Profiler, RefCellExt, REQUEST_LOCALS}; +use crate::module_globals; +use crate::profiling::Profiler; use core::ptr; -use log::debug; -use std::sync::atomic::Ordering; +use core::sync::atomic::Ordering; #[cfg(not(php_frameless))] mod execute_internal { @@ -79,7 +79,7 @@ mod execute_internal { unsafe { prev_execute_internal(execute_data, return_value) }; // See safety section of `execute_data_func_is_trampoline` docs for why - // the leaf frame is used instead of the execute_data ptr. + // the leaf frame is used instead of the execute_data ptr. ddog_php_prof_interrupt_function(leaf_frame); } @@ -109,31 +109,29 @@ static mut PREV_INTERRUPT_FUNCTION: Option = None; #[no_mangle] #[inline(never)] pub extern "C" fn ddog_php_prof_interrupt_function(execute_data: *mut zend_execute_data) { - let result = REQUEST_LOCALS.try_with_borrow(|locals| { - if !locals.system_settings().profiling_enabled { - return; - } - - /* Other extensions/modules or the engine itself may trigger an - * interrupt, but given how expensive it is to gather a stack trace, - * it should only be done if we triggered it ourselves. So - * interrupt_count serves dual purposes: - * 1. Track how many interrupts there were. - * 2. Ensure we don't collect on someone else's interrupt. - */ - let interrupt_count = locals.interrupt_count.swap(0, Ordering::SeqCst); - if interrupt_count == 0 { - return; - } - - if let Some(profiler) = Profiler::get() { - // Safety: execute_data was provided by the engine, and the profiler doesn't mutate it. - profiler.collect_time(execute_data, interrupt_count); - } - }); + // SAFETY: interrupt callbacks run while the current PHP thread's module globals are valid. + let atomic_count = unsafe { &(*module_globals::get_profiler_globals()).interrupt_count }; + + /* Other extensions/modules or the engine itself may trigger an + * interrupt, but given how expensive it is to gather a stack trace, + * it should only be done if we triggered it ourselves. So + * interrupt_count serves dual purposes: + * 1. Track how many interrupts there were. + * 2. Ensure we don't collect on someone else's interrupt. + */ + let interrupt_count = atomic_count.swap(0, Ordering::Relaxed); + if interrupt_count == 0 { + return; + } + collect_time_if_enabled(execute_data, interrupt_count); +} - if let Err(err) = result { - debug!("ddog_php_prof_interrupt_function failed to borrow request locals: {err}"); +#[inline(never)] +#[export_name = "ddog_php_prof_collect_time_if_enabled"] +extern "C" fn collect_time_if_enabled(execute_data: *mut zend_execute_data, interrupt_count: u32) { + if let Some(profiler) = Profiler::get() { + // Safety: execute_data was provided by the engine, and the profiler doesn't mutate it. + profiler.collect_time(execute_data, interrupt_count); } } @@ -144,7 +142,9 @@ mod frameless { use crate::bindings::{ zend_flf_functions, zend_flf_handlers, zend_frameless_function_info, }; - use crate::{profiling::Profiler, zend, RefCellExt, REQUEST_LOCALS}; + use crate::module_globals; + use crate::wall_time::collect_time_if_enabled; + use crate::zend; use dynasmrt::{dynasm, DynasmApi, ExecutableBuffer}; use log::error; use std::ffi::c_void; @@ -270,24 +270,19 @@ mod frameless { #[no_mangle] #[inline(never)] pub extern "C" fn ddog_php_prof_icall_trampoline_target() { - let interrupt_count = REQUEST_LOCALS - .try_with_borrow(|locals| { - if !locals.system_settings().profiling_enabled { - return 0; - } - locals.interrupt_count.swap(0, Ordering::SeqCst) - }) - .unwrap_or(0); + // SAFETY: frameless handlers run while the current PHP thread's module globals are + // valid. Retain the pointer so the authoritative swap reuses the same TSRM lookup. + let atomic_count = + unsafe { &(*module_globals::get_profiler_globals()).interrupt_count }; + let interrupt_count = atomic_count.swap(0, Ordering::Relaxed); if interrupt_count == 0 { return; } - if let Some(profiler) = Profiler::get() { - // SAFETY: profiler doesn't mutate execute_data - let execute_data = unsafe { zend::ddog_php_prof_get_current_execute_data() }; - profiler.collect_time(execute_data, interrupt_count); - } + // Fetching execute data is intentionally delayed until a profiler interrupt is pending. + let execute_data = unsafe { zend::ddog_php_prof_get_current_execute_data() }; + collect_time_if_enabled(execute_data, interrupt_count); } } From 38a7d38dd89ec1c8037eca461e4bda1fa7b3ec6d Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Fri, 31 Jul 2026 10:25:08 -0600 Subject: [PATCH 24/25] perf(profiling): reuse module globals in allocation hooks --- profiling/src/allocation/allocation_ge84.rs | 65 +++++--- profiling/src/allocation/allocation_le83.rs | 61 ++++--- profiling/src/allocation/mod.rs | 44 ++---- profiling/src/allocation/profiling_stats.rs | 167 +++++--------------- profiling/src/module_globals.rs | 8 +- profiling/src/profiling/mod.rs | 6 +- 6 files changed, 149 insertions(+), 202 deletions(-) diff --git a/profiling/src/allocation/allocation_ge84.rs b/profiling/src/allocation/allocation_ge84.rs index 9ffd83ecd5..bfc1d4d2ba 100644 --- a/profiling/src/allocation/allocation_ge84.rs +++ b/profiling/src/allocation/allocation_ge84.rs @@ -1,8 +1,6 @@ -use crate::allocation::{ - allocation_profiling_stats_should_collect, collect_allocation, current_execute_data, - untrack_allocation, -}; +use crate::allocation::{collect_allocation, untrack_allocation}; use crate::bindings as zend; +use crate::module_globals::{self, ProfilerGlobals}; use crate::PROFILER_NAME; use core::ptr; use libc::{c_char, c_int, c_void, size_t}; @@ -10,6 +8,9 @@ use log::{debug, trace, warn}; use std::sync::atomic::Ordering::Relaxed; use std::sync::LazyLock; +#[cfg(php_zts)] +use crate::allocation::current_execute_data_from_cache; + #[cfg(php_debug)] use libc::c_uint; @@ -322,7 +323,14 @@ unsafe fn alloc_prof_malloc_impl(len: size_t) -> *mut c_void #[cfg(feature = "debug_stats")] ALLOCATION_PROFILING_SIZE.fetch_add(len as u64, Relaxed); - let state = tls_zend_mm_state_copy!(); + #[cfg(php_zts)] + let ls_cache = module_globals::get_tsrm_ls_cache(); + #[cfg(php_zts)] + let globals = module_globals::get_profiler_globals_from_cache(ls_cache); + #[cfg(not(php_zts))] + let globals = module_globals::get_profiler_globals(); + let state = (*globals).zend_mm_state.get(); + let ptr = if CUSTOM { let alloc = state.prev_custom_mm_alloc.unwrap(); #[cfg(php_debug)] @@ -343,12 +351,21 @@ unsafe fn alloc_prof_malloc_impl(len: size_t) -> *mut c_void // during startup, minit, rinit, ... current_execute_data is null // we are only interested in allocations during userland operations - if current_execute_data().is_null() { + #[cfg(php_zts)] + let execute_data = current_execute_data_from_cache(ls_cache); + #[cfg(not(php_zts))] + let execute_data = ptr::addr_of!(zend::executor_globals.current_execute_data).read(); + if execute_data.is_null() { return ptr; } - if allocation_profiling_stats_should_collect(len) { - collect_allocation(ptr, len); + if ProfilerGlobals::should_collect(globals, len) { + collect_allocation( + unsafe { &(*globals).interrupt_count }, + execute_data, + ptr, + len, + ); } ptr @@ -494,7 +511,14 @@ unsafe fn alloc_prof_realloc_impl( #[cfg(feature = "debug_stats")] ALLOCATION_PROFILING_SIZE.fetch_add(len as u64, Relaxed); - let state = tls_zend_mm_state_copy!(); + #[cfg(php_zts)] + let ls_cache = module_globals::get_tsrm_ls_cache(); + #[cfg(php_zts)] + let globals = module_globals::get_profiler_globals_from_cache(ls_cache); + #[cfg(not(php_zts))] + let globals = module_globals::get_profiler_globals(); + let state = (*globals).zend_mm_state.get(); + let ptr = if CUSTOM { let realloc = state.prev_custom_mm_realloc.unwrap(); #[cfg(php_debug)] @@ -521,23 +545,24 @@ unsafe fn alloc_prof_realloc_impl( untrack_allocation(prev_ptr); } - alloc_prof_realloc_sample(ptr, len) -} + #[cfg(php_zts)] + let execute_data = current_execute_data_from_cache(ls_cache); + #[cfg(not(php_zts))] + let execute_data = ptr::addr_of!(zend::executor_globals.current_execute_data).read(); -#[inline(always)] -unsafe fn alloc_prof_realloc_sample(ptr: *mut c_void, len: size_t) -> *mut c_void { // during startup, minit, rinit, ... current_execute_data is null // we are only interested in allocations during userland operations - if current_execute_data().is_null() { - return ptr; - } - - if ptr.is_null() { + if execute_data.is_null() || ptr.is_null() { return ptr; } - if allocation_profiling_stats_should_collect(len) { - collect_allocation(ptr, len); + if ProfilerGlobals::should_collect(globals, len) { + collect_allocation( + unsafe { &(*globals).interrupt_count }, + execute_data, + ptr, + len, + ); } ptr diff --git a/profiling/src/allocation/allocation_le83.rs b/profiling/src/allocation/allocation_le83.rs index 7c2436119f..14637a2384 100644 --- a/profiling/src/allocation/allocation_le83.rs +++ b/profiling/src/allocation/allocation_le83.rs @@ -1,15 +1,9 @@ -use crate::allocation::{ - allocation_profiling_stats_should_collect, collect_allocation, current_execute_data, - untrack_allocation, -}; -#[cfg(php_zts)] -use crate::allocation::{current_execute_data_from_cache, get_zend_mm_state_from_cache}; +use crate::allocation::{collect_allocation, untrack_allocation}; use crate::bindings::{ self as zend, datadog_php_install_handler, datadog_php_zif_handler, ddog_php_prof_copy_long_into_zval, }; -#[cfg(php_zts)] -use crate::module_globals; +use crate::module_globals::{self, ProfilerGlobals}; use crate::{RefCellExt, PROFILER_NAME, REQUEST_LOCALS}; use core::ptr; use libc::{c_char, c_int, c_void, size_t}; @@ -17,6 +11,9 @@ use log::{debug, trace, warn}; use std::sync::atomic::Ordering::Relaxed; use std::sync::LazyLock; +#[cfg(php_zts)] +use crate::allocation::current_execute_data_from_cache; + #[cfg(feature = "debug_stats")] use crate::allocation::{ALLOCATION_PROFILING_COUNT, ALLOCATION_PROFILING_SIZE}; @@ -315,9 +312,10 @@ unsafe fn alloc_prof_malloc_impl(len: size_t) -> *mut c_void #[cfg(php_zts)] let ls_cache = module_globals::get_tsrm_ls_cache(); #[cfg(php_zts)] - let state = (*get_zend_mm_state_from_cache(ls_cache)).get(); + let globals = module_globals::get_profiler_globals_from_cache(ls_cache); #[cfg(not(php_zts))] - let state = tls_zend_mm_state_copy!(); + let globals = module_globals::get_profiler_globals(); + let state = (*globals).zend_mm_state.get(); let ptr = if CUSTOM { state.prev_custom_mm_alloc.unwrap()(len) @@ -340,13 +338,18 @@ unsafe fn alloc_prof_malloc_impl(len: size_t) -> *mut c_void #[cfg(php_zts)] let execute_data = current_execute_data_from_cache(ls_cache); #[cfg(not(php_zts))] - let execute_data = current_execute_data(); + let execute_data = ptr::addr_of!(zend::executor_globals.current_execute_data).read(); if execute_data.is_null() { return ptr; } - if allocation_profiling_stats_should_collect(len) { - collect_allocation(ptr, len); + if ProfilerGlobals::should_collect(globals, len) { + collect_allocation( + unsafe { &(*globals).interrupt_count }, + execute_data, + ptr, + len, + ); } ptr @@ -436,7 +439,14 @@ unsafe fn alloc_prof_realloc_impl( #[cfg(feature = "debug_stats")] ALLOCATION_PROFILING_SIZE.fetch_add(len as u64, Relaxed); - let state = tls_zend_mm_state_copy!(); + #[cfg(php_zts)] + let ls_cache = module_globals::get_tsrm_ls_cache(); + #[cfg(php_zts)] + let globals = module_globals::get_profiler_globals_from_cache(ls_cache); + #[cfg(not(php_zts))] + let globals = module_globals::get_profiler_globals(); + let state = (*globals).zend_mm_state.get(); + let ptr = if CUSTOM { state.prev_custom_mm_realloc.unwrap()(prev_ptr, len) } else { @@ -461,23 +471,24 @@ unsafe fn alloc_prof_realloc_impl( untrack_allocation(prev_ptr); } - alloc_prof_realloc_sample(ptr, len) -} + #[cfg(php_zts)] + let execute_data = current_execute_data_from_cache(ls_cache); + #[cfg(not(php_zts))] + let execute_data = ptr::addr_of!(zend::executor_globals.current_execute_data).read(); -#[inline(always)] -unsafe fn alloc_prof_realloc_sample(ptr: *mut c_void, len: size_t) -> *mut c_void { // during startup, minit, rinit, ... current_execute_data is null // we are only interested in allocations during userland operations - if current_execute_data().is_null() { + if execute_data.is_null() || ptr.is_null() { return ptr; } - if ptr.is_null() { - return ptr; - } - - if allocation_profiling_stats_should_collect(len) { - collect_allocation(ptr, len); + if ProfilerGlobals::should_collect(globals, len) { + collect_allocation( + unsafe { &(*globals).interrupt_count }, + execute_data, + ptr, + len, + ); } ptr diff --git a/profiling/src/allocation/mod.rs b/profiling/src/allocation/mod.rs index d39d51af47..4766331dfb 100644 --- a/profiling/src/allocation/mod.rs +++ b/profiling/src/allocation/mod.rs @@ -14,7 +14,7 @@ use log::{debug, trace}; use rand_distr::{Distribution, Poisson}; use std::ffi::c_void; use std::num::{NonZero, NonZeroU32, NonZeroU64}; -use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::atomic::{AtomicU32, AtomicU64, Ordering}; #[cfg(not(php_zts))] use rand::rngs::StdRng; @@ -39,13 +39,6 @@ pub(crate) unsafe fn get_zend_mm_state() -> *mut Cell { ptr::addr_of_mut!((*globals).zend_mm_state) } -#[cfg(php_zts)] -#[inline] -pub(crate) unsafe fn get_zend_mm_state_from_cache(ls_cache: *mut c_void) -> *mut Cell { - let globals = module_globals::get_profiler_globals_from_cache(ls_cache); - ptr::addr_of_mut!((*globals).zend_mm_state) -} - #[cfg(php_zts)] #[inline(always)] pub(crate) unsafe fn current_execute_data_from_cache( @@ -68,15 +61,6 @@ pub(crate) unsafe fn current_execute_data_from_cache( ptr::addr_of!((*globals).current_execute_data).read() } -#[inline(always)] -pub(crate) unsafe fn current_execute_data() -> *mut zend::zend_execute_data { - #[cfg(not(php_zts))] - return ptr::addr_of!(zend::executor_globals.current_execute_data).read(); - - #[cfg(php_zts)] - zend::ddog_php_prof_get_current_execute_data() -} - /// Macros for accessing ZendMMState from PHP globals. /// These are shared between PHP 8.3- and 8.4+ implementations. /// They are exported at the crate root and can be used in submodules. @@ -175,7 +159,7 @@ pub static ALLOCATION_PROFILING_COUNT: AtomicU64 = AtomicU64::new(0); pub static ALLOCATION_PROFILING_SIZE: AtomicU64 = AtomicU64::new(0); pub struct AllocationProfilingStats { - /// number of bytes until next sample collection + /// Number of bytes remaining until the next sample collection. next_sample: i64, poisson: Poisson, #[cfg(php_zts)] @@ -206,42 +190,46 @@ impl AllocationProfilingStats { fn should_collect_allocation(&mut self, len: size_t) -> bool { self.next_sample -= len as i64; - if self.next_sample > 0 { return false; } self.next_sampling_interval(); - true } } /// Collect an allocation sample and optionally track it for live heap profiling. /// +/// # Safety +/// `execute_data` must be null or a valid pointer provided by the engine. The +/// profiler may walk the execution frames reachable through it. +/// /// # Arguments /// * `ptr` - The pointer returned by the allocator (used for live heap tracking) /// * `len` - The size of the allocation in bytes #[cold] -pub fn collect_allocation(ptr: *mut c_void, len: size_t) { +pub unsafe fn collect_allocation( + interrupt_count: &AtomicU32, + execute_data: *mut zend::zend_execute_data, + ptr: *mut c_void, + len: size_t, +) { if let Some(profiler) = Profiler::get() { // Check if there's a pending time interrupt that we can handle now // instead of waiting for an interrupt handler. This is slightly more // accurate and efficient, win-win. - // SAFETY: allocation samples are collected on an initialized PHP request thread. - let globals = unsafe { module_globals::get_profiler_globals() }; - // SAFETY: the current thread's module globals are valid through GSHUTDOWN. - let interrupt_count = unsafe { (*globals).interrupt_count.swap(0, Ordering::Relaxed) }; + let pending_interrupts = interrupt_count.swap(0, Ordering::Relaxed); // SAFETY: execute_data was provided by the engine, and the profiler - // doesn't mutate it. + // only reads the execution frames reachable through it. unsafe { profiler.collect_allocations( - zend::ddog_php_prof_get_current_execute_data(), + execute_data, ptr, 1_i64, len as i64, - (interrupt_count > 0).then_some(interrupt_count), + (pending_interrupts > 0).then_some(pending_interrupts), ) }; } diff --git a/profiling/src/allocation/profiling_stats.rs b/profiling/src/allocation/profiling_stats.rs index b2c0e30d26..87452abed9 100644 --- a/profiling/src/allocation/profiling_stats.rs +++ b/profiling/src/allocation/profiling_stats.rs @@ -1,124 +1,50 @@ -//! The thread-local allocation profiling stats are held in this module. -//! The stats are used on the hot-path of allocation, so this code is -//! performance sensitive. It is encapsulated so that some unsafe techniques -//! can be used but expose a relatively safe API. +//! Per-thread allocation profiling stats stored in PHP module globals. +//! The stats are used on the allocation hot path, so callers should thread +//! through an already-resolved [`ProfilerGlobals`] pointer whenever possible. use super::{AllocationProfilingStats, ALLOCATION_PROFILING_INTERVAL}; +use crate::module_globals::{self, ProfilerGlobals}; use libc::size_t; -use std::mem::MaybeUninit; +use std::num::NonZeroU64; +use std::sync::atomic::Ordering; #[cfg(php_zend_mm_set_custom_handlers_ex)] use super::allocation_ge84; #[cfg(not(php_zend_mm_set_custom_handlers_ex))] use super::allocation_le83; -#[cfg(php_zts)] -use std::cell::UnsafeCell; -use std::num::NonZeroU64; -use std::sync::atomic::Ordering; -#[cfg(php_zts)] -thread_local! { - /// This is initialized in ginit, before any memory allocator hooks are - /// installed. During a request, all accesses will be initialized. +impl ProfilerGlobals { + /// Updates the allocation sampling state from the PHP globals. /// - /// This is not pub so that unsafe code can be contained to this module. - static ALLOCATION_PROFILING_STATS: UnsafeCell> = - const { UnsafeCell::new(MaybeUninit::uninit()) }; -} - -#[cfg(not(php_zts))] -static mut ALLOCATION_PROFILING_STATS: MaybeUninit = - const { MaybeUninit::uninit() }; - -/// Accesses the thread-local [`AllocationProfilingStats`], passing a mutable -/// reference to the contained `MaybeUninit` to `F`. -/// -/// # Safety -/// -/// 1. There should not be any active borrows to the thread-local variable -/// [`AllocationProfilingStats`] when this function is called. -/// 2. Function `F` should not do anything which causes a new borrow on -/// [`AllocationProfilingStats`]. -/// 3. Do not call this function in ALLOCATION_PROFILING_STATS's destructor, -/// as it assumes that [`std::thread::LocalKey::try_with`] cannot fail. -/// -/// This is not pub to limit caller's ability to violate these conditions. -unsafe fn allocation_profiling_stats_mut(f: F) -> R -where - F: FnOnce(&mut MaybeUninit) -> R, -{ - #[cfg(php_zts)] - { - let result = ALLOCATION_PROFILING_STATS.try_with(|cell| { - let ptr: *mut MaybeUninit = cell.get(); - // SAFETY: the cell is statically initialized to [`MaybeUninit::uninit`] so the - // _cell_ is valid and initialized memory. As required by this own - // function's safety requirements, there should not be any active borrows - // to [`ALLOCATION_PROFILING_STATS`], so this mutable dereference is sound. - let uninit = unsafe { &mut *ptr }; - f(uninit) - }); - // SAFETY: this function is not called in a destructor, therefore it - // cannot return an AccessError: - // > If the key has been destroyed (which may happen if this is called - // > in a destructor), this function will return an AccessError. - unsafe { result.unwrap_unchecked() } - } - - #[cfg(not(php_zts))] - { - // SAFETY: For non-ZTS builds, ALLOCATION_PROFILING_STATS is a static variable. - // As required by this function's safety requirements, there should not be any - // active borrows to ALLOCATION_PROFILING_STATS, so this mutable reference is sound. - let uninit = unsafe { - let ptr: *mut MaybeUninit = - std::ptr::addr_of_mut!(ALLOCATION_PROFILING_STATS); - &mut *ptr - }; - f(uninit) - } -} - -/// Given the provided allocation length `len`, return whether the allocation -/// should be collected. This is a mutable operation, as the thread-local -/// variable will be modified to reduce the distance until the next sample. -pub fn allocation_profiling_stats_should_collect(len: size_t) -> bool { - let f = |maybe_uninit: &mut MaybeUninit| { - // SAFETY: ALLOCATION_PROFILING_STATS was initialized in GINIT. - let stats = unsafe { maybe_uninit.assume_init_mut() }; + /// # Safety + /// `globals` must point to initialized module globals for the current + /// thread, and no mutable access to its allocation profiling state may be + /// active. + #[inline(always)] + pub unsafe fn should_collect(globals: *mut ProfilerGlobals, len: size_t) -> bool { + // SAFETY: the state is initialized in GINIT and all accesses occur on the + // owning PHP thread. Allocator reentrancy cannot overlap this borrow because + // sampling state is released before stack collection begins. + let stats = + unsafe { (&mut *(*globals).allocation_profiling_stats.get()).assume_init_mut() }; stats.should_collect_allocation(len) - }; - - // SAFETY: - // 1. This function doesn't expose any way for the caller to keep a - // borrow alive, nor do the other public functions, so there cannot be - // any existing borrows alive. - // 2. This closure will not cause any new borrows. - // 3. This function isn't called during ALLOCATION_PROFILING_STATS's dtor, - // as MaybeUninit's destructor does nothing, you have to specifically drop - // it. Even if the destructor were called, AllocationProfilingStats's dtor - // doesn't access the TLS variable (it can't, it doesn't have access). - unsafe { allocation_profiling_stats_mut(f) } + } } /// Initializes the allocation profiler's globals. /// /// # Safety -/// -/// Must be called once per PHP thread ginit. +/// Must be called once per PHP thread GINIT. pub unsafe fn ginit() { - // SAFETY: - // 1. During ginit, there will not be any other borrows to stats. - // 2. This closure will not make new borrows to stats. - // 3. This is not during the thread-local destructor. + let interval = ALLOCATION_PROFILING_INTERVAL.load(Ordering::Relaxed); + // SAFETY: ALLOCATION_PROFILING_INTERVAL is always greater than zero. + let sampling_distance = unsafe { NonZeroU64::new_unchecked(interval) }; + // SAFETY: GINIT runs with allocated module globals and before allocator hooks. + let globals = unsafe { module_globals::get_profiler_globals() }; unsafe { - allocation_profiling_stats_mut(|uninit| { - let interval = ALLOCATION_PROFILING_INTERVAL.load(Ordering::Relaxed); - // SAFETY: ALLOCATION_PROFILING_INTERVAL must always be > 0. - let nonzero = NonZeroU64::new_unchecked(interval); - uninit.write(AllocationProfilingStats::new(nonzero)); - }) - }; + (&mut *(*globals).allocation_profiling_stats.get()) + .write(AllocationProfilingStats::new(sampling_distance)); + } #[cfg(not(php_zend_mm_set_custom_handlers_ex))] allocation_le83::alloc_prof_ginit(); @@ -126,36 +52,23 @@ pub unsafe fn ginit() { allocation_ge84::alloc_prof_ginit(); } -/// Initializes the allocation profiler's globals with the provided sampling -/// distance. +/// Reinitializes allocation sampling with the configured distance. /// /// # Safety -/// -/// Must be called once per PHP thread minit, unless the allocation profiling -/// is disabled, in which case it can be skipped. +/// Must be called once per PHP thread MINIT, unless allocation profiling is disabled. pub unsafe fn minit(sampling_distance: NonZeroU64) { - // SAFETY: - // 1. During minit, there will not be any other borrows. - // 2. This closure will not make new borrows. - // 3. This is not during the thread-local destructor. - unsafe { - allocation_profiling_stats_mut(|uninit| { - // SAFETY: previously initialized in ginit, we're just - // re-initializing it because we now have config - *uninit.assume_init_mut() = AllocationProfilingStats::new(sampling_distance); - }) - }; + // SAFETY: GINIT initialized this state, and MINIT has exclusive lifecycle access. + let globals = unsafe { module_globals::get_profiler_globals() }; + let stats = unsafe { (&mut *(*globals).allocation_profiling_stats.get()).assume_init_mut() }; + *stats = AllocationProfilingStats::new(sampling_distance); } -/// Shuts down the allocation profiler's globals. +/// Drops the allocation sampling state. /// /// # Safety -/// -/// Must be called once per PHP thread gshutdown. +/// Must be called once per PHP thread GSHUTDOWN after allocator hooks are removed. pub unsafe fn gshutdown() { - // SAFETY: - // 1. During gshutdown, there will not be any other borrows. - // 2. This closure will not make new borrows. - // 3. This is not during the thread-local destructor. - unsafe { allocation_profiling_stats_mut(|maybe_uninit| maybe_uninit.assume_init_drop()) } + // SAFETY: GINIT initialized this state, and GSHUTDOWN has exclusive lifecycle access. + let globals = unsafe { module_globals::get_profiler_globals() }; + unsafe { (&mut *(*globals).allocation_profiling_stats.get()).assume_init_drop() }; } diff --git a/profiling/src/module_globals.rs b/profiling/src/module_globals.rs index e0868ee881..30333bfdb0 100644 --- a/profiling/src/module_globals.rs +++ b/profiling/src/module_globals.rs @@ -1,6 +1,7 @@ use crate::allocation; -use core::cell::Cell; +use core::cell::{Cell, UnsafeCell}; use core::ffi::c_void; +use core::mem::MaybeUninit; use core::ptr; use core::sync::atomic::AtomicU32; @@ -20,6 +21,9 @@ pub struct ProfilerGlobals { /// the PHP thread, so the value must remain atomic despite living in /// thread-local PHP module globals. pub interrupt_count: AtomicU32, + /// Per-thread allocation sampling state. Kept in PHP globals so allocator + /// hooks can reuse an already-resolved TSRM cache instead of accessing Rust TLS. + pub allocation_profiling_stats: UnsafeCell>, } /// We need TSRM to call into GINIT and GSHUTDOWN to observe spawning and @@ -37,6 +41,7 @@ pub static mut GLOBALS_ID: i32 = 0; pub static mut GLOBALS: ProfilerGlobals = ProfilerGlobals { zend_mm_state: Cell::new(ZendMMState::new()), interrupt_count: AtomicU32::new(0), + allocation_profiling_stats: UnsafeCell::new(MaybeUninit::uninit()), }; #[cfg(all(test, php_zts))] @@ -128,6 +133,7 @@ pub unsafe extern "C" fn ginit(_globals_ptr: *mut c_void) { let globals = _globals_ptr.cast::(); (*globals).zend_mm_state = Cell::new(ZendMMState::new()); (*globals).interrupt_count = AtomicU32::new(0); + (*globals).allocation_profiling_stats = UnsafeCell::new(MaybeUninit::uninit()); } // SAFETY: this is called in thread ginit as expected, and no other places. diff --git a/profiling/src/profiling/mod.rs b/profiling/src/profiling/mod.rs index 759066c937..574320c166 100644 --- a/profiling/src/profiling/mod.rs +++ b/profiling/src/profiling/mod.rs @@ -1178,8 +1178,12 @@ impl Profiler { /// /// If heap live profiling is enabled, the allocation is tracked for later /// cancellation when freed. + /// + /// # Safety + /// `execute_data` must be null or a valid pointer provided by the engine. + /// The profiler walks the execution frames reachable through it. #[cfg_attr(feature = "tracing", tracing::instrument(skip_all))] - pub fn collect_allocations( + pub unsafe fn collect_allocations( &self, execute_data: *mut zend_execute_data, ptr: *mut std::ffi::c_void, From 616427c1635655fba50c4eda9d1f3f822ac90c5a Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Fri, 31 Jul 2026 11:29:04 -0600 Subject: [PATCH 25/25] fix(profiling): satisfy clippy for allocation globals --- profiling/src/allocation/profiling_stats.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/profiling/src/allocation/profiling_stats.rs b/profiling/src/allocation/profiling_stats.rs index 87452abed9..8cb6c44000 100644 --- a/profiling/src/allocation/profiling_stats.rs +++ b/profiling/src/allocation/profiling_stats.rs @@ -25,8 +25,7 @@ impl ProfilerGlobals { // SAFETY: the state is initialized in GINIT and all accesses occur on the // owning PHP thread. Allocator reentrancy cannot overlap this borrow because // sampling state is released before stack collection begins. - let stats = - unsafe { (&mut *(*globals).allocation_profiling_stats.get()).assume_init_mut() }; + let stats = unsafe { (*(*globals).allocation_profiling_stats.get()).assume_init_mut() }; stats.should_collect_allocation(len) } } @@ -42,7 +41,7 @@ pub unsafe fn ginit() { // SAFETY: GINIT runs with allocated module globals and before allocator hooks. let globals = unsafe { module_globals::get_profiler_globals() }; unsafe { - (&mut *(*globals).allocation_profiling_stats.get()) + (*(*globals).allocation_profiling_stats.get()) .write(AllocationProfilingStats::new(sampling_distance)); } @@ -59,7 +58,7 @@ pub unsafe fn ginit() { pub unsafe fn minit(sampling_distance: NonZeroU64) { // SAFETY: GINIT initialized this state, and MINIT has exclusive lifecycle access. let globals = unsafe { module_globals::get_profiler_globals() }; - let stats = unsafe { (&mut *(*globals).allocation_profiling_stats.get()).assume_init_mut() }; + let stats = unsafe { (*(*globals).allocation_profiling_stats.get()).assume_init_mut() }; *stats = AllocationProfilingStats::new(sampling_distance); } @@ -70,5 +69,5 @@ pub unsafe fn minit(sampling_distance: NonZeroU64) { pub unsafe fn gshutdown() { // SAFETY: GINIT initialized this state, and GSHUTDOWN has exclusive lifecycle access. let globals = unsafe { module_globals::get_profiler_globals() }; - unsafe { (&mut *(*globals).allocation_profiling_stats.get()).assume_init_drop() }; + unsafe { (*(*globals).allocation_profiling_stats.get()).assume_init_drop() }; }