Skip to content

Commit 89c75b2

Browse files
dubloomdubloom
authored
Merge branch 'master' into dubloom/process-tags
2 parents 67e08df + 1316514 commit 89c75b2

10 files changed

Lines changed: 506 additions & 422 deletions

File tree

‎.github/workflows/prof_correctness.yml‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,10 @@ jobs:
102102
export DD_PROFILING_OUTPUT_PPROF=$PWD/profiling/tests/correctness/allocations_1byte/test.pprof
103103
export DD_PROFILING_ALLOCATION_SAMPLING_DISTANCE=1
104104
php -d extension=$PWD/target/profiler-release/libdatadog_php_profiling.so profiling/tests/correctness/allocations.php
105+
mkdir -p profiling/tests/correctness/allocations_1byte_no_zend_alloc/
106+
export DD_PROFILING_OUTPUT_PPROF=$PWD/profiling/tests/correctness/allocations_1byte_no_zend_alloc/test.pprof
107+
export DD_PROFILING_ALLOCATION_SAMPLING_DISTANCE=1
108+
USE_ZEND_ALLOC=0 php -d extension=$PWD/target/profiler-release/libdatadog_php_profiling.so profiling/tests/correctness/allocations.php
105109
unset DD_PROFILING_ALLOCATION_SAMPLING_DISTANCE
106110
107111
- name: Run ZTS tests
@@ -131,6 +135,12 @@ jobs:
131135
expected_json: profiling/tests/correctness/allocations.json
132136
pprof_path: profiling/tests/correctness/allocations_1byte/
133137

138+
- name: Check profiler correctness for allocations with 1 byte sampling distance and `USE_ZEND_ALLOC=0`
139+
uses: Datadog/prof-correctness/analyze@main
140+
with:
141+
expected_json: profiling/tests/correctness/allocations.json
142+
pprof_path: profiling/tests/correctness/allocations_1byte_no_zend_alloc/
143+
134144
- name: Check profiler correctness for time
135145
uses: Datadog/prof-correctness/analyze@main
136146
with:

‎profiling/build.rs‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -400,10 +400,7 @@ fn cfg_php_feature_flags(vernum: u64) {
400400
if vernum >= 80400 {
401401
println!("cargo:rustc-cfg=php_frameless");
402402
println!("cargo:rustc-cfg=php_opcache_restart_hook");
403-
// Commenting the following line temporary disables the new hooking mechanism for
404-
// allocation profiling until we solved the intefereing with
405-
// `memory_get_usage()`/`memory_get_peak_usage()`
406-
// println!("cargo:rustc-cfg=php_zend_mm_set_custom_handlers_ex");
403+
println!("cargo:rustc-cfg=php_zend_mm_set_custom_handlers_ex");
407404
}
408405
}
409406

‎profiling/src/allocation/allocation_ge84.rs‎

Lines changed: 187 additions & 221 deletions
Large diffs are not rendered by default.

‎profiling/src/allocation/allocation_le83.rs‎

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::{RefCellExt, PROFILER_NAME, REQUEST_LOCALS};
77
use core::{cell::Cell, ptr};
88
use lazy_static::lazy_static;
99
use libc::{c_char, c_int, c_void, size_t};
10-
use log::{debug, error, trace, warn};
10+
use log::{debug, trace, warn};
1111
use std::sync::atomic::Ordering::Relaxed;
1212

1313
#[cfg(feature = "debug_stats")]
@@ -132,19 +132,9 @@ pub fn alloc_prof_ginit() {
132132
}
133133

134134
pub fn first_rinit_should_disable_due_to_jit() -> bool {
135-
if NEEDS_RUN_TIME_CHECK_FOR_ENABLED_JIT
135+
NEEDS_RUN_TIME_CHECK_FOR_ENABLED_JIT
136136
&& alloc_prof_needs_disabled_for_jit(crate::RUNTIME_PHP_VERSION_ID.load(Relaxed))
137137
&& *JIT_ENABLED
138-
{
139-
if zend::PHP_VERSION_ID >= 80400 {
140-
error!("Memory allocation profiling will be disabled as long as JIT is active. To enable allocation profiling disable JIT or upgrade PHP to at least version 8.4.7. See https://github.com/DataDog/dd-trace-php/pull/3199");
141-
} else {
142-
error!("Memory allocation profiling will be disabled as long as JIT is active. To enable allocation profiling disable JIT or upgrade PHP to at least version 8.1.21 or 8.2.8. See https://github.com/DataDog/dd-trace-php/pull/2088");
143-
}
144-
true
145-
} else {
146-
false
147-
}
148138
}
149139

150140
pub fn alloc_prof_rinit() {
@@ -373,7 +363,9 @@ unsafe fn alloc_prof_prev_alloc(len: size_t) -> *mut c_void {
373363
}
374364

375365
unsafe fn alloc_prof_orig_alloc(len: size_t) -> *mut c_void {
376-
let heap = zend::zend_mm_get_heap();
366+
// Safety: `ZEND_MM_STATE.heap` will be initialised in `alloc_prof_rinit()` and custom ZendMM
367+
// handlers are only installed and pointing to this function if initialization was succesful.
368+
let heap = tls_zend_mm_state_get!(heap).unwrap_unchecked();
377369
let (prepare, restore) = tls_zend_mm_state_get!(prepare_restore_zend_heap);
378370
let custom_heap = prepare(heap);
379371
let ptr: *mut c_void = zend::_zend_mm_alloc(heap, len);
@@ -398,7 +390,9 @@ unsafe fn alloc_prof_prev_free(ptr: *mut c_void) {
398390
}
399391

400392
unsafe fn alloc_prof_orig_free(ptr: *mut c_void) {
401-
let heap = zend::zend_mm_get_heap();
393+
// Safety: `ZEND_MM_STATE.heap` will be initialised in `alloc_prof_rinit()` and custom ZendMM
394+
// handlers are only installed and pointing to this function if initialization was succesful.
395+
let heap = tls_zend_mm_state_get!(heap).unwrap_unchecked();
402396
zend::_zend_mm_free(heap, ptr);
403397
}
404398

@@ -432,7 +426,9 @@ unsafe fn alloc_prof_prev_realloc(prev_ptr: *mut c_void, len: size_t) -> *mut c_
432426
}
433427

434428
unsafe fn alloc_prof_orig_realloc(prev_ptr: *mut c_void, len: size_t) -> *mut c_void {
435-
let heap = zend::zend_mm_get_heap();
429+
// Safety: `ZEND_MM_STATE.heap` will be initialised in `alloc_prof_rinit()` and custom ZendMM
430+
// handlers are only installed and pointing to this function if initialization was succesful.
431+
let heap = tls_zend_mm_state_get!(heap).unwrap_unchecked();
436432
let (prepare, restore) = tls_zend_mm_state_get!(prepare_restore_zend_heap);
437433
let custom_heap = prepare(heap);
438434
let ptr: *mut c_void = zend::_zend_mm_realloc(heap, prev_ptr, len);

‎profiling/src/allocation/mod.rs‎

Lines changed: 34 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -11,24 +11,28 @@ use crate::bindings::{self as zend};
1111
use crate::profiling::Profiler;
1212
use crate::{RefCellExt, REQUEST_LOCALS};
1313
use libc::size_t;
14-
use log::{debug, error, trace};
14+
use log::{debug, trace};
1515
use rand_distr::{Distribution, Poisson};
1616
use std::ffi::c_void;
17+
use std::num::{NonZero, NonZeroU32, NonZeroU64};
1718
use std::sync::atomic::{AtomicU64, Ordering};
1819

20+
use crate::config::SystemSettings;
1921
#[cfg(not(php_zts))]
2022
use rand::rngs::StdRng;
2123
#[cfg(php_zts)]
2224
use rand::rngs::ThreadRng;
2325
#[cfg(not(php_zts))]
2426
use rand::SeedableRng;
2527

26-
/// Default sampling interval in bytes (4MB)
27-
pub const DEFAULT_ALLOCATION_SAMPLING_INTERVAL: u64 = 1024 * 4096;
28+
/// Default sampling interval in bytes (4 MiB).
29+
// SAFETY: value is > 0.
30+
pub const DEFAULT_ALLOCATION_SAMPLING_INTERVAL: NonZeroU32 =
31+
unsafe { NonZero::new_unchecked(1024 * 4096) };
2832

29-
/// Sampling distance feed into poison sampling algo
33+
/// Sampling distance feed into poison sampling algo. This must be > 0.
3034
pub static ALLOCATION_PROFILING_INTERVAL: AtomicU64 =
31-
AtomicU64::new(DEFAULT_ALLOCATION_SAMPLING_INTERVAL);
35+
AtomicU64::new(DEFAULT_ALLOCATION_SAMPLING_INTERVAL.get() as u64);
3236

3337
/// This will store the count of allocations (including reallocations) during
3438
/// a profiling period. This will overflow when doing more than u64::MAX
@@ -53,10 +57,9 @@ pub struct AllocationProfilingStats {
5357
}
5458

5559
impl AllocationProfilingStats {
56-
fn new() -> AllocationProfilingStats {
57-
// Safety: this will only error if lambda <= 0
58-
let poisson =
59-
Poisson::new(ALLOCATION_PROFILING_INTERVAL.load(Ordering::Relaxed) as f64).unwrap();
60+
fn new(sampling_distance: NonZeroU64) -> AllocationProfilingStats {
61+
// SAFETY: this will only error if lambda <= 0, and it's NonZeroU64.
62+
let poisson = unsafe { Poisson::new(sampling_distance.get() as f64).unwrap_unchecked() };
6063
let mut stats = AllocationProfilingStats {
6164
next_sample: 0,
6265
poisson,
@@ -105,30 +108,35 @@ pub fn alloc_prof_startup() {
105108
allocation_le83::alloc_prof_startup();
106109
}
107110

108-
pub fn alloc_prof_first_rinit() {
109-
let (allocation_enabled, sampling_distance) = REQUEST_LOCALS
110-
.try_with_borrow(|locals| {
111-
let settings = locals.system_settings();
112-
(settings.profiling_allocation_enabled, settings.profiling_allocation_sampling_distance)
113-
})
114-
.unwrap_or_else(|err| {
115-
error!("Allocation profiling first rinit failed because it failed to borrow the request locals. Please report this to Datadog: {err}");
116-
(false, DEFAULT_ALLOCATION_SAMPLING_INTERVAL as u32)
117-
});
111+
pub fn first_rinit(settings: &SystemSettings) {
112+
if !settings.profiling_allocation_enabled {
113+
return;
114+
}
118115

119-
if !allocation_enabled {
116+
let sampling_distance = settings.profiling_allocation_sampling_distance;
117+
ALLOCATION_PROFILING_INTERVAL.store(sampling_distance.get() as u64, Ordering::Relaxed);
118+
119+
trace!("Memory allocation profiling initialized with a sampling distance of {sampling_distance} bytes.");
120+
}
121+
122+
/// # Safety
123+
///
124+
/// Must be called exactly once per extension minit.
125+
pub unsafe fn minit(settings: &SystemSettings) {
126+
if !settings.profiling_allocation_enabled {
120127
return;
121128
}
122129

123-
ALLOCATION_PROFILING_INTERVAL.store(sampling_distance as u64, Ordering::Relaxed);
130+
let sampling_distance = settings.profiling_allocation_sampling_distance;
131+
ALLOCATION_PROFILING_INTERVAL.store(sampling_distance.get() as u64, Ordering::Relaxed);
132+
133+
// SAFETY: called in minit.
134+
unsafe { profiling_stats::minit(sampling_distance.into()) };
124135

125-
trace!(
126-
"Memory allocation profiling initialized with a sampling distance of {} bytes.",
127-
ALLOCATION_PROFILING_INTERVAL.load(Ordering::Relaxed)
128-
);
136+
trace!("Memory allocation profiling initialized with a sampling distance of {sampling_distance} bytes.");
129137
}
130138

131-
pub fn alloc_prof_rinit() {
139+
pub fn rinit() {
132140
let allocation_enabled = REQUEST_LOCALS
133141
.try_with_borrow(|locals| locals.system_settings().profiling_allocation_enabled)
134142
.unwrap_or_else(|err| {

‎profiling/src/allocation/profiling_stats.rs‎

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,18 @@
33
//! performance sensitive. It is encapsulated so that some unsafe techniques
44
//! can be used but expose a relatively safe API.
55
6-
use super::AllocationProfilingStats;
6+
use super::{AllocationProfilingStats, ALLOCATION_PROFILING_INTERVAL};
77
use libc::size_t;
88
use std::mem::MaybeUninit;
99

10-
#[cfg(php_zts)]
11-
use std::cell::UnsafeCell;
12-
1310
#[cfg(php_zend_mm_set_custom_handlers_ex)]
1411
use super::allocation_ge84;
1512
#[cfg(not(php_zend_mm_set_custom_handlers_ex))]
1613
use super::allocation_le83;
14+
#[cfg(php_zts)]
15+
use std::cell::UnsafeCell;
16+
use std::num::NonZeroU64;
17+
use std::sync::atomic::Ordering;
1718

1819
#[cfg(php_zts)]
1920
thread_local! {
@@ -107,12 +108,15 @@ pub fn allocation_profiling_stats_should_collect(len: size_t) -> bool {
107108
/// Must be called once per PHP thread ginit.
108109
pub unsafe fn ginit() {
109110
// SAFETY:
110-
// 1. During ginit, there will not be any other borrows.
111-
// 2. This closure will not make new borrows.
111+
// 1. During ginit, there will not be any other borrows to stats.
112+
// 2. This closure will not make new borrows to stats.
112113
// 3. This is not during the thread-local destructor.
113114
unsafe {
114115
allocation_profiling_stats_mut(|uninit| {
115-
uninit.write(AllocationProfilingStats::new());
116+
let interval = ALLOCATION_PROFILING_INTERVAL.load(Ordering::Relaxed);
117+
// SAFETY: ALLOCATION_PROFILING_INTERVAL must always be > 0.
118+
let nonzero = NonZeroU64::new_unchecked(interval);
119+
uninit.write(AllocationProfilingStats::new(nonzero));
116120
})
117121
};
118122

@@ -122,15 +126,33 @@ pub unsafe fn ginit() {
122126
allocation_ge84::alloc_prof_ginit();
123127
}
124128

129+
/// Initializes the allocation profiler's globals with the provided sampling
130+
/// distance.
131+
///
132+
/// # Safety
133+
///
134+
/// Must be called once per PHP thread minit, unless the allocation profiling
135+
/// is disabled, in which case it can be skipped.
136+
pub unsafe fn minit(sampling_distance: NonZeroU64) {
137+
// SAFETY:
138+
// 1. During minit, there will not be any other borrows.
139+
// 2. This closure will not make new borrows.
140+
// 3. This is not during the thread-local destructor.
141+
unsafe {
142+
allocation_profiling_stats_mut(|uninit| {
143+
// SAFETY: previously initialized in ginit, we're just
144+
// re-initializing it because we now have config
145+
*uninit.assume_init_mut() = AllocationProfilingStats::new(sampling_distance);
146+
})
147+
};
148+
}
149+
125150
/// Shuts down the allocation profiler's globals.
126151
///
127152
/// # Safety
128153
///
129154
/// Must be called once per PHP thread gshutdown.
130155
pub unsafe fn gshutdown() {
131-
#[cfg(php_zend_mm_set_custom_handlers_ex)]
132-
allocation_ge84::alloc_prof_gshutdown();
133-
134156
// SAFETY:
135157
// 1. During gshutdown, there will not be any other borrows.
136158
// 2. This closure will not make new borrows.

0 commit comments

Comments
 (0)