Skip to content

Commit ff8f987

Browse files
authored
build(profiling): remove once_cell as a dependency (#3607)
Since Rust 1.70 there are types in core we can use, or in some cases we can avoid the type and rely on get_module as a place to run the bits that are non-const.
1 parent 21d2f00 commit ff8f987

File tree

7 files changed

+71
-67
lines changed

7 files changed

+71
-67
lines changed

Cargo.lock

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

profiling/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ lazy_static = { version = "1.4" }
3131
libc = "0.2"
3232
# TRACE set to max to support runtime configuration.
3333
log = { version = "0.4", features = ["max_level_trace", "release_max_level_trace"]}
34-
once_cell = { version = "1.12" }
3534
serde_json = {version = "1.0"}
3635
rand = { version = "0.8.5" }
3736
rand_distr = { version = "0.4.3" }

profiling/src/bindings/mod.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -246,13 +246,16 @@ pub struct ZendExtension {
246246

247247
pub use ZendExtension as zend_extension;
248248

249-
impl Default for ModuleEntry {
250-
fn default() -> Self {
249+
impl ModuleEntry {
250+
/// Creates a new ModuleEntry with default values for const-compatible fields.
251+
/// Non-const fields (functions, build_id) are set to null and should be initialized separately.
252+
#[allow(clippy::new_without_default)]
253+
pub const fn new() -> Self {
251254
Self {
252-
size: std::mem::size_of::<Self>() as c_ushort,
255+
size: core::mem::size_of::<Self>() as c_ushort,
253256
zend_api: ZEND_MODULE_API_NO,
254-
zend_debug: ZEND_DEBUG as u8,
255-
zts: USING_ZTS as u8,
257+
zend_debug: ZEND_DEBUG as c_uchar,
258+
zts: USING_ZTS as c_uchar,
256259
ini_entry: ptr::null(),
257260
deps: ptr::null(),
258261
name: c"".as_ptr(),
@@ -264,20 +267,18 @@ impl Default for ModuleEntry {
264267
info_func: None,
265268
version: ptr::null(),
266269
globals_size: 0,
267-
268270
#[cfg(php_zts)]
269271
globals_id_ptr: ptr::null_mut(),
270272
#[cfg(not(php_zts))]
271273
globals_ptr: ptr::null_mut(),
272-
273274
globals_ctor: None,
274275
globals_dtor: None,
275276
post_deactivate_func: None,
276277
module_started: 0,
277278
type_: MODULE_PERSISTENT as c_uchar,
278279
handle: ptr::null_mut(),
279280
module_number: -1,
280-
build_id: unsafe { datadog_module_build_id() },
281+
build_id: ptr::null(),
281282
}
282283
}
283284
}

profiling/src/lib.rs

Lines changed: 56 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,13 @@ use core::ptr;
3434
use lazy_static::lazy_static;
3535
use libdd_common::{cstr, tag, tag::Tag};
3636
use log::{debug, error, info, trace, warn};
37-
use once_cell::sync::{Lazy, OnceCell};
3837
use profiling::{LocalRootSpanResourceMessage, Profiler, VmInterrupt};
3938
use sapi::Sapi;
4039
use std::borrow::Cow;
4140
use std::cell::{BorrowError, BorrowMutError, RefCell};
4241
use std::ops::{Deref, DerefMut};
4342
use std::sync::atomic::{AtomicBool, AtomicU32, Ordering};
44-
use std::sync::{Arc, Once};
43+
use std::sync::{Arc, Once, OnceLock};
4544
use std::thread::{AccessError, LocalKey};
4645
use std::time::{Duration, Instant};
4746
use uuid::Uuid;
@@ -146,7 +145,7 @@ lazy_static! {
146145
/// Additionally, the tracer is going to ask for this in its ACTIVATE handler,
147146
/// so whatever it is replaced with needs to also follow the
148147
/// initialize-on-first-use pattern.
149-
static RUNTIME_ID: OnceCell<Uuid> = OnceCell::new();
148+
static RUNTIME_ID: OnceLock<Uuid> = OnceLock::new();
150149
// If ddtrace is loaded, we fetch the uuid from there instead
151150
extern "C" {
152151
pub static ddtrace_runtime_id: *const Uuid;
@@ -159,55 +158,62 @@ extern "C" {
159158
#[cfg(php_zts)]
160159
static mut GLOBALS_ID_PTR: i32 = 0;
161160

162-
/// The function `get_module` is what makes this a PHP module. Please do not
163-
/// call this directly; only let it be called by the engine. Generally it is
164-
/// only called once, but if someone accidentally loads the module twice then
165-
/// it might get called more than once, though it will warn and not use the
166-
/// consecutive return value.
161+
/// Module dependencies for the profiler extension.
162+
static MODULE_DEPS: [zend::ModuleDep; 8] = [
163+
zend::ModuleDep::required(cstr!("standard")),
164+
zend::ModuleDep::required(cstr!("json")),
165+
zend::ModuleDep::optional(cstr!("ddtrace")),
166+
// Optionally, be dependent on these event extensions so that the functions they provide
167+
// are registered in the function table and we can hook into them.
168+
zend::ModuleDep::optional(cstr!("ev")),
169+
zend::ModuleDep::optional(cstr!("event")),
170+
zend::ModuleDep::optional(cstr!("libevent")),
171+
zend::ModuleDep::optional(cstr!("uv")),
172+
zend::ModuleDep::end(),
173+
];
174+
175+
/// The module entry for the profiler extension. Fields that aren't
176+
/// const-compatible are set in get_module().
177+
static mut MODULE: zend::ModuleEntry = zend::ModuleEntry {
178+
deps: MODULE_DEPS.as_ptr(),
179+
name: PROFILER_NAME.as_ptr(),
180+
functions: ptr::null(), // Will be set in get_module()
181+
module_startup_func: Some(minit),
182+
module_shutdown_func: Some(mshutdown),
183+
request_startup_func: Some(rinit),
184+
request_shutdown_func: Some(rshutdown),
185+
info_func: Some(minfo),
186+
version: PROFILER_VERSION.as_ptr(),
187+
globals_size: 1,
188+
#[cfg(php_zts)]
189+
globals_id_ptr: ptr::addr_of_mut!(GLOBALS_ID_PTR),
190+
#[cfg(not(php_zts))]
191+
globals_ptr: ptr::null_mut(),
192+
globals_ctor: Some(ginit),
193+
globals_dtor: Some(gshutdown),
194+
post_deactivate_func: Some(prshutdown),
195+
build_id: ptr::null(), // Will be set in get_module()
196+
..zend::ModuleEntry::new()
197+
};
198+
199+
/// The function `get_module` is what makes this a PHP module.
200+
///
201+
/// # Safety
202+
///
203+
/// Do not call this function manually; it will be called by the engine.
204+
/// Generally it is only called once, but if someone accidentally loads the
205+
/// module twice then it might get called more than once, though it will warn
206+
/// and not use the consecutive return value.
167207
#[no_mangle]
168-
pub extern "C" fn get_module() -> &'static mut zend::ModuleEntry {
169-
static DEPS: [zend::ModuleDep; 8] = [
170-
zend::ModuleDep::required(cstr!("standard")),
171-
zend::ModuleDep::required(cstr!("json")),
172-
zend::ModuleDep::optional(cstr!("ddtrace")),
173-
// Optionally, be dependent on these event extensions so that the functions they provide
174-
// are registered in the function table and we can hook into them.
175-
zend::ModuleDep::optional(cstr!("ev")),
176-
zend::ModuleDep::optional(cstr!("event")),
177-
zend::ModuleDep::optional(cstr!("libevent")),
178-
zend::ModuleDep::optional(cstr!("uv")),
179-
zend::ModuleDep::end(),
180-
];
181-
182-
// In PHP modules written in C, this just returns the address of a global,
183-
// mutable variable. In Rust, you cannot initialize such a complicated
184-
// global variable because of initialization order issues that have been
185-
// found through decades of C++ experience.
186-
// There are a variety of ways to deal with this; this is just one way.
187-
static mut MODULE: Lazy<zend::ModuleEntry> = Lazy::new(|| zend::ModuleEntry {
188-
name: PROFILER_NAME.as_ptr(),
189-
// SAFETY: php_ffi.c defines this correctly
190-
functions: unsafe { bindings::ddog_php_prof_functions },
191-
module_startup_func: Some(minit),
192-
module_shutdown_func: Some(mshutdown),
193-
request_startup_func: Some(rinit),
194-
request_shutdown_func: Some(rshutdown),
195-
info_func: Some(minfo),
196-
version: PROFILER_VERSION.as_ptr(),
197-
post_deactivate_func: Some(prshutdown),
198-
deps: DEPS.as_ptr(),
199-
globals_ctor: Some(ginit),
200-
globals_dtor: Some(gshutdown),
201-
globals_size: 1,
202-
#[cfg(php_zts)]
203-
globals_id_ptr: ptr::addr_of_mut!(GLOBALS_ID_PTR),
204-
#[cfg(not(php_zts))]
205-
globals_ptr: ptr::null_mut(),
206-
..Default::default()
207-
});
208+
pub unsafe extern "C" fn get_module() -> *mut zend::ModuleEntry {
209+
let module = ptr::addr_of_mut!(MODULE);
208210

209-
// SAFETY: well, it's at least as safe as what every single C extension does.
210-
unsafe { &mut *ptr::addr_of_mut!(MODULE) }
211+
// Set fields that aren't const-compatible.
212+
unsafe {
213+
ptr::addr_of_mut!((*module).functions).write(bindings::ddog_php_prof_functions);
214+
ptr::addr_of_mut!((*module).build_id).write(bindings::datadog_module_build_id());
215+
}
216+
module
211217
}
212218

213219
unsafe extern "C" fn ginit(_globals_ptr: *mut c_void) {

profiling/src/profiling/mod.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,12 @@ use libdd_profiling::api::{
3030
use libdd_profiling::exporter::Tag;
3131
use libdd_profiling::internal::Profile as InternalProfile;
3232
use log::{debug, info, trace, warn};
33-
use once_cell::sync::OnceCell;
3433
use std::borrow::Cow;
3534
use std::collections::HashMap;
3635
use std::hash::Hash;
3736
use std::num::NonZeroI64;
3837
use std::sync::atomic::{AtomicBool, AtomicPtr, AtomicU32, Ordering};
39-
use std::sync::{Arc, Barrier};
38+
use std::sync::{Arc, Barrier, OnceLock};
4039
use std::thread::JoinHandle;
4140
use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};
4241

@@ -58,7 +57,7 @@ const UPLOAD_CHANNEL_CAPACITY: usize = 8;
5857

5958
/// The global profiler. Profiler gets made during the first rinit after an
6059
/// minit, and is destroyed on mshutdown.
61-
static mut PROFILER: OnceCell<Profiler> = OnceCell::new();
60+
static mut PROFILER: OnceLock<Profiler> = OnceLock::new();
6261

6362
/// Order this array this way:
6463
/// 1. Always enabled types.
@@ -657,7 +656,7 @@ const DDPROF_TIME: &str = "ddprof_time";
657656
const DDPROF_UPLOAD: &str = "ddprof_upload";
658657

659658
impl Profiler {
660-
/// Will initialize the `PROFILER` OnceCell and makes sure that only one thread will do so.
659+
/// Will initialize the `PROFILER` OnceLock and makes sure that only one thread will do so.
661660
pub fn init(system_settings: &SystemSettings) {
662661
// SAFETY: the `get_or_init` access is a thread-safe API, and the
663662
// PROFILER is only being mutated in single-threaded phases such as

profiling/src/profiling/thread_utils.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::SAPI;
22
use libc::sched_yield;
3-
use once_cell::sync::OnceCell;
3+
use std::cell::OnceCell;
44
use std::mem::MaybeUninit;
55
use std::thread::JoinHandle;
66
use std::time::{Duration, Instant};

profiling/src/sapi.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
use crate::zend::sapi_request_info;
22
use log::warn;
3-
use once_cell::sync::OnceCell;
43
use std::borrow::Cow;
54
use std::collections::HashMap;
65
use std::ffi::{CStr, OsStr};
76
use std::fmt::{Display, Formatter};
87
use std::os::unix::ffi::OsStrExt;
98
use std::path::Path;
9+
use std::sync::OnceLock;
1010

1111
// todo: unify with ../component/sapi
1212
#[derive(Copy, Clone, Eq, PartialEq)]
@@ -27,7 +27,7 @@ pub enum Sapi {
2727

2828
impl Sapi {
2929
pub fn from_name(name: &str) -> Sapi {
30-
static SAPIS: OnceCell<HashMap<&str, Sapi>> = OnceCell::new();
30+
static SAPIS: OnceLock<HashMap<&str, Sapi>> = OnceLock::new();
3131
let sapis = SAPIS.get_or_init(|| {
3232
HashMap::from_iter([
3333
("apache2handler", Sapi::Apache2Handler),

0 commit comments

Comments
 (0)