Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions .github/workflows/prof_correctness.yml
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ jobs:
export DD_PROFILING_EXCEPTION_MESSAGE_ENABLED=1
php -v
php -d extension=target/profiler-release/libdatadog_php_profiling.so --ri datadog-profiling
for test_case in "allocations" "time" "strange_frames" "timeline" "exceptions" "io" "allocation_time_combined"; do
for test_case in "allocations" "time" "strange_frames" "timeline" "exceptions" "io" "allocation_time_combined" "generators"; do
mkdir -p profiling/tests/correctness/"$test_case"/
export DD_PROFILING_OUTPUT_PPROF=$PWD/profiling/tests/correctness/"$test_case"/test.pprof
php -d extension="${PWD}/target/profiler-release/libdatadog_php_profiling.so" "profiling/tests/correctness/${test_case}.php"
Expand All @@ -98,7 +98,7 @@ jobs:
export DD_PROFILING_EXCEPTION_MESSAGE_ENABLED=1
php -v
php -d extension=target/profiler-release/libdatadog_php_profiling.so --ri datadog-profiling
for test_case in "allocations" "time" "strange_frames" "timeline" "exceptions" "io" "allocation_time_combined"; do
for test_case in "allocations" "time" "strange_frames" "timeline" "exceptions" "io" "allocation_time_combined" "generators"; do
mkdir -p profiling/tests/correctness/"$test_case"/
export DD_PROFILING_OUTPUT_PPROF=$PWD/profiling/tests/correctness/"$test_case"/test.pprof
php -d extension=$PWD/target/profiler-release/libdatadog_php_profiling.so profiling/tests/correctness/"$test_case".php
Expand Down Expand Up @@ -170,6 +170,12 @@ jobs:
expected_json: profiling/tests/correctness/allocation_time_combined.json
pprof_path: profiling/tests/correctness/allocation_time_combined/

- name: Check profiler correctness for generators
uses: Datadog/prof-correctness/analyze@main
with:
expected_json: profiling/tests/correctness/generators.json
pprof_path: profiling/tests/correctness/generators/

- name: Check profiler correctness for IO
uses: Datadog/prof-correctness/analyze@main
with:
Expand Down
1 change: 1 addition & 0 deletions profiling/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,7 @@ fn apple_linker_flags() {
"_zend_extensions",
"_zend_flf_functions",
"_zend_gc_get_status",
"_zend_generator_check_placeholder_frame",
"_zend_get_executed_filename_ex",
"_zend_get_executed_lineno",
"_zend_get_extension",
Expand Down
7 changes: 7 additions & 0 deletions profiling/src/bindings/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,13 @@ extern "C" {
func: &zend_function,
) -> Option<&mut [usize; 2]>;

/// mock for testing; passthrough stub for zend_generator_check_placeholder_frame
/// so `cargo test` builds don't need PHP's Zend symbols at link time.
#[cfg(feature = "stack_walking_tests")]
pub fn ddog_test_zend_generator_check_placeholder_frame(
ptr: *mut zend_execute_data,
) -> *mut zend_execute_data;

/// Returns the PHP_VERSION_ID of the engine at run-time, not the version
/// the extension was built against at compile-time.
pub fn ddog_php_prof_php_version_id() -> u32;
Expand Down
5 changes: 5 additions & 0 deletions profiling/src/php_ffi.c
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,11 @@ uintptr_t *ddog_php_prof_function_run_time_cache(zend_function const *func) {
}

#if CFG_STACK_WALKING_TESTS
zend_execute_data *ddog_test_zend_generator_check_placeholder_frame(zend_execute_data *ptr) {
// Tests do not construct real generator placeholder frames; pass through.
return ptr;
}

uintptr_t *ddog_test_php_prof_function_run_time_cache(zend_function const *func) {
#if CFG_RUN_TIME_CACHE
if (_ignore_run_time_cache) return NULL;
Expand Down
28 changes: 28 additions & 0 deletions profiling/src/profiling/stack_walking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,13 +300,29 @@ mod detail {
top_execute_data: *mut zend_execute_data,
string_set: &mut StringSet,
) -> Result<Backtrace, CollectStackSampleError> {
#[cfg(feature = "stack_walking_tests")]
use crate::bindings::ddog_test_zend_generator_check_placeholder_frame as zend_generator_check_placeholder_frame;
#[cfg(not(feature = "stack_walking_tests"))]
use crate::bindings::zend_generator_check_placeholder_frame;

let max_depth = 512;
let mut samples = Vec::new();
let mut execute_data_ptr = top_execute_data;

samples.try_reserve(max_depth >> 3)?;

while let Some(execute_data) = unsafe { execute_data_ptr.as_ref() } {
// Nested generators leave a placeholder frame (func == NULL,
// This == generator object) on the stack. Resolve it to the real
// frame the same way zend_fetch_debug_backtrace and the observer
// API do.
if execute_data.func.is_null() {
execute_data_ptr =
unsafe { zend_generator_check_placeholder_frame(execute_data_ptr) };
}
let Some(execute_data) = (unsafe { execute_data_ptr.as_ref() }) else {
break;
};
// allowed because it's only used on the frameless path
#[allow(unused_variables)]
if let Some(func) = unsafe { execute_data.func.as_ref() } {
Expand Down Expand Up @@ -485,6 +501,11 @@ mod detail {
pub fn collect_stack_sample(
top_execute_data: *mut zend_execute_data,
) -> Result<Backtrace, CollectStackSampleError> {
#[cfg(feature = "stack_walking_tests")]
use crate::bindings::ddog_test_zend_generator_check_placeholder_frame as zend_generator_check_placeholder_frame;
#[cfg(not(feature = "stack_walking_tests"))]
use crate::bindings::zend_generator_check_placeholder_frame;

#[cfg(feature = "tracing")]
let _span = tracing::trace_span!("collect_stack_sample").entered();

Expand All @@ -493,6 +514,13 @@ mod detail {
let mut execute_data_ptr = top_execute_data;

while let Some(execute_data) = unsafe { execute_data_ptr.as_ref() } {
if execute_data.func.is_null() {
execute_data_ptr =
unsafe { zend_generator_check_placeholder_frame(execute_data_ptr) };
}
let Some(execute_data) = (unsafe { execute_data_ptr.as_ref() }) else {
break;
};
let maybe_frame = unsafe { collect_call_frame(execute_data) };
if let Some(frame) = maybe_frame {
samples.try_push(frame)?;
Expand Down
36 changes: 36 additions & 0 deletions profiling/tests/correctness/generators.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
{
"scale_by_duration": true,
"test_name": "php_generators",
"stacks": [
{
"profile-type": "alloc-size",
"stack-content": [
{
"regular_expression": "<?php;main;middle;leaf;standard\\|str_repeat$",
"percent": 50,
"error_margin": 5
},
{
"regular_expression": "<?php;main;middle;leaf;standard\\|str_replace$",
"percent": 50,
"error_margin": 5
}
]
},
{
"profile-type": "alloc-samples",
"stack-content": [
{
"regular_expression": "<?php;main;middle;leaf;standard\\|str_repeat$",
"percent": 50,
"error_margin": 5
},
{
"regular_expression": "<?php;main;middle;leaf;standard\\|str_replace$",
"percent": 50,
"error_margin": 5
}
]
}
]
}
40 changes: 40 additions & 0 deletions profiling/tests/correctness/generators.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php

/* The purpose of this test is to verify that stacks collected inside
* generators which use `yield from` (delegation) correctly include the
* delegating generator frame. PHP inserts a placeholder frame between the
* delegate and the delegator which has a NULL `func`; without resolving it
* via `zend_generator_check_placeholder_frame`, the delegator (`middle`)
* would be missing from the stack.
*/

function leaf()
{
$s = str_repeat("a", 1024 * 12_000);
str_replace('a', 'b', $s);
yield 1;
}

function middle()
{
yield from leaf();
}

function main()
{
$duration = $_ENV["EXECUTION_TIME"] ?? 10;
$end = microtime(true) + $duration;
while (microtime(true) < $end) {
$start = microtime(true);
foreach (middle() as $_) {
// consume the generator
}
$elapsed = microtime(true) - $start;
// sleep for the remainder to 100 ms so we end up doing 10 iterations per second
$sleep = (0.1 - $elapsed);
if ($sleep > 0.0) {
usleep((int) ($sleep * 1_000_000));
}
}
}
main();
Loading