Skip to content

Commit b0995bc

Browse files
committed
Resolve TODOs
Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
1 parent b9fc370 commit b0995bc

File tree

2 files changed

+39
-14
lines changed

2 files changed

+39
-14
lines changed

profiling/src/php_ffi.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,10 @@ bool ddog_php_prof_is_post_startup(void) {
109109
static post_startup_cb_result (*orig_post_startup_cb)(void) = NULL;
110110

111111
static post_startup_cb_result ddog_php_prof_post_startup_cb(void) {
112+
#if CFG_FRAMELESS
113+
ddog_php_prof_post_startup(); // before preload+JIT (which may hardcode the flf handlers)
114+
#endif
115+
112116
if (orig_post_startup_cb) {
113117
post_startup_cb_result (*cb)(void) = orig_post_startup_cb;
114118

@@ -118,10 +122,6 @@ static post_startup_cb_result ddog_php_prof_post_startup_cb(void) {
118122
}
119123
}
120124

121-
#if CFG_FRAMELESS
122-
ddog_php_prof_post_startup();
123-
#endif
124-
125125
_is_post_startup = true;
126126

127127
return SUCCESS;

profiling/src/wall_time.rs

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ pub extern "C" fn ddog_php_prof_interrupt_function(execute_data: *mut zend_execu
137137
}
138138
}
139139

140-
#[cfg(php_frameless)]
140+
//#[cfg(php_frameless)]
141141
mod frameless {
142142
#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
143143
mod trampoline {
@@ -147,11 +147,16 @@ mod frameless {
147147
use dynasmrt::DynasmLabelApi;
148148
#[cfg(target_arch = "x86_64")]
149149
use dynasmrt::x64::Assembler;
150-
use dynasmrt::{dynasm, DynasmApi};
150+
use dynasmrt::{dynasm, DynasmApi, ExecutableBuffer};
151151
use std::ffi::c_void;
152-
use super::super::ddog_php_prof_interrupt_function;
153-
use crate::bindings::{zend_flf_functions, zend_flf_handlers};
154-
use crate::zend;
152+
use std::sync::atomic::Ordering;
153+
use log::debug;
154+
use crate::bindings::{zend_flf_functions, zend_flf_handlers, zend_frameless_function_info};
155+
use crate::{profiling::Profiler, RefCellExt, REQUEST_LOCALS, zend};
156+
157+
// This ensures that the memory stays reachable and is replaced on apache reload for example
158+
static mut INFOS: Vec<zend_frameless_function_info> = Vec::new();
159+
static mut BUFFER: Option<ExecutableBuffer> = None;
155160

156161
pub unsafe fn install() {
157162
// Collect frameless functions ahead of time to batch-process them.
@@ -239,15 +244,35 @@ mod frameless {
239244
ptr = ptr.add(1);
240245
}
241246
}
242-
std::mem::forget(infos); // TODO: leaks memory
243-
std::mem::forget(buffer); // TODO: leaks memory
247+
248+
INFOS = infos;
249+
BUFFER = Some(buffer);
244250
}
245251

246252
#[no_mangle]
247253
#[inline(never)]
248-
pub unsafe extern "C" fn ddog_php_prof_icall_trampoline_target() {
249-
// TODO: First check for REQUEST_LOCALS.interrupt_count before fetching execute data to make this less expensive
250-
ddog_php_prof_interrupt_function(zend::ddog_php_prof_get_current_execute_data());
254+
pub extern "C" fn ddog_php_prof_icall_trampoline_target() {
255+
let result = REQUEST_LOCALS.try_with_borrow(|locals| {
256+
if !locals.system_settings().profiling_enabled {
257+
return;
258+
}
259+
260+
// Check whether we are actually wanting an interrupt to be handled.
261+
let interrupt_count = locals.interrupt_count.swap(0, Ordering::SeqCst);
262+
if interrupt_count == 0 {
263+
return;
264+
}
265+
266+
if let Some(profiler) = Profiler::get() {
267+
// SAFETY: profiler doesn't mutate execute_data
268+
let execute_data = unsafe { zend::ddog_php_prof_get_current_execute_data() };
269+
profiler.collect_time(execute_data, interrupt_count);
270+
}
271+
});
272+
273+
if let Err(err) = result {
274+
debug!("ddog_php_prof_icall_trampoline_target failed to borrow request locals: {err}");
275+
}
251276
}
252277
}
253278

0 commit comments

Comments
 (0)