perf(profiling): use an arena-based string table

.github/workflows/prof_asan.yml

@@ -40,7 +40,9 @@ jobs:
           export LDFLAGS='-fsanitize=address'
           export RUSTC_LINKER=lld-16
           triplet=$(uname -m)-unknown-linux-gnu
-          RUSTFLAGS='-Zsanitizer=address' cargo +nightly-2023-05-03 build -Zbuild-std --target $triplet --release
+          rustup install nightly-2024-02-27
+          rustup component add rust-src --toolchain nightly-2024-02-27-x86_64-unknown-linux-gnu
+          RUSTFLAGS='-Zsanitizer=address' cargo +nightly-2024-02-27 build -Zbuild-std --target $triplet --release
           cp -v "$CARGO_TARGET_DIR/$triplet/release/libdatadog_php_profiling.so" "$(php-config --extension-dir)/datadog-profiling.so"
 
       - name: Cache build dependencies

profiling/Cargo.toml

@@ -3,7 +3,7 @@ name = "datadog-php-profiling"
 version = "0.98.0"
 edition = "2021"
 license = "Apache-2.0"
-rust-version = "1.64"
+rust-version = "1.71"
 
 [lib]
 crate-type = ["cdylib"]
@@ -12,24 +12,24 @@ bench = false # disables cargo build in libtest bench
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
-# with ahash 0.8.7 we run into https://github.com/tkaitchuck/aHash/issues/195
-# should be fixed with updating Rust to 1.72
-ahash = { version = "=0.8.6" }
+ahash = { version = "0.8.9" }
 anyhow = { version = "1.0" }
 bumpalo = { version = "3.12", features = ["collections"] }
 cfg-if = { version = "1.0" }
 cpu-time = { version = "1.0" }
 crossbeam-channel = { version = "0.5", default-features = false, features = ["std"] }
-datadog-profiling = { git = "https://github.com/DataDog/libdatadog", tag = "v6.0.0" }
-ddcommon = { git = "https://github.com/DataDog/libdatadog", tag = "v6.0.0" }
+datadog-profiling = { git = "https://github.com/DataDog/libdatadog", rev = "a0c0ed111f8afcb96d346c86383b0164ed89ad83" }
+ddcommon = { git = "https://github.com/DataDog/libdatadog", rev = "a0c0ed111f8afcb96d346c86383b0164ed89ad83" }
 env_logger = { version = "0.10" }
+hashbrown = { version = "0.14.3", default-features = false}
 indexmap = { version = "2.2.0" }
 lazy_static = { version = "1.4" }
 libc = "0.2"
 # TRACE set to max to support runtime configuration.
 log = { version = "0.4", features = ["max_level_trace", "release_max_level_trace"]}
 once_cell = { version = "1.12" }
 ouroboros = { version = "0.17.0" }
+rustc-hash = "1.1.0"
 serde_json = {version = "1.0"}
 rand = { version = "0.8.5" }
 rand_distr = { version = "0.4.3" }

profiling/benches/stack_walking.rs

@@ -4,13 +4,18 @@ use datadog_php_profiling::profiling::stalk_walking::collect_stack_sample;
 
 #[cfg(all(target_arch = "x86_64", target_os = "linux"))]
 use criterion_perf_events::Perf;
+use datadog_php_profiling::profiling::activate_run_time_cache;
 #[cfg(all(target_arch = "x86_64", target_os = "linux"))]
 use perfcnt::linux::HardwareEventType as Hardware;
 #[cfg(all(target_arch = "x86_64", target_os = "linux"))]
 use perfcnt::linux::PerfCounterBuilderLinux as Builder;
 
 fn benchmark(c: &mut Criterion) {
     let mut group = c.benchmark_group("walk_stack");
+    #[cfg(php_run_time_cache)]
+    unsafe {
+        activate_run_time_cache()
+    };
     group.sampling_mode(SamplingMode::Flat);
     for depth in [1, 50, 99].iter() {
         let stack = unsafe { zend::ddog_php_test_create_fake_zend_execute_data(99) };

profiling/src/config.rs

@@ -168,7 +168,7 @@ impl Default for AgentEndpoint {
     }
 }
 
-impl TryFrom<AgentEndpoint> for datadog_profiling::exporter::Endpoint {
+impl TryFrom<AgentEndpoint> for ddcommon::Endpoint {
     type Error = anyhow::Error;
 
     fn try_from(value: AgentEndpoint) -> Result<Self, Self::Error> {
@@ -179,7 +179,7 @@ impl TryFrom<AgentEndpoint> for datadog_profiling::exporter::Endpoint {
     }
 }
 
-impl TryFrom<&AgentEndpoint> for datadog_profiling::exporter::Endpoint {
+impl TryFrom<&AgentEndpoint> for ddcommon::Endpoint {
     type Error = anyhow::Error;
 
     fn try_from(value: &AgentEndpoint) -> Result<Self, Self::Error> {
@@ -1001,7 +1001,6 @@ pub(crate) unsafe fn on_fork_in_child() {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use core::mem::MaybeUninit;
     use libc::memcmp;
 
     #[test]

profiling/src/lib.rs

@@ -6,7 +6,6 @@ mod logging;
 mod pcntl;
 pub mod profiling;
 mod sapi;
-mod string_table;
 
 #[cfg(feature = "allocation_profiling")]
 mod allocation;
@@ -17,6 +16,7 @@ mod exception;
 #[cfg(feature = "timeline")]
 mod timeline;
 
+mod string_set;
 mod wall_time;
 
 use crate::config::{SystemSettings, INITIAL_SYSTEM_SETTINGS};
@@ -326,8 +326,8 @@ pub struct RequestLocals {
 
     /// SystemSettings are global. Note that if this is being read in fringe
     /// conditions such as in mshutdown when there were no requests served,
-    /// then the settings are still memory safe but they may not have the real
-    /// configuration. Instead they have a best-effort values such as
+    /// then the settings are still memory safe, but they may not have the real
+    /// configuration. Instead, they have a best-effort values such as
     /// INITIAL_SYSTEM_SETTINGS, or possibly the values which were available
     /// in MINIT.
     pub system_settings: ptr::NonNull<SystemSettings>,
@@ -583,7 +583,7 @@ extern "C" fn rshutdown(_type: c_int, _module_number: c_int) -> ZendResult {
     #[cfg(php_run_time_cache)]
     {
         profiling::FUNCTION_CACHE_STATS.with(|cell| {
-            let stats = cell.borrow();
+            let stats = cell.get();
             let hit_rate = stats.hit_rate();
             debug!("Process cumulative {stats:?} hit_rate: {hit_rate}");
         });

profiling/src/logging.rs

@@ -1,5 +1,4 @@
 use env_logger::Target;
-pub use log::Level;
 pub use log::LevelFilter;
 use std::fs::File;
 use std::os::unix::io::FromRawFd;

profiling/src/profiling/mod.rs

@@ -263,7 +263,10 @@ impl TimeCollector {
     /// makes sense to use an older time than now because if the profiler was
     /// running 4 seconds ago and we're only creating a profile now, that means
     /// we didn't collect any samples during that 4 seconds.
-    fn create_profile(message: &SampleMessage, started_at: SystemTime) -> InternalProfile {
+    fn create_profile(
+        message: &SampleMessage,
+        started_at: SystemTime,
+    ) -> anyhow::Result<InternalProfile> {
         let sample_types: Vec<ApiValueType> = message
             .key
             .sample_types
@@ -299,7 +302,8 @@ impl TimeCollector {
                 },
                 value: period.min(i64::MAX as u128) as i64,
             }),
-        );
+            512 * 1024 * 1024,
+        )?;
 
         #[cfg(feature = "allocation_profiling")]
         if let (Some(alloc_size_offset), Some(alloc_samples_offset)) =
@@ -333,7 +337,7 @@ impl TimeCollector {
             }
         }
 
-        profile
+        Ok(profile)
     }
 
     fn handle_resource_message(
@@ -348,8 +352,16 @@ impl TimeCollector {
         let local_root_span_id = message.local_root_span_id;
         for (_, profile) in profiles.iter_mut() {
             let endpoint = Cow::Borrowed(message.resource.as_str());
-            profile.add_endpoint(local_root_span_id, endpoint.clone());
-            profile.add_endpoint_count(endpoint, 1);
+            match profile.add_endpoint(local_root_span_id, endpoint.clone()) {
+                // todo: the count being broadcast to all profiles is probably
+                //       incorrect because it will be over-counted.
+                Ok(_) => profile.add_endpoint_count(endpoint, 1),
+                Err(err) => {
+                    debug!(
+                        "failed to add endpoint for local root span id {local_root_span_id}: {err}"
+                    )
+                }
+            }
         }
     }
 
@@ -367,10 +379,11 @@ impl TimeCollector {
         let profile: &mut InternalProfile = if let Some(value) = profiles.get_mut(&message.key) {
             value
         } else {
-            profiles.insert(
-                message.key.clone(),
-                Self::create_profile(&message, started_at.systemtime),
-            );
+            let Ok(profile) = Self::create_profile(&message, started_at.systemtime) else {
+                error!("Failed to create a new profile. Please report this to Datadog.");
+                return;
+            };
+            profiles.insert(message.key.clone(), profile);
             profiles
                 .get_mut(&message.key)
                 .expect("entry to exist; just inserted it")
@@ -407,7 +420,9 @@ impl TimeCollector {
         match profile.add_sample(sample, timestamp) {
             Ok(_id) => {}
             Err(err) => {
-                warn!("Failed to add sample to the profile: {err}")
+                // If the string table becomes full, this could flood customer
+                // logs, which is why I've chosen this level.
+                debug!("Failed to add sample to the profile: {err}")
             }
         }
     }