Skip to content

Commit a8cd140

Browse files
realFlowControlKimi
andauthored
refactor(profiling): remove lazy_static, optimize Sapi/RefCellExt (#3990)
Co-authored-by: Kimi <noreply@moonshot.ai>
1 parent 36d5401 commit a8cd140

6 files changed

Lines changed: 81 additions & 93 deletions

File tree

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
@@ -30,7 +30,6 @@ libdd-profiling = { path = "../libdatadog/libdd-profiling" }
3030
libdd-common = { path = "../libdatadog/libdd-common" }
3131
libdd-library-config-ffi = { path = "../libdatadog/libdd-library-config-ffi" }
3232
env_logger = { version = "0.11", default-features = false }
33-
lazy_static = { version = "1.4" }
3433
libc = "0.2"
3534
# TRACE set to max to support runtime configuration.
3635
log = { version = "0.4", features = ["max_level_trace", "release_max_level_trace"]}

profiling/src/allocation/allocation_ge84.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@ use crate::allocation::{
44
use crate::bindings as zend;
55
use crate::PROFILER_NAME;
66
use core::ptr;
7-
use lazy_static::lazy_static;
87
use libc::{c_char, c_int, c_void, size_t};
98
use log::{debug, trace, warn};
109
use std::sync::atomic::Ordering::Relaxed;
10+
use std::sync::LazyLock;
1111

1212
#[cfg(php_debug)]
1313
use libc::c_uint;
@@ -94,9 +94,7 @@ fn alloc_prof_needs_disabled_for_jit(version: u32) -> bool {
9494
(80400..80407).contains(&version)
9595
}
9696

97-
lazy_static! {
98-
static ref JIT_ENABLED: bool = unsafe { zend::ddog_php_jit_enabled() };
99-
}
97+
static JIT_ENABLED: LazyLock<bool> = LazyLock::new(|| unsafe { zend::ddog_php_jit_enabled() });
10098

10199
pub fn alloc_prof_ginit() {
102100
unsafe { zend::ddog_php_opcache_init_handle() };

profiling/src/allocation/allocation_le83.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ use crate::bindings::{
77
};
88
use crate::{RefCellExt, PROFILER_NAME, REQUEST_LOCALS};
99
use core::ptr;
10-
use lazy_static::lazy_static;
1110
use libc::{c_char, c_int, c_void, size_t};
1211
use log::{debug, trace, warn};
1312
use std::sync::atomic::Ordering::Relaxed;
13+
use std::sync::LazyLock;
1414

1515
#[cfg(feature = "debug_stats")]
1616
use crate::allocation::{ALLOCATION_PROFILING_COUNT, ALLOCATION_PROFILING_SIZE};
@@ -77,9 +77,7 @@ fn alloc_prof_needs_disabled_for_jit(version: u32) -> bool {
7777
|| (80400..80407).contains(&version)
7878
}
7979

80-
lazy_static! {
81-
static ref JIT_ENABLED: bool = unsafe { zend::ddog_php_jit_enabled() };
82-
}
80+
static JIT_ENABLED: LazyLock<bool> = LazyLock::new(|| unsafe { zend::ddog_php_jit_enabled() });
8381

8482
pub fn alloc_prof_ginit() {
8583
unsafe { zend::ddog_php_opcache_init_handle() };

profiling/src/lib.rs

Lines changed: 65 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ use bindings::{
3535
use clocks::*;
3636
use core::ffi::{c_char, c_int, CStr};
3737
use core::ptr;
38-
use lazy_static::lazy_static;
3938
use libdd_common::{cstr, tag, tag::Tag};
4039
use log::{debug, error, info, trace, warn};
4140
use profiling::{LocalRootSpanResourceMessage, Profiler, VmInterrupt};
@@ -44,7 +43,7 @@ use std::borrow::Cow;
4443
use std::cell::{BorrowError, BorrowMutError, RefCell};
4544
use std::ops::{Deref, DerefMut};
4645
use std::sync::atomic::{AtomicBool, AtomicU32, Ordering};
47-
use std::sync::{Arc, Once, OnceLock};
46+
use std::sync::{Arc, LazyLock, Once, OnceLock};
4847
use std::thread::{AccessError, LocalKey};
4948
use std::time::{Duration, Instant};
5049
use uuid::Uuid;
@@ -75,73 +74,73 @@ static mut RUNTIME_PHP_VERSION: &str = {
7574
}
7675
};
7776

78-
lazy_static! {
79-
/// # Safety
80-
/// The first time this is accessed must be after config is initialized in
81-
/// the first RINIT and before mshutdown!
82-
static ref GLOBAL_TAGS: Vec<Tag> = {
83-
let mut tags = vec![
84-
tag!("language", "php"),
85-
tag!("profiler_version", env!("PROFILER_VERSION")),
86-
// SAFETY: calling getpid() is safe.
87-
Tag::new("process_id", unsafe { libc::getpid() }.to_string())
88-
.expect("process_id tag to be valid"),
89-
Tag::new("runtime-id", runtime_id().to_string()).expect("runtime-id tag to be valid"),
90-
];
91-
92-
// This should probably be "language_version", but this is the
93-
// standardized tag name.
94-
// SAFETY: PHP_VERSION is safe to access in rinit (only
95-
// mutated during minit).
96-
add_tag(&mut tags, "runtime_version", unsafe { RUNTIME_PHP_VERSION });
97-
add_tag(&mut tags, "php.sapi", SAPI.as_ref());
98-
// In case we ever add PHP debug build support, we should add `zend-zts-debug` and
99-
// `zend-nts-debug`. For the time being we only support `zend-zts-ndebug` and
100-
// `zend-nts-ndebug`
101-
let runtime_engine = if cfg!(php_zts) {
102-
"zend-zts-ndebug"
103-
} else {
104-
"zend-nts-ndebug"
105-
};
106-
add_tag(&mut tags, "runtime_engine", runtime_engine);
107-
tags
77+
/// # Safety
78+
/// The first time this is accessed must be after config is initialized in
79+
/// the first RINIT and before mshutdown!
80+
static GLOBAL_TAGS: LazyLock<Vec<Tag>> = LazyLock::new(|| {
81+
let mut tags = vec![
82+
tag!("language", "php"),
83+
tag!("profiler_version", env!("PROFILER_VERSION")),
84+
// SAFETY: calling getpid() is safe.
85+
Tag::new("process_id", unsafe { libc::getpid() }.to_string())
86+
.expect("process_id tag to be valid"),
87+
Tag::new("runtime-id", runtime_id().to_string()).expect("runtime-id tag to be valid"),
88+
];
89+
90+
// This should probably be "language_version", but this is the
91+
// standardized tag name.
92+
// SAFETY: PHP_VERSION is safe to access in rinit (only
93+
// mutated during minit).
94+
add_tag(&mut tags, "runtime_version", unsafe { RUNTIME_PHP_VERSION });
95+
add_tag(&mut tags, "php.sapi", SAPI.as_ref());
96+
// In case we ever add PHP debug build support, we should add `zend-zts-debug` and
97+
// `zend-nts-debug`. For the time being we only support `zend-zts-ndebug` and
98+
// `zend-nts-ndebug`
99+
let runtime_engine = if cfg!(php_zts) {
100+
"zend-zts-ndebug"
101+
} else {
102+
"zend-nts-ndebug"
108103
};
104+
add_tag(&mut tags, "runtime_engine", runtime_engine);
105+
tags
106+
});
109107

110-
/// The Server API the profiler is running under.
111-
static ref SAPI: Sapi = {
112-
#[cfg(not(test))]
113-
{
114-
// SAFETY: sapi_module is initialized before minit and there should be
115-
// no concurrent threads.
116-
let sapi_module = unsafe { zend::sapi_module };
117-
if sapi_module.name.is_null() {
118-
panic!("the sapi_module's name is a null pointer");
119-
}
120-
121-
// SAFETY: value has been checked for NULL; I haven't checked that the
122-
// engine ensures its length is less than `isize::MAX`, but it is a
123-
// risk I'm willing to take.
124-
let sapi_name = unsafe { CStr::from_ptr(sapi_module.name) };
125-
Sapi::from_name(sapi_name.to_string_lossy().as_ref())
126-
}
127-
// When running `cargo test` we do not link against PHP, so `zend::sapi_name` is not
128-
// available and we just return `Sapi::Unkown`
129-
#[cfg(test)]
130-
{
131-
Sapi::Unknown
108+
/// The Server API the profiler is running under.
109+
static SAPI: LazyLock<Sapi> = LazyLock::new(|| {
110+
#[cfg(not(test))]
111+
{
112+
// SAFETY: sapi_module is initialized before minit and there should be
113+
// no concurrent threads.
114+
let sapi_module = unsafe { zend::sapi_module };
115+
if sapi_module.name.is_null() {
116+
panic!("the sapi_module's name is a null pointer");
132117
}
133-
};
134118

135-
// SAFETY: PROFILER_NAME is a byte slice that satisfies the safety requirements.
136-
// Panic: we own this string and it should be UTF8 (see PROFILER_NAME above).
137-
static ref PROFILER_NAME_STR: &'static str = PROFILER_NAME.to_str().unwrap();
119+
// SAFETY: value has been checked for NULL; I haven't checked that the
120+
// engine ensures its length is less than `isize::MAX`, but it is a
121+
// risk I'm willing to take.
122+
let sapi_name = unsafe { CStr::from_ptr(sapi_module.name) };
123+
Sapi::from_name(sapi_name.to_string_lossy().as_ref())
124+
}
125+
// When running `cargo test` we do not link against PHP, so `zend::sapi_name` is not
126+
// available and we just return `Sapi::Unkown`
127+
#[cfg(test)]
128+
{
129+
Sapi::Unknown
130+
}
131+
});
132+
133+
// SAFETY: PROFILER_NAME is a byte slice that satisfies the safety requirements.
134+
// Panic: we own this string and it should be UTF8 (see PROFILER_NAME above).
135+
static PROFILER_NAME_STR: LazyLock<&'static str> = LazyLock::new(|| PROFILER_NAME.to_str().unwrap());
138136

139-
// SAFETY: PROFILER_VERSION is a byte slice that satisfies the safety requirements.
140-
static ref PROFILER_VERSION_STR: &'static str = unsafe { CStr::from_ptr(PROFILER_VERSION.as_ptr() as *const c_char) }
137+
// SAFETY: PROFILER_VERSION is a byte slice that satisfies the safety requirements.
138+
static PROFILER_VERSION_STR: LazyLock<&'static str> = LazyLock::new(|| {
139+
unsafe { CStr::from_ptr(PROFILER_VERSION.as_ptr() as *const c_char) }
141140
.to_str()
142141
// Panic: we own this string and it should be UTF8 (see PROFILER_VERSION above).
143-
.unwrap();
144-
}
142+
.unwrap()
143+
});
145144

146145
/// The runtime ID, which is basically a universally unique "pid". This makes
147146
/// it almost const, the exception being to re-initialize it from a child fork
@@ -472,13 +471,15 @@ trait RefCellExt<T> {
472471
where
473472
F: FnOnce(&mut T) -> R;
474473

474+
#[inline]
475475
fn borrow_or_false<F>(&'static self, f: F) -> bool
476476
where
477477
F: FnOnce(&T) -> bool,
478478
{
479479
self.try_with_borrow(f).unwrap_or(false)
480480
}
481481

482+
#[inline]
482483
fn borrow_mut_or_false<F>(&'static self, f: F) -> bool
483484
where
484485
F: FnOnce(&mut T) -> bool,
@@ -488,6 +489,7 @@ trait RefCellExt<T> {
488489
}
489490

490491
impl<T> RefCellExt<T> for LocalKey<RefCell<T>> {
492+
#[inline]
491493
fn try_with_borrow<F, R>(&'static self, f: F) -> Result<R, RefCellExtError>
492494
where
493495
F: FnOnce(&T) -> R,
@@ -497,6 +499,7 @@ impl<T> RefCellExt<T> for LocalKey<RefCell<T>> {
497499
})??)
498500
}
499501

502+
#[inline]
500503
fn try_with_borrow_mut<F, R>(&'static self, f: F) -> Result<R, RefCellExtError>
501504
where
502505
F: FnOnce(&mut T) -> R,

profiling/src/sapi.rs

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

119
// todo: unify with ../component/sapi
1210
#[derive(Copy, Clone, Eq, PartialEq)]
@@ -27,25 +25,18 @@ pub enum Sapi {
2725

2826
impl Sapi {
2927
pub fn from_name(name: &str) -> Sapi {
30-
static SAPIS: OnceLock<HashMap<&str, Sapi>> = OnceLock::new();
31-
let sapis = SAPIS.get_or_init(|| {
32-
HashMap::from_iter([
33-
("apache2handler", Sapi::Apache2Handler),
34-
("cgi-fcgi", Sapi::CgiFcgi),
35-
("cli", Sapi::Cli),
36-
("cli-server", Sapi::CliServer),
37-
("embed", Sapi::Embed),
38-
("frankenphp", Sapi::FrankenPHP),
39-
("fpm-fcgi", Sapi::FpmFcgi),
40-
("litespeed", Sapi::Litespeed),
41-
("phpdbg", Sapi::PhpDbg),
42-
("tea", Sapi::Tea),
43-
])
44-
});
45-
46-
match sapis.get(name) {
47-
None => Sapi::Unknown,
48-
Some(sapi) => *sapi,
28+
match name {
29+
"apache2handler" => Sapi::Apache2Handler,
30+
"cgi-fcgi" => Sapi::CgiFcgi,
31+
"cli" => Sapi::Cli,
32+
"cli-server" => Sapi::CliServer,
33+
"embed" => Sapi::Embed,
34+
"frankenphp" => Sapi::FrankenPHP,
35+
"fpm-fcgi" => Sapi::FpmFcgi,
36+
"litespeed" => Sapi::Litespeed,
37+
"phpdbg" => Sapi::PhpDbg,
38+
"tea" => Sapi::Tea,
39+
_ => Sapi::Unknown,
4940
}
5041
}
5142

0 commit comments

Comments
 (0)