Skip to content

Commit 9a2b413

Browse files
morrisonleviclaude
andauthored
feat(profiling): support generator unwinding (#3807)
* test(profiling): add correctness test for generator unwinding Covers the yield-from placeholder-frame path: the PHP script chains main -> middle -> leaf via `yield from`, and the JSON asserts that allocations inside `leaf` report the full delegating chain. Without resolving the placeholder frame, `middle` would be missing from the stack. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(profiling): support generator unwinding * test(profiling): provide test stub for zend_generator_check_placeholder_frame `cargo test` links the profiler as a regular binary, so the Zend symbol cannot be resolved the way it is for the cdylib (lazy, at PHP load time). Add a passthrough stub under CFG_STACK_WALKING_TESTS and swap the import in stack_walking.rs the same way ddog_php_prof_function_run_time_cache is handled. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Revert "test(profiling): provide test stub for zend_generator_check_placeholder_frame" This reverts commit ea7bb074999dbcde5a8f78d3c673383818641719. * Revert "feat(profiling): support generator unwinding" This reverts commit 944a3cb8104f8c6faec54be4b8d12277b6c45dcd. * Reapply "feat(profiling): support generator unwinding" This reverts commit 0368d3e. * Reapply "test(profiling): provide test stub for zend_generator_check_placeholder_frame" This reverts commit 319db5a. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 4feb951 commit 9a2b413

7 files changed

Lines changed: 125 additions & 2 deletions

File tree

.github/workflows/prof_correctness.yml

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ jobs:
8080
export DD_PROFILING_EXCEPTION_MESSAGE_ENABLED=1
8181
php -v
8282
php -d extension=target/profiler-release/libdatadog_php_profiling.so --ri datadog-profiling
83-
for test_case in "allocations" "time" "strange_frames" "timeline" "exceptions" "io" "allocation_time_combined"; do
83+
for test_case in "allocations" "time" "strange_frames" "timeline" "exceptions" "io" "allocation_time_combined" "generators"; do
8484
mkdir -p profiling/tests/correctness/"$test_case"/
8585
export DD_PROFILING_OUTPUT_PPROF=$PWD/profiling/tests/correctness/"$test_case"/test.pprof
8686
php -d extension="${PWD}/target/profiler-release/libdatadog_php_profiling.so" "profiling/tests/correctness/${test_case}.php"
@@ -98,7 +98,7 @@ jobs:
9898
export DD_PROFILING_EXCEPTION_MESSAGE_ENABLED=1
9999
php -v
100100
php -d extension=target/profiler-release/libdatadog_php_profiling.so --ri datadog-profiling
101-
for test_case in "allocations" "time" "strange_frames" "timeline" "exceptions" "io" "allocation_time_combined"; do
101+
for test_case in "allocations" "time" "strange_frames" "timeline" "exceptions" "io" "allocation_time_combined" "generators"; do
102102
mkdir -p profiling/tests/correctness/"$test_case"/
103103
export DD_PROFILING_OUTPUT_PPROF=$PWD/profiling/tests/correctness/"$test_case"/test.pprof
104104
php -d extension=$PWD/target/profiler-release/libdatadog_php_profiling.so profiling/tests/correctness/"$test_case".php
@@ -170,6 +170,12 @@ jobs:
170170
expected_json: profiling/tests/correctness/allocation_time_combined.json
171171
pprof_path: profiling/tests/correctness/allocation_time_combined/
172172

173+
- name: Check profiler correctness for generators
174+
uses: Datadog/prof-correctness/analyze@main
175+
with:
176+
expected_json: profiling/tests/correctness/generators.json
177+
pprof_path: profiling/tests/correctness/generators/
178+
173179
- name: Check profiler correctness for IO
174180
uses: Datadog/prof-correctness/analyze@main
175181
with:

profiling/build.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,7 @@ fn apple_linker_flags() {
542542
"_zend_extensions",
543543
"_zend_flf_functions",
544544
"_zend_gc_get_status",
545+
"_zend_generator_check_placeholder_frame",
545546
"_zend_get_executed_filename_ex",
546547
"_zend_get_executed_lineno",
547548
"_zend_get_extension",

profiling/src/bindings/mod.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,13 @@ extern "C" {
365365
func: &zend_function,
366366
) -> Option<&mut [usize; 2]>;
367367

368+
/// mock for testing; passthrough stub for zend_generator_check_placeholder_frame
369+
/// so `cargo test` builds don't need PHP's Zend symbols at link time.
370+
#[cfg(feature = "stack_walking_tests")]
371+
pub fn ddog_test_zend_generator_check_placeholder_frame(
372+
ptr: *mut zend_execute_data,
373+
) -> *mut zend_execute_data;
374+
368375
/// Returns the PHP_VERSION_ID of the engine at run-time, not the version
369376
/// the extension was built against at compile-time.
370377
pub fn ddog_php_prof_php_version_id() -> u32;

profiling/src/php_ffi.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,11 @@ uintptr_t *ddog_php_prof_function_run_time_cache(zend_function const *func) {
383383
}
384384

385385
#if CFG_STACK_WALKING_TESTS
386+
zend_execute_data *ddog_test_zend_generator_check_placeholder_frame(zend_execute_data *ptr) {
387+
// Tests do not construct real generator placeholder frames; pass through.
388+
return ptr;
389+
}
390+
386391
uintptr_t *ddog_test_php_prof_function_run_time_cache(zend_function const *func) {
387392
#if CFG_RUN_TIME_CACHE
388393
if (_ignore_run_time_cache) return NULL;

profiling/src/profiling/stack_walking.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,13 +300,29 @@ mod detail {
300300
top_execute_data: *mut zend_execute_data,
301301
string_set: &mut StringSet,
302302
) -> Result<Backtrace, CollectStackSampleError> {
303+
#[cfg(feature = "stack_walking_tests")]
304+
use crate::bindings::ddog_test_zend_generator_check_placeholder_frame as zend_generator_check_placeholder_frame;
305+
#[cfg(not(feature = "stack_walking_tests"))]
306+
use crate::bindings::zend_generator_check_placeholder_frame;
307+
303308
let max_depth = 512;
304309
let mut samples = Vec::new();
305310
let mut execute_data_ptr = top_execute_data;
306311

307312
samples.try_reserve(max_depth >> 3)?;
308313

309314
while let Some(execute_data) = unsafe { execute_data_ptr.as_ref() } {
315+
// Nested generators leave a placeholder frame (func == NULL,
316+
// This == generator object) on the stack. Resolve it to the real
317+
// frame the same way zend_fetch_debug_backtrace and the observer
318+
// API do.
319+
if execute_data.func.is_null() {
320+
execute_data_ptr =
321+
unsafe { zend_generator_check_placeholder_frame(execute_data_ptr) };
322+
}
323+
let Some(execute_data) = (unsafe { execute_data_ptr.as_ref() }) else {
324+
break;
325+
};
310326
// allowed because it's only used on the frameless path
311327
#[allow(unused_variables)]
312328
if let Some(func) = unsafe { execute_data.func.as_ref() } {
@@ -485,6 +501,11 @@ mod detail {
485501
pub fn collect_stack_sample(
486502
top_execute_data: *mut zend_execute_data,
487503
) -> Result<Backtrace, CollectStackSampleError> {
504+
#[cfg(feature = "stack_walking_tests")]
505+
use crate::bindings::ddog_test_zend_generator_check_placeholder_frame as zend_generator_check_placeholder_frame;
506+
#[cfg(not(feature = "stack_walking_tests"))]
507+
use crate::bindings::zend_generator_check_placeholder_frame;
508+
488509
#[cfg(feature = "tracing")]
489510
let _span = tracing::trace_span!("collect_stack_sample").entered();
490511

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

495516
while let Some(execute_data) = unsafe { execute_data_ptr.as_ref() } {
517+
if execute_data.func.is_null() {
518+
execute_data_ptr =
519+
unsafe { zend_generator_check_placeholder_frame(execute_data_ptr) };
520+
}
521+
let Some(execute_data) = (unsafe { execute_data_ptr.as_ref() }) else {
522+
break;
523+
};
496524
let maybe_frame = unsafe { collect_call_frame(execute_data) };
497525
if let Some(frame) = maybe_frame {
498526
samples.try_push(frame)?;
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
{
2+
"scale_by_duration": true,
3+
"test_name": "php_generators",
4+
"stacks": [
5+
{
6+
"profile-type": "alloc-size",
7+
"stack-content": [
8+
{
9+
"regular_expression": "<?php;main;middle;leaf;standard\\|str_repeat$",
10+
"percent": 50,
11+
"error_margin": 5
12+
},
13+
{
14+
"regular_expression": "<?php;main;middle;leaf;standard\\|str_replace$",
15+
"percent": 50,
16+
"error_margin": 5
17+
}
18+
]
19+
},
20+
{
21+
"profile-type": "alloc-samples",
22+
"stack-content": [
23+
{
24+
"regular_expression": "<?php;main;middle;leaf;standard\\|str_repeat$",
25+
"percent": 50,
26+
"error_margin": 5
27+
},
28+
{
29+
"regular_expression": "<?php;main;middle;leaf;standard\\|str_replace$",
30+
"percent": 50,
31+
"error_margin": 5
32+
}
33+
]
34+
}
35+
]
36+
}
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<?php
2+
3+
/* The purpose of this test is to verify that stacks collected inside
4+
* generators which use `yield from` (delegation) correctly include the
5+
* delegating generator frame. PHP inserts a placeholder frame between the
6+
* delegate and the delegator which has a NULL `func`; without resolving it
7+
* via `zend_generator_check_placeholder_frame`, the delegator (`middle`)
8+
* would be missing from the stack.
9+
*/
10+
11+
function leaf()
12+
{
13+
$s = str_repeat("a", 1024 * 12_000);
14+
str_replace('a', 'b', $s);
15+
yield 1;
16+
}
17+
18+
function middle()
19+
{
20+
yield from leaf();
21+
}
22+
23+
function main()
24+
{
25+
$duration = $_ENV["EXECUTION_TIME"] ?? 10;
26+
$end = microtime(true) + $duration;
27+
while (microtime(true) < $end) {
28+
$start = microtime(true);
29+
foreach (middle() as $_) {
30+
// consume the generator
31+
}
32+
$elapsed = microtime(true) - $start;
33+
// sleep for the remainder to 100 ms so we end up doing 10 iterations per second
34+
$sleep = (0.1 - $elapsed);
35+
if ($sleep > 0.0) {
36+
usleep((int) ($sleep * 1_000_000));
37+
}
38+
}
39+
}
40+
main();

0 commit comments

Comments
 (0)